Skip to content

[CIR] Streamline creation of mlir::IntegerAttrs using mlir::Builder #1645

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

Merged
merged 1 commit into from
May 27, 2025

Conversation

xlauko
Copy link
Collaborator

@xlauko xlauko commented May 26, 2025

  • Uses getI<bitwidth>IntegerAttr builder method instead of explicit attribute and its type creation.
  • Adds few helper functions getAlignmentAttr to build alignment representing mlir::IntegerAttr.
  • Removes duplicit type parameters, that are inferred from mlir::IntegerAttr.

@xlauko xlauko marked this pull request as ready for review May 26, 2025 11:07
@xlauko
Copy link
Collaborator Author

xlauko commented May 26, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@xlauko xlauko force-pushed the users/xlauko/cir-integer-attr-builders branch from f31f26d to 3e78c65 Compare May 26, 2025 11:11
@xlauko
Copy link
Collaborator Author

xlauko commented May 26, 2025

@bcardosolopes I believe

add(mlir::IntegerAttr::get(intTy,
use of mlir::IntegerAttr is not correct as it disallows non-core types, but at this location it is created with cir::IntType.

It is probably nowhere exercised as making it NYI did not crashed any test. Should I make it NYI for now?

@bcardosolopes
Copy link
Member

use of mlir::IntegerAttr is not correct as it disallows non-core types, but at this location it is created with cir::IntType.
It is probably nowhere exercised as making it NYI did not crashed any test. Should I make it NYI for now?

If it has no users we should probably just delete for now.

@bcardosolopes bcardosolopes merged commit 576d2ff into main May 27, 2025
9 checks passed
xlauko added a commit to llvm/llvm-project that referenced this pull request May 28, 2025
- Uses getI<bitwidth>IntegerAttr builder method instead of explicit attribute and its type creation.
- Adds few helper functions `getAlignmentAttr` to build alignment representing mlir::IntegerAttr.
- Removes duplicit type parameters, that are inferred from mlir::IntegerAttr.

This mirrors incubator changes from llvm/clangir#1645 (comment)
xlauko added a commit to llvm/llvm-project that referenced this pull request May 29, 2025
- Uses getI<bitwidth>IntegerAttr builder method instead of explicit attribute and its type creation.
- Adds few helper functions `getAlignmentAttr` to build alignment representing mlir::IntegerAttr.
- Removes duplicit type parameters, that are inferred from mlir::IntegerAttr.

This mirrors incubator changes from llvm/clangir#1645 (comment)
xlauko added a commit to llvm/llvm-project that referenced this pull request May 29, 2025
…141830)

- Uses getI<bitwidth>IntegerAttr builder method instead of explicit attribute and its type creation.
- Adds few helper functions `getAlignmentAttr` to build alignment representing mlir::IntegerAttr.
- Removes duplicit type parameters, that are inferred from mlir::IntegerAttr.

This mirrors incubator changes from llvm/clangir#1645 (comment)
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request May 29, 2025
…::Builder (#141830)

- Uses getI<bitwidth>IntegerAttr builder method instead of explicit attribute and its type creation.
- Adds few helper functions `getAlignmentAttr` to build alignment representing mlir::IntegerAttr.
- Removes duplicit type parameters, that are inferred from mlir::IntegerAttr.

This mirrors incubator changes from llvm/clangir#1645 (comment)
xlauko added a commit that referenced this pull request May 29, 2025
The current implementation incorrectly uses `mlir::IntegerAttr::get` with the unsupported type `cir::IntType`, which is not compatible and was never tested. As discussed in PR #1645, this is to be marked as NYI until a proper implementation is provided.
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