[PATCH] D126986: [RISCV] Support LUI+ADDIW in doPeepholeLoadStoreADDI.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp:2248
+    Offset += cast<ConstantSDNode>(Base.getOperand(1))->getSExtValue();
+    if (!isInt<32>(Offset))
+      return false;
----------------
reames wrote:
> Correct me if I'm wrong here, but isn't this the same as proving we could replace the ADDIW with an ADDI?  If so, should that be done somewhere else generically?
Yes. So we definitely teach constant materialization to emit LUI+ADDI instead of LUI+ADDIW for more cases. And update a bunch of tests.

But I think we would then need to teach the RISCVSExtWRemoval pass that LUI+ADDI produces a sign extended result most of the time. It already knows ADDIW always produces a sign extended result. So as long as we always emit LUI+ADDIW we didn't need a special case.

So we'd just be moving the problem around.

Since I also wrote something similar to this patch for RISCVMergeBaseOffset D126729. Maybe that's another vote in favor of changing so we only have to deal with it in RISCVSExtWRemoval?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126986