Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Complete Refactor, also add track macro and ::expr, .var commands. #10

Merged
merged 20 commits into from
Sep 18, 2023

Conversation

entropylost
Copy link
Contributor

This pull request does a number of things:

  • It includes the Float4::expr statements from Replace make_float4 with Float4::expr, etc. #7.
  • It completely refactors the luisa_compute crate, making the luisa_compute::prelude import suitable for general use, and reducing things exposed at the top level.
  • It removes the const_, var!, def! functions, instead replacing them with x.expr(), Var::<X>::zeroed(), and x.var() respectively (x.var() works for both values and exprs).
  • It adds a new macro track, which significantly simplifies kernel syntax.

Refactoring changes:

The primary change made is that after importing luisa::prelude, the primitive expression types are hidden behind an import of luisa::lang::types::core::*. Similarily, Float4Expr is behind luisa::lang::types::vector. This is done as there usually isn't much of a reason to use the direct Float4Expr, Float types, and the Expr<f32> types are easier to keep track of due to their regularity.

Additionally, luisa::lang::printer was moved to luisa::printer, most functions were put in luisa::lang::functions, and control flow in luisa::lang::control_flow, and the previous Kernel types in luisa::lang were moved to luisa::runtime::kernel, which is exported in luisa::runtime. I also added a luisa::internal_prelude module, which provides most imports necessary normally.

Why remove const_?

The primary reason I made this change is that the const_, var!, def! expressions were unnecessarily noisy, and somewhat confusing about their purpose - var! does not provide an indication anywhere that it fills a place with zeros.

track macro:

The track macro rewrites the expression within it, replacing comparison and control flow operators (and || and &&) with variants that can work either in an Expr context or outside of it. For example, this allowed converting this section in the mpm example:

|| {
    // ...
    let vx = select(
        (coord.x().cmplt(BOUND) & vx.cmplt(0.0f32))
            | (coord.x() + BOUND).cmpgt(N_GRID as u32) & vx.cmpgt(0.0f32),
        0.0f32.into(),
        vx,
    );
    let vy = select(
        (coord.y().cmplt(BOUND) & vy.cmplt(0.0f32))
            | (coord.y() + BOUND).cmpgt(N_GRID as u32) & vy.cmpgt(0.0f32),
        0.0f32.into(),
        vy,
    );
    // ...
}

into

track!(|| {
    // ...
    let vx = select(
        coord.x() < BOUND && (vx < 0.0f32)
            || coord.x() + BOUND > N_GRID as u32 && (vx > 0.0f32),
        0.0f32.into(),
        vx,
    );
    let vy = select(
        coord.y() < BOUND && (vy < 0.0f32)
            || coord.y() + BOUND > N_GRID as u32 && (vy > 0.0f32),
        0.0f32.into(),
        vy,
    );
    // ...
})

which is significantly easier to read.

@shiinamiyuki
Copy link
Collaborator

Thank you! It looks amazing! Let me review the changes and I'll merge if everything looks good.

Copy link
Collaborator

@shiinamiyuki shiinamiyuki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good.

@shiinamiyuki shiinamiyuki merged commit dc6338c into LuisaGroup:main Sep 18, 2023
1 check passed
@entropylost entropylost deleted the refactor branch September 18, 2023 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants