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

Upgrading llvm and stablehlo hash #3053

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

christopherlmunoz
Copy link
Contributor

llvm: b270525f730be6e7196667925f5a9bfa153262e9
stablehlo: 459e481b

Copy link
Collaborator

@hamptonm1 hamptonm1 left a comment

Choose a reason for hiding this comment

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

Looks good but I had one comment

@@ -48,7 +48,7 @@ Value buildOnnxToTosaPaddingConstOp(mlir::PatternRewriter &rewriter,

// Create a new pad vec in the right format
// ONNX : [b1, b2, b3, b4, e1, e2, e3, e4]
// TOSA :[[b1, e1], [b2, e2], [b3, e3], [b4, e4]]
// TOSA :[b1, e1, b2, e2, b3, e3, b4, e4]
Copy link
Collaborator

Choose a reason for hiding this comment

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

@jorickert This update was made due to the following error message :

/Users/christophermunoz/repos/cmunoz1/onnx-mlir/test/mlir/conversion/onnx_to_tosa/NN/AveragePool.mlir:139:8: error: 'tosa.pad' op operand #1 must be 1D tensor of 32-bit signless integer or 64-bit signless integer values, but got 'tensor<4x2xi64>'

Just want to make sure this update is fine.... I am not too familiar with TOSA, so asking just in case.

Copy link
Collaborator

Choose a reason for hiding this comment

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

LLVM made some updates to TOSA as shown in this PR: llvm/llvm-project#122547

Copy link
Collaborator

@AlexandreEichenberger AlexandreEichenberger left a comment

Choose a reason for hiding this comment

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

@hamptonm1 thanks for reviewing this, I will assume that you will approve it once you are satisfied with the answers to your question.

Signed-off-by: Christopher Munoz <[email protected]>

formatting fixes
@christopherlmunoz christopherlmunoz force-pushed the llvm_update branch 2 times, most recently from 272152d to 3784890 Compare January 30, 2025 13:02
@hamptonm1
Copy link
Collaborator

@AlexandreEichenberger @tungld I think Chris mentioned he was working with you guys to resolve the round backend test but if we can not get it resolved, would it be okay to comment it out and open an issue to fix it later on?

----------------------------- Captured stderr call -----------------------------
failed to legalize operation 'vector.shape_cast'
=========================== short test summary info ============================
 Release/test.py::OnnxBackendNodeModelTest::test_round_cpu - subprocess.Calle...

Signed-off-by: Christopher Munoz <[email protected]>
@christopherlmunoz christopherlmunoz force-pushed the llvm_update branch 2 times, most recently from 8b9e8b1 to c6307f4 Compare February 6, 2025 02:42
@christopherlmunoz
Copy link
Contributor Author

@AlexandreEichenberger @tungld I think Chris mentioned he was working with you guys to resolve the round backend test but if we can not get it resolved, would it be okay to comment it out and open an issue to fix it later on?

----------------------------- Captured stderr call -----------------------------
failed to legalize operation 'vector.shape_cast'
=========================== short test summary info ============================
 Release/test.py::OnnxBackendNodeModelTest::test_round_cpu - subprocess.Calle...

I spoke with Tung again. Created issue #3068 and commented out tests. Will continue the investigation and work to resolve this asap.

Signed-off-by: Christopher Munoz <[email protected]>
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.

3 participants