[llvm] LICM: hoist BO assoc when BinOp is in RHS (PR #107072)

[Nikita Popov via llvm-commits]

================
@@ -2825,30 +2823,45 @@ static bool hoistBOAssociation(Instruction &I, Loop &L,
   if (Opcode != Instruction::Add && Opcode != Instruction::Mul)
     return false;
 
-  auto *BO0 = dyn_cast<BinaryOperator>(BO->getOperand(0));
+  bool BinOpInRHS = isa<BinaryOperator>(BO->getOperand(1));
+  auto *BO0 = dyn_cast<BinaryOperator>(BO->getOperand(BinOpInRHS));
   if (!BO0 || BO0->getOpcode() != Opcode || !BO0->isAssociative() ||
       BO0->hasNUsesOrMore(3))
     return false;
 
   Value *LV = BO0->getOperand(0);
   Value *C1 = BO0->getOperand(1);
-  Value *C2 = BO->getOperand(1);
+  Value *C2 = BO->getOperand(!BinOpInRHS);
 
-  if (L.isLoopInvariant(LV) && !L.isLoopInvariant(C1)) {
-    assert(BO0->isCommutative() && "Associativity implies commutativity");
+  if (!L.isLoopInvariant(C2))
+    return false;
+  if (!L.isLoopInvariant(LV) && L.isLoopInvariant(C1)) {
+    if (BinOpInRHS)
+      assert(BO0->isCommutative() && "Associativity implies commutativity");
+  } else if (L.isLoopInvariant(LV) && !L.isLoopInvariant(C1)) {
+    if (!BinOpInRHS)
+      assert(BO0->isCommutative() && "Associativity implies commutativity");
     std::swap(LV, C1);
-  }
-  if (L.isLoopInvariant(LV) || !L.isLoopInvariant(C1) || !L.isLoopInvariant(C2))
+  } else {
     return false;
+  }
 
   auto *Preheader = L.getLoopPreheader();
   assert(Preheader && "Loop is not in simplify form?");
 
+  // To create C2 op C1, instead of C1 op C2.
+  if (BinOpInRHS)
+    std::swap(C1, C2);
+
   IRBuilder<> Builder(Preheader->getTerminator());
   auto *Inv = Builder.CreateBinOp(Opcode, C1, C2, "invariant.op");
 
-  auto *NewBO = BinaryOperator::Create(
-      Opcode, LV, Inv, BO->getName() + ".reass", BO->getIterator());
+  auto *NewBO =
+      BinOpInRHS
+          ? BinaryOperator::Create(Opcode, Inv, LV, BO->getName() + ".reass",
+                                   BO->getIterator())
+          : BinaryOperator::Create(Opcode, LV, Inv, BO->getName() + ".reass",
+                                   BO->getIterator());
----------------
nikic wrote:

As the operations are commutative, there is no need to retain the original order.

https://github.com/llvm/llvm-project/pull/107072