[PATCH] D150230: [GlobalISel] Implement some binary reassociations, G_ADD for now

[Amara Emerson via Phabricator via llvm-commits]

aemerson added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:4490
+
+  std::optional<APInt> C1 =
+      isConstantOrConstantSplatVector(*MRI.getVRegDef(Op01), MRI);
----------------
paquette wrote:
> might as well just return false here if `!C1` to save a level of indentation
I actually left it like this to prepare for future reassociations that don't need C1 to match.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:4503
+    }
+    if (getTargetLowering().isReassocProfitable(MRI, Op0, Op1)) {
+      // Reassociate: (op (op x, c1), y) -> (op (op x, y), c1)
----------------
paquette wrote:
> maybe pull the `MRI.hasOneNonDBGUse(Op00)` in here and then we don't need the second `if`?
Sure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150230