This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Optimize all-constant mask BUILD_VECTORs
ClosedPublic

Authored by frasercrmck on Mar 18 2021, 5:26 AM.

Details

Summary

This patch adds an optimization for mask-vector BUILD_VECTOR nodes whose
elements are all constants or undef. It lowers such operations by
building up the vector via a series of integer operations, in which
multiple mask elements are inserted into a vector at a time via
i8/i16/i32/i64 element types. The final result is then bitcast from that
integer vector.

We restrict this optimization in certain circumstances when optimizing
for size. If we are required to use more than one integer insert
operation, then it will likely increase code size compared with using a
load from a constant pool.

Diff Detail

Event Timeline

frasercrmck created this revision.Mar 18 2021, 5:26 AM
frasercrmck requested review of this revision.Mar 18 2021, 5:26 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 18 2021, 5:26 AM
craig.topper added inline comments.Mar 19 2021, 11:17 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
1186

I think BitValue needs to be casted to uint64_t otherwise NumViaIntegerBits==64 doesn't work. I think otherwise BitValue is promoted to 'int' for the shift. Which is UB if BitPos > 32. Then I think the shift result is sign extended to be 64 bits for the OR.

If NumViaIntegerBits is 32 or less you probably want to sign extend Bits from 32 to 64 to get optimal constant materialization.

frasercrmck marked an inline comment as done.Mar 22 2021, 8:07 AM
frasercrmck added inline comments.
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
1186

Yes you're right. Not the first time I've made that mistake, sadly. Thanks.

Can we not sign extend from NumViaIntegerBits to 64? I was thinking you might better catch some i8/i16s when the sign-extended value is easier to materialize.

craig.topper added inline comments.Mar 22 2021, 8:29 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
1186

That would be fine. I’m not sure it will have any effect on the number of instructions needed to materialize.

frasercrmck marked 2 inline comments as done.
  • rebase
  • fix shift overflow
  • sign-extend constants from 32 to 64 to help materializatoin
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
1186

No it doesn't seem to, and I think it the sign-extension makes some of the constants harder to intuit. So I've left it at 32.

This revision is now accepted and ready to land.Mar 22 2021, 10:38 AM
This revision was automatically updated to reflect the committed changes.