[PATCH] D98860: [RISCV] Optimize all-constant mask BUILD_VECTORs

[Fraser Cormack via Phabricator via llvm-commits]

frasercrmck marked an inline comment as done.
frasercrmck added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:1178
+        bool BitValue = !V.isUndef() && cast<ConstantSDNode>(V)->getZExtValue();
+        Bits |= (BitValue << BitPos);
+      }
----------------
craig.topper wrote:
> 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.
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98860/new/

https://reviews.llvm.org/D98860