[PATCH] D105069: [GlobalISel] Add re-association combine for G_PTR_ADD to allow better addressing mode usage.

[Amara Emerson via Phabricator via llvm-commits]

aemerson added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:1677
+  // to go beyond the bounds of our legal addressing modes.
+  if (!MRI.hasOneNonDBGUse(Add2))
+    return false;
----------------
foad wrote:
> aemerson wrote:
> > arsenm wrote:
> > > Check isLegalAddressingMode, or some similar hook?
> > I don't think it's necessary for this. We should avoid doing this combine no matter what with multiple users since doing so doesn't give any benefit since the def can't be deleted.
> > We should avoid doing this combine no matter what with multiple users since doing so doesn't give any benefit since the def can't be deleted.
> 
> I don't understand. If Add2 is used by two ptr_adds, and both of them can be folded to give legal addresses, then we should do that and delete Add2. We are seeing some cases downstream where that is no longer happening because of this check.
Ok, fair enough. I’ll look into that case today.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105069