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

Initial changes for llvm uplift #2568

Merged
merged 10 commits into from
Nov 14, 2023
Merged

Conversation

philass
Copy link
Member

@philass philass commented Oct 17, 2023

The changes in this uplift are mainly related to following the migration guide in this discussion post https://discourse.llvm.org/t/psa-bufferization-new-buffer-deallocation-pipeline/73375/11

@philass
Copy link
Member Author

philass commented Oct 31, 2023

The final issues seem to be related to https://discourse.llvm.org/t/psa-bufferization-new-buffer-deallocation-pipeline/73375/11

I'm working through them now

@philass philass force-pushed the plassen/llvm-update branch from 90be6e1 to aac995d Compare November 8, 2023 00:35
Copy link
Member Author

Choose a reason for hiding this comment

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

This is quite broken. This just disables the checks and offending ops

Copy link
Collaborator

Choose a reason for hiding this comment

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

@philass I understand your pain... the same goes for ONNX upgrade ( a lot of issues). Are we planning on just waiting for the next torch-mlir /llvm commit or are you going to work through this still?

I noticed torch-mlir has upgraded to a more recent llvm commit as well: llvm/torch-mlir#2517

Copy link
Member Author

Choose a reason for hiding this comment

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

I will land this PR. Than we can figure out how the next bump should go

@philass philass force-pushed the plassen/llvm-update branch from 4f464a7 to 9daceb3 Compare November 8, 2023 17:12
@philass philass force-pushed the plassen/llvm-update branch from 9daceb3 to f152ff5 Compare November 8, 2023 17:13
@philass
Copy link
Member Author

philass commented Nov 14, 2023

Hey @AlexandreEichenberger, @tungld, @chentong319

This should be ready for review. Only one lit test has a lingering issue. All other lit tests and end to end tests pass.

test/mlir/driver/buffer_loop_hoisting.mlir no longer works. I don't understand the part of the codebase too well. Everything else succeeds. How would you guys suggest I proceed.

Selfishly I would like to either

  1. Delete the test
  2. Leave the test changed as it is right now in the PR to get the lit tests to pass.

The test can than be updated at a later point in time once it is fully triaged.

Someone familiar with this part of the codebase could triage the issue. The LLVM dialect, memref, and bufferization dialect or not things I regularly work with. So I'm likely not the best person to trouble shoot this lit test.

This would allow us to unblock the LLVM uplift which has already slipped (Really my fault there).

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.

LGTM, thanks for working on this, much appreciated.

Would it make sense to open an issue with the lit test you had to yank?

@philass
Copy link
Member Author

philass commented Nov 14, 2023

@AlexandreEichenberger Thanks for the review

Would it make sense to open an issue with the lit test you had to yank?

Will do

@philass philass merged commit 96a2558 into onnx:main Nov 14, 2023
5 checks passed
@jenkins-droid
Copy link
Collaborator

Jenkins Linux ppc64le Build #12432 [push] Initial changes for llvm... started at 14:11

@jenkins-droid
Copy link
Collaborator

Jenkins Linux s390x Build #13438 [push] Initial changes for llvm... started at 14:02

@jenkins-droid
Copy link
Collaborator

Jenkins Linux amd64 Build #13411 [push] Initial changes for llvm... started at 13:02

@jenkins-droid
Copy link
Collaborator

Jenkins Linux s390x Build #13438 [push] Initial changes for llvm... failed after 31 min

@jenkins-droid
Copy link
Collaborator

Jenkins Linux amd64 Build #13411 [push] Initial changes for llvm... failed after 1 hr 32 min

@jenkins-droid
Copy link
Collaborator

Jenkins Linux ppc64le Build #12432 [push] Initial changes for llvm... failed after 2 hr 22 min

cjvolzka added a commit to cjvolzka/onnx-mlir that referenced this pull request Nov 15, 2023
* 'main' of github.ibm.com:zosdev/onnx-mlir:
  Use dim_params in dynamic dimension analysis (onnx#2620)
  Update rapidcheck to include the fix for missing <cstdint> include (onnx#2623)
  Initial changes for llvm uplift (onnx#2568)
  [build] Add cmake option to enable/disable Java components build (onnx#2613)
  [StableHLO] Lowers PadOp (constant mode) & GatherElements Op to StableHLO (onnx#2602)
  [DialectBuilder] add builder funcrions for ONNXSumOp and ONNXConvOp (onnx#2572)
  Import dim_param for model inputs and outputs (onnx#2616)
  Parse and set --mcpu in onnx-mlir-opt command (onnx#2614)
  Layernorm: convert instance norm and group norm to layer norm. (onnx#2595)
  [NNPA] Use F16 as element type for zTensor (onnx#2611)
  detect LayerNorm in presence of reciprocal and div of 1 (onnx#2609)

# Conflicts:
#	test/mlir/conversion/onnx_to_krnl/NN/Normalization_O3_SIMD_canonicalize.mlir
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