[PATCH] D109528: [AArch64][GlobalISel] Add a new reassociation for G_PTR_ADDs.

[Amara Emerson via Phabricator via llvm-commits]

aemerson added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:4139-4146
+    LHSPtrAdd->moveBefore(&MI);
+    Register RHSReg = MI.getOffsetReg();
+    Observer.changingInstr(MI);
+    MI.getOperand(2).setReg(LHSCstOff);
+    Observer.changedInstr(MI);
+    Observer.changingInstr(*LHSPtrAdd);
+    LHSPtrAdd->getOperand(2).setReg(RHSReg);
----------------
arsenm wrote:
> Would it be cleaner to just insert a fresh instruction where it needs to be and let CSE deal with it?
I don't really think that's cleaner since it has the same effect and would make the original G_PTR_ADD dead. Seems better to avoid creating a new instruction and just move it.

But it just occurred to me that since we have a oneuse check for the inner G_PTR_ADD, there shouldn't be a problem in having the two in different blocks, sinking it can't break any other uses.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109528