LICM: generalize further strengthening check

Author: artagnon

llvm/lib/Transforms/Scalar/LICM.cpp

@@ -2803,13 +2803,14 @@ static bool hoistMulAddAssociation(Instruction &I, Loop &L,
 
 /// Reassociate associative binary expressions of the form
 ///
-/// 1. "(LV op C1) op C2" ==> "LV op (C1 op C2)" if op is an associative BinOp
-/// 2. "(C1 op LV) op C2" ==> "LV op (C1 op C2)" if op is a commutative BinOp
-/// 3. "C2 op (C1 op LV)" ==> "(C2 op C1) op LV" if op an associative BinOp
-/// 4. "C2 op (LV op C1)" ==> "(C2 op C1) op LV" if op is a commutative BinOp
+/// 1. "(LV op C1) op C2" ==> "LV op (C1 op C2)"
+/// 2. "(C1 op LV) op C2" ==> "LV op (C1 op C2)"
+/// 3. "C2 op (C1 op LV)" ==> "LV op (C1 op C2)"
+/// 4. "C2 op (LV op C1)" ==> "LV op (C1 op C2)"
 ///
-/// where LV is a loop variant, and C1 and C2 are loop invariants that we want
-/// to hoist.
+/// where op is an associative BinOp, LV is a loop variant, and C1 and C2 are
+/// loop invariants that we want to hoist, noting that associativity implies
+/// commutativity.
 static bool hoistBOAssociation(Instruction &I, Loop &L,
                                ICFLoopSafetyInfo &SafetyInfo,
                                MemorySSAUpdater &MSSAU, AssumptionCache *AC,
@@ -2833,35 +2834,21 @@ static bool hoistBOAssociation(Instruction &I, Loop &L,
   Value *C1 = BO0->getOperand(1);
   Value *C2 = BO->getOperand(!BinOpInRHS);
 
-  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");
+  assert(BO->isCommutative() && BO0->isCommutative() &&
+         "Associativity implies commutativity");
+  if (L.isLoopInvariant(LV) && !L.isLoopInvariant(C1))
     std::swap(LV, C1);
-  } else {
+  if (L.isLoopInvariant(LV) || !L.isLoopInvariant(C1) || !L.isLoopInvariant(C2))
     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 =
-      BinOpInRHS
-          ? BinaryOperator::Create(Opcode, Inv, LV, BO->getName() + ".reass",
-                                   BO->getIterator())
-          : BinaryOperator::Create(Opcode, LV, Inv, BO->getName() + ".reass",
-                                   BO->getIterator());
+  auto *NewBO = BinaryOperator::Create(
+      Opcode, LV, Inv, BO->getName() + ".reass", BO->getIterator());
 
   // Copy NUW for ADDs if both instructions have it.
   if (Opcode == Instruction::Add && BO->hasNoUnsignedWrap() &&

llvm/test/Transforms/LICM/hoist-binop.ll

@@ -97,13 +97,13 @@ loop:
 define void @add_nuw_comm2(i64 %c1, i64 %c2) {
 ; CHECK-LABEL: @add_nuw_comm2(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[INVARIANT_OP:%.*]] = add nuw i64 [[C2:%.*]], [[C1:%.*]]
+; CHECK-NEXT:    [[INVARIANT_OP:%.*]] = add nuw i64 [[C1:%.*]], [[C2:%.*]]
 ; CHECK-NEXT:    br label [[LOOP:%.*]]
 ; CHECK:       loop:
 ; CHECK-NEXT:    [[INDEX:%.*]] = phi i64 [ 0, [[ENTRY:%.*]] ], [ [[INDEX_NEXT_REASS:%.*]], [[LOOP]] ]
 ; CHECK-NEXT:    [[STEP_ADD:%.*]] = add nuw i64 [[INDEX]], [[C1]]
 ; CHECK-NEXT:    call void @use(i64 [[STEP_ADD]])
-; CHECK-NEXT:    [[INDEX_NEXT_REASS]] = add nuw i64 [[INVARIANT_OP]], [[INDEX]]
+; CHECK-NEXT:    [[INDEX_NEXT_REASS]] = add nuw i64 [[INDEX]], [[INVARIANT_OP]]
 ; CHECK-NEXT:    br label [[LOOP]]
 ;
 entry:
@@ -122,13 +122,13 @@ loop:
 define void @add_nuw_comm3(i64 %c1, i64 %c2) {
 ; CHECK-LABEL: @add_nuw_comm3(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[INVARIANT_OP:%.*]] = add nuw i64 [[C2:%.*]], [[C1:%.*]]
+; CHECK-NEXT:    [[INVARIANT_OP:%.*]] = add nuw i64 [[C1:%.*]], [[C2:%.*]]
 ; CHECK-NEXT:    br label [[LOOP:%.*]]
 ; CHECK:       loop:
 ; CHECK-NEXT:    [[INDEX:%.*]] = phi i64 [ 0, [[ENTRY:%.*]] ], [ [[INDEX_NEXT_REASS:%.*]], [[LOOP]] ]
 ; CHECK-NEXT:    [[STEP_ADD:%.*]] = add nuw i64 [[C1]], [[INDEX]]
 ; CHECK-NEXT:    call void @use(i64 [[STEP_ADD]])
-; CHECK-NEXT:    [[INDEX_NEXT_REASS]] = add nuw i64 [[INVARIANT_OP]], [[INDEX]]
+; CHECK-NEXT:    [[INDEX_NEXT_REASS]] = add nuw i64 [[INDEX]], [[INVARIANT_OP]]
 ; CHECK-NEXT:    br label [[LOOP]]
 ;
 entry:
@@ -196,13 +196,13 @@ loop:
 define void @mul_nuw_comm2(i64 %c1, i64 %c2) {
 ; CHECK-LABEL: @mul_nuw_comm2(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[INVARIANT_OP:%.*]] = mul i64 [[C2:%.*]], [[C1:%.*]]
+; CHECK-NEXT:    [[INVARIANT_OP:%.*]] = mul i64 [[C1:%.*]], [[C2:%.*]]
 ; CHECK-NEXT:    br label [[LOOP:%.*]]
 ; CHECK:       loop:
 ; CHECK-NEXT:    [[INDEX:%.*]] = phi i64 [ 0, [[ENTRY:%.*]] ], [ [[INDEX_NEXT_REASS:%.*]], [[LOOP]] ]
 ; CHECK-NEXT:    [[STEP_ADD:%.*]] = mul nuw i64 [[INDEX]], [[C1]]
 ; CHECK-NEXT:    call void @use(i64 [[STEP_ADD]])
-; CHECK-NEXT:    [[INDEX_NEXT_REASS]] = mul i64 [[INVARIANT_OP]], [[INDEX]]
+; CHECK-NEXT:    [[INDEX_NEXT_REASS]] = mul i64 [[INDEX]], [[INVARIANT_OP]]
 ; CHECK-NEXT:    br label [[LOOP]]
 ;
 entry:
@@ -221,13 +221,13 @@ loop:
 define void @mul_nuw_comm3(i64 %c1, i64 %c2) {
 ; CHECK-LABEL: @mul_nuw_comm3(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[INVARIANT_OP:%.*]] = mul i64 [[C2:%.*]], [[C1:%.*]]
+; CHECK-NEXT:    [[INVARIANT_OP:%.*]] = mul i64 [[C1:%.*]], [[C2:%.*]]
 ; CHECK-NEXT:    br label [[LOOP:%.*]]
 ; CHECK:       loop:
 ; CHECK-NEXT:    [[INDEX:%.*]] = phi i64 [ 0, [[ENTRY:%.*]] ], [ [[INDEX_NEXT_REASS:%.*]], [[LOOP]] ]
 ; CHECK-NEXT:    [[STEP_ADD:%.*]] = mul nuw i64 [[C1]], [[INDEX]]
 ; CHECK-NEXT:    call void @use(i64 [[STEP_ADD]])
-; CHECK-NEXT:    [[INDEX_NEXT_REASS]] = mul i64 [[INVARIANT_OP]], [[INDEX]]
+; CHECK-NEXT:    [[INDEX_NEXT_REASS]] = mul i64 [[INDEX]], [[INVARIANT_OP]]
 ; CHECK-NEXT:    br label [[LOOP]]
 ;
 entry: