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

Integrate LLVM at llvm/llvm-project@8bd65e5 #343

Merged
merged 2 commits into from
Jan 16, 2023
Merged

Conversation

marbre
Copy link
Contributor

@marbre marbre commented Jan 11, 2023

Updates LLVM usage to match llvm/llvm-project@8bd65e5.

Further updates the MLIR-HLO submodule to tensorflow/mlir-hlo@78c6dcf.

@marbre marbre force-pushed the integrate branch 2 times, most recently from 983359b to 5fe5be1 Compare January 11, 2023 15:37
Updates LLVM usage to match llvm/llvm-project@8bd65e5.
* TOSA ops switched to DenseArrayAttr with commit llvm/llvm-project@9e1a344.
  The types of attributes are therfore aligned in the reference
  implementation.
* Uses std::nullopt instead of None.
* Moves from llvm::makeArrayRef to ArrayRef deduction guides, see
  https://reviews.llvm.org/D140896.

Further updates the MLIR-HLO submodule to tensorflow/mlir-hlo@78c6dcf
and bumps TensorFlow to tf-nightly 2.12.0.dev20230112.
@marbre marbre marked this pull request as ready for review January 12, 2023 15:55
@marbre
Copy link
Contributor Author

marbre commented Jan 12, 2023

@NatashaKnk this would change reference implementation for the tosa.tile op as tileOp.getMultiplesAttr() now returns an DenseArrayAttr which is emitted as int64_t by EmitC. Would this be okay for you or do we need to handle this in a different way? I am aware that the spec still states it should be an int32_t...

Copy link
Member

@simon-camp simon-camp left a comment

Choose a reason for hiding this comment

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

I've created a tracking issue for the disabled test, otherwise LGTM

reference-implementation/unittests/tosa.cpp Outdated Show resolved Hide resolved
Co-authored-by: Simon Camphausen <simon.camphausen@iml.fraunhofer.de>
@NatashaKnk
Copy link
Contributor

This would need to be changed to match up with the TOSA spec, but since we don't have anything running off it yet I think it's okay to submit as-is and I'll send a follow-up PR. Thanks for the heads up!

@marbre marbre merged commit 55b5669 into iml130:main Jan 16, 2023
@marbre marbre deleted the integrate branch January 16, 2023 17:16
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