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

[Fraser Cormack via Phabricator via llvm-commits]

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:
> frasercrmck wrote:
> > 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.
> That would be fine. I’m not sure it will have any effect on the number of instructions needed to materialize.
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98860