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

Proposal: Struct Exprs #11

Closed
entropylost opened this issue Sep 18, 2023 · 5 comments
Closed

Proposal: Struct Exprs #11

entropylost opened this issue Sep 18, 2023 · 5 comments

Comments

@entropylost
Copy link
Contributor

(From #7 )
@shiinamiyuki I managed to make a way of using structs for Expr and Var that still allows impls.

The primary code for this is in https://github.com/iMplode-nZ/luisa_expr_tests/blob/main/a/src/lib.rs and https://github.com/iMplode-nZ/luisa_expr_tests/blob/main/b/src/main.rs.

TLDR: I made the Value type provide custom ExprProxy and VarProxy types, and implemented Deref<Target = T::ExprProxy> for Expr<T>, with the enforcement that the ExprProxy must be of the same layout, so that this deref can be implemented using a transmute. Then, the user can implement things on their custom ExprProxy type. I also made the VarProxy be required to implement Deref<Target = Expr<T>> so that you can treat a Var as an Expr.

I'd like to ask clarification on a number of issues:

  • In luisa, does calling vector.x() cause side effects / add code to the kernel, if the value of it isn't used?
  • Do people think it'd be worth implementing direct vector.x accessors using the hack in main.rs, or would that not be worth it since it'd make people be able to screw up by setting the x values? (Perhaps that could be avoided with a custom lint?)
@entropylost
Copy link
Contributor Author

(Also, if anybody has a discord server or another way of communicating that doesn't have a 1 day round trip, that'd be nice to know)

@shiinamiyuki
Copy link
Collaborator

shiinamiyuki commented Sep 18, 2023

In luisa, does calling vector.x() cause side effects / add code to the kernel, if the value of it isn't used?

It does add code to the kernel. It won't affect performance though. (Maybe increased compile time?)

(Also, if anybody has a discord server or another way of communicating that doesn't have a 1 day round trip, that'd be nice to know)

You can use https://discord.com/invite/ymYEBkUa7F

  • Do people think it'd be worth implementing direct vector.x accessors using the hack in main.rs, or would that not be worth it since it'd make people be able to screw up by setting the x values? (Perhaps that could be avoided with a custom lint?)

We have discussed the struct.field syntax a while ago. C++ DSL is using this syntax and we believe it is also doable here (similar to your solution). There is a Bump in the RECORDER in lang.rs which is specifically reserved for these hacks.
There are some historical reasons why we choose to use struct.field() here: It has a 1-to-1 mapping with IR and generates super clean code that makes debugging the backend and IR transformation passes much easier, which is especially useful as this library is used in an academic project where we have to met certain deadlines.
Now the IR is already stabilized so maybe we should adopt the more intuitive struct.field syntax.

At the meantime, the v.store(*v + 1) and *v.get_mut() syntax is very annoying. I think it would be nice if we can support something like *v.x +=1. (Of course you cannot actually make v mutable as it just breaks the DSL). I'm wondering if this can be solved either by using the track! macro or some other hack?

@entropylost
Copy link
Contributor Author

@shiinamiyuki I can definitely implement the get_mut hijack with track! It also might be possible to make the foo.x generate nicer IR by making it only add the x() to the DSL upon deref.

@entropylost
Copy link
Contributor Author

I'd like to handle that after doing the struct expr thing if doing that, since it'd make managing things significantly simpler.

@shiinamiyuki
Copy link
Collaborator

Closing as it is already handled in #12

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

No branches or pull requests

2 participants