This is an archive of the discontinued LLVM Phabricator instance.

[mlir] Switch segment size attributes to DenseI32ArrayAttr
ClosedPublic

Authored by Mogball on Aug 11 2022, 10:59 AM.

Details

Summary

Switch variadic operand and result segment size attributes to use the
dense i32 array. Dense integer arrays were introduced primarily to
represent index lists. They are a better fit for segment sizes than
dense elements attrs.

Depends on D131738

Diff Detail

Event Timeline

Mogball created this revision.Aug 11 2022, 10:59 AM
Herald added a reviewer: rriddle. · View Herald Transcript
Herald added a reviewer: aartbik. · View Herald Transcript
Herald added a reviewer: ftynse. · View Herald Transcript
Herald added projects: Restricted Project, Restricted Project. · View Herald Transcript
Mogball requested review of this revision.Aug 11 2022, 10:59 AM
jpienaar added inline comments.
flang/test/Fir/convert-to-llvm-openmp-and-fir.fir
204

Now the discussion on the generic syntax for this becomes more important: it feels unfortunate to introduce this widely if we are planning to change it.

Mogball added inline comments.Aug 11 2022, 11:07 AM
flang/test/Fir/convert-to-llvm-openmp-and-fir.fir
204

Yes. I am open to changing it myself. But what mnemonic did we decide on? vector< or array< or dense-array<? I'm more partial towards just array< even though it may create confusion with array attr.

Mogball updated this revision to Diff 452030.Aug 11 2022, 4:19 PM

update for changed syntax

Mogball edited the summary of this revision. (Show Details)Aug 11 2022, 4:20 PM
Mogball marked an inline comment as done.
This revision is now accepted and ready to land.Aug 11 2022, 4:25 PM
This revision was landed with ongoing or failed builds.Aug 11 2022, 5:56 PM
This revision was automatically updated to reflect the committed changes.

This didn't update Python (beyond apparently grep/sed comment change).

Repro: ~/llvm-project/build$ PYTHONPATH=$PWD/tools/mlir/python_packages/mlir_core:$PYTHONPATH python3 ../mlir/test/python/dialects/ods_helpers.py | FileCheck ../mlir/test/python/dialects/ods_helpers.py

There is no quick fix forward as DenseI*ArrayAttr doesn't seem to be exposed to either C API or Python. I will revert for now.

I'll send a patch for python bindings and then re-land this one.