[PATCH] D128738: [RISCV] Match RISCVISD::ADD_LO in SelectAddrRegImm.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp:1895
+          // that margin; if so (low part + CVal) can't overflow.
+          const DataLayout &DL = CurDAG->getDataLayout();
+          Align Alignment = GA->getGlobal()->getPointerAlignment(DL);
----------------
reames wrote:
> Er, correct me if I'm wrong here, but don't we need to be checking the combined offset for safety against the alignment?  If the original GV had a base aligned to 16, and an offset of 15, wouldn't adding a new offset of 15 cause problems?  We could in theory adjust the base, but do we actually do that?
> 
> (If this concern is valid, the original code had it too.)
I think you're right. As far as I know GA->getOffset() is always 0 which is probably why it hasn't caused a problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128738