[PATCH] D79690: [RISCV] Fold ADDIs into load/stores with nonzero offsets

[Luís Marques via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp:219-224
+      int64_t Offset1 = Const->getSExtValue();
+      int64_t CombinedOffset = Offset1 + Offset2;
+      if (!isInt<12>(CombinedOffset))
+        continue;
+      ImmOperand = CurDAG->getTargetConstant(CombinedOffset, SDLoc(ImmOperand),
+                                             ImmOperand.getValueType());
----------------
lenary wrote:
> I don't understand where this code is being tested. Can you point to a testcase that changes because of this change?
That's the situation I mention in the review summary. That case doesn't seem to currently be generated by LLVM, so I couldn't generate a test that would be impacted by it. I can try to look harder or see if e.g. a MIR test could cover that case. Since the code was reasonably straightforward and the alternative (skipping on 0) was actually being harder to document in the code comments I kept the optimization, since it should be reasonably clear to reason about it being correct or not, despite the lack of specific test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79690