[PATCH] D116574: Materializing constants with 'rori'

[Baoshan Pang via Phabricator via llvm-commits]

BaoshanPang marked 15 inline comments as done.
BaoshanPang added a comment.

@craig.topper

Thanks for helping on review.
Please help to check if there is still any issue with new version.



================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVMatInt.cpp:157-158
+    // for case: 0xaffffffffffffffa
+    if (UpperTrailingOnes < 32 && LowerLeadingOnes < 32 &&
+        ((64 - UpperTrailingOnes - LowerLeadingOnes) < 12)) {
+      NegImm12 = (Val << (32 - UpperTrailingOnes)) |
----------------
jrtc27 wrote:
> Again this seems overly strict, you just need LowerLeadingOnes + UpperTrailingOnes > (64 - 12) and 0 < UpperTrailingOnes < 32?
Yes, I agree we should only use "LowerLeadingOnes + UpperTrailingOnes > (64 - 12)".

For 0 < UpperTrailingOnes < 32, it seems we don't need it, as:
if UpperTrailingOnes is 0 then it is impossible to meet: LowerLeadingOnes + UpperTrailingOnes > (64 - 12)
and if UpperTrailingOnes is 32 then the 'Val' should be handled by case 1.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116574