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

Add custom builder to reduce op allowing type inference #1965

Merged
merged 3 commits into from
Jan 30, 2024

Conversation

sdasgup3
Copy link
Member

The PR implements the a custom reduce op builder similar to what we have for mhlo code.

Background

#1869 allows the block arguments of reduce op to have different element types than that of the input arguments of reduce op and the output element type of the reduce op has to equal to those block arguments. As a consequence the output type of reduce op can no longer be inferred from the operand types. The auto-generated builders creates a reduce op with empty block and, as a result, does not allow inferring the type.

The proposed solution is to create a custom builder which takes the element-type of the block arguments as arguments allowing result type inference.

@sdasgup3 sdasgup3 requested a review from GleasonK January 29, 2024 20:49
@sdasgup3 sdasgup3 changed the title Add custom builder to reduce op Add custom builder to reduce op allowing type inference Jan 29, 2024
Copy link
Contributor

@mlevesquedion mlevesquedion left a comment

Choose a reason for hiding this comment

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

LGTM. Consider adding some kind of test for this builder.

@sjain-stanford
Copy link
Contributor

@sdasgup3 Gently checking-in on this, and appreciate your help here. Unfortunately cherry-picking from your fork doesn't play well with the git submodule setup in torch-mlir which expects a commit on main, so I can't proceed with the fix without this landing.

 Error: fatal: Fetched in submodule path 'externals/stablehlo', but it did not contain f2b6877713027370f26569929fb5d841a79d3589. Direct fetching of that commit failed.

stablehlo/dialect/StablehloOps.cpp Outdated Show resolved Hide resolved
stablehlo/dialect/StablehloOps.cpp Outdated Show resolved Hide resolved
@sdasgup3 sdasgup3 merged commit 728a7b1 into openxla:main Jan 30, 2024
9 checks passed
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.

4 participants