Page MenuHomePhabricator

[MLIR] Add a bitcast method to DenseElementsAttr
ClosedPublic

Authored by GMNGeoffrey on Aug 5 2021, 4:31 PM.

Details

Summary

This method bitcasts a DenseElementsAttr elementwise to one of the same
shape with a different element type.

Diff Detail

Event Timeline

GMNGeoffrey created this revision.Aug 5 2021, 4:31 PM
GMNGeoffrey requested review of this revision.Aug 5 2021, 4:31 PM

This modifies the bitcast folders added in https://reviews.llvm.org/D105376, so is dependent on that patch.

rriddle added inline comments.Aug 5 2021, 4:40 PM
mlir/lib/IR/BuiltinAttributes.cpp
1027

This should be true on construction.

1044

Is this necessary? It's used above.

1046–1047

If we use getDenseElementBitWidth here, you could drop the isIntOrFloat checks and ideally support complex as well.

1049

You'll should also probably assert that the number of elements match.

mlir/lib/IR/BuiltinTypes.cpp
448–455 ↗(On Diff #364649)

This looks unrelated.

GMNGeoffrey marked 3 inline comments as done.
  • Respond to review comments
GMNGeoffrey marked 2 inline comments as done.Aug 6 2021, 4:02 PM
GMNGeoffrey added inline comments.
mlir/lib/IR/BuiltinAttributes.cpp
1027

It was intended to be a check for integer/float. Deleted though, per your comment below.

1044

It's not. I was following the pattern in reshape, which I assumed was intended as defense against some future mistake (or just a general principle of putting void casts before asserts with anything they use. I'll delete this and the one above, if this isn't some pattern I'm supposed to be following.

1046–1047

Done.

1049

That's true by construction with clone (this accepts an element type not a ShapedType)

mlir/lib/IR/BuiltinTypes.cpp
448–455 ↗(On Diff #364649)

This was something I added while debugging. I've split it into a separate patch: https://reviews.llvm.org/D107676. Better debug messages are generally helpful though, I think :-)

rriddle accepted this revision.Aug 9 2021, 4:55 PM
This revision is now accepted and ready to land.Aug 9 2021, 4:55 PM
This revision was automatically updated to reflect the committed changes.
GMNGeoffrey marked 2 inline comments as done.