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

IfOp #149

Merged
merged 5 commits into from
Oct 31, 2024
Merged

IfOp #149

merged 5 commits into from
Oct 31, 2024

Conversation

Pangoraw
Copy link
Collaborator

@Pangoraw Pangoraw commented Oct 29, 2024

  • region inlining if predicate is known
  • transform to select for values which are not defined in inner regions
  • forward
  • reverse

I don't think the reverse could be used as is in a XLA pipeline since Enzyme is not yet able to remove all enzyme ops (which I think it technically could if there is no loop?).

Copy link
Member

@wsmoses wsmoses left a comment

Choose a reason for hiding this comment

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

lgtm [though also we'll need to figure out a good interface for the push/pop removal]

@Pangoraw
Copy link
Collaborator Author

though also we'll need to figure out a good interface for the push/pop removal

Yes. I also took a look at BatchOpInterface for IfOp and in its current state the interface only allows to return a single operation so we could not really unroll the batching like we discussed. So maybe BatchOpInterface has to be modified to provide a IRBuilder to createBatch and implementers do the result mapping themselves.

@wsmoses
Copy link
Member

wsmoses commented Oct 31, 2024

though also we'll need to figure out a good interface for the push/pop removal

Yes. I also took a look at BatchOpInterface for IfOp and in its current state the interface only allows to return a single operation so we could not really unroll the batching like we discussed. So maybe BatchOpInterface has to be modified to provide a IRBuilder to createBatch and implementers do the result mapping themselves.

Yeah I think so, as is it assumed a single op would suffice for batching

@wsmoses wsmoses merged commit 6109edc into EnzymeAD:main Oct 31, 2024
5 of 9 checks passed
@Pangoraw Pangoraw deleted the if branch November 2, 2024 10:46
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