[PATCH] D127499: [InstCombine] Don't push operation across loop phi

[Nikita Popov via Phabricator via llvm-commits]

nikic created this revision.
nikic added reviewers: spatel, syzaara.
Herald added a subscriber: hiraditya.
Herald added a project: All.
nikic requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

When pushing an operation across a phi node, we should avoid doing so across a loop backedge. This is generally non-profitable, because it does not reduce the number of times the operation is executed, and could lead to an infinite combine loop.

The code was already guarding against this, but using an insufficiently strong condition, which did not cover the case where the operation was originally outside the loop (in which case the transform moves the operation from outside the loop into the loop, which is particularly undesirable).


https://reviews.llvm.org/D127499

Files:
  llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
  llvm/test/Transforms/InstCombine/cast_phi.ll


Index: llvm/test/Transforms/InstCombine/cast_phi.ll
===================================================================
--- llvm/test/Transforms/InstCombine/cast_phi.ll
+++ llvm/test/Transforms/InstCombine/cast_phi.ll
@@ -320,15 +320,15 @@
 ; CHECK-NEXT:    br label [[LOOP:%.*]]
 ; CHECK:       loop:
 ; CHECK-NEXT:    [[IV:%.*]] = phi i32 [ 0, [[ENTRY:%.*]] ], [ [[IV_NEXT:%.*]], [[LOOP_LATCH:%.*]] ]
-; CHECK-NEXT:    [[PHI:%.*]] = phi i8 [ 1, [[ENTRY]] ], [ [[PHI_CAST:%.*]], [[LOOP_LATCH]] ]
+; CHECK-NEXT:    [[PHI:%.*]] = phi i32 [ 1, [[ENTRY]] ], [ [[IV_NEXT]], [[LOOP_LATCH]] ]
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp ult i32 [[IV]], 100
 ; CHECK-NEXT:    br i1 [[CMP]], label [[LOOP_LATCH]], label [[EXIT:%.*]]
 ; CHECK:       loop.latch:
 ; CHECK-NEXT:    [[IV_NEXT]] = add i32 [[IV]], 1
-; CHECK-NEXT:    [[PHI_CAST]] = trunc i32 [[IV_NEXT]] to i8
 ; CHECK-NEXT:    br label [[LOOP]]
 ; CHECK:       exit:
-; CHECK-NEXT:    ret i8 [[PHI]]
+; CHECK-NEXT:    [[TRUNC:%.*]] = trunc i32 [[PHI]] to i8
+; CHECK-NEXT:    ret i8 [[TRUNC]]
 ;
 entry:
   br label %loop
@@ -353,16 +353,16 @@
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    br label [[LOOP:%.*]]
 ; CHECK:       loop:
-; CHECK-NEXT:    [[IV:%.*]] = phi i32 [ 0, [[ENTRY:%.*]] ], [ [[PHI_CAST:%.*]], [[LOOP_LATCH:%.*]] ]
-; CHECK-NEXT:    [[CMP_NOT:%.*]] = icmp eq i32 [[IV]], [[END:%.*]]
+; CHECK-NEXT:    [[IV:%.*]] = phi i8 [ 0, [[ENTRY:%.*]] ], [ [[IV_NEXT:%.*]], [[LOOP_LATCH:%.*]] ]
+; CHECK-NEXT:    [[IV_EXT:%.*]] = zext i8 [[IV]] to i32
+; CHECK-NEXT:    [[CMP_NOT:%.*]] = icmp eq i32 [[IV_EXT]], [[END:%.*]]
 ; CHECK-NEXT:    br i1 [[CMP_NOT]], label [[EXIT:%.*]], label [[LOOP_LATCH]]
 ; CHECK:       loop.latch:
-; CHECK-NEXT:    [[STEP_EXT:%.*]] = zext i8 [[STEP:%.*]] to i32
-; CHECK-NEXT:    [[IV_NEXT:%.*]] = add nuw nsw i32 [[IV]], [[STEP_EXT]]
-; CHECK-NEXT:    [[PHI_CAST]] = and i32 [[IV_NEXT]], 255
+; CHECK-NEXT:    [[IV_NEXT]] = add i8 [[IV]], [[STEP:%.*]]
 ; CHECK-NEXT:    br label [[LOOP]]
 ; CHECK:       exit:
-; CHECK-NEXT:    ret i32 [[IV]]
+; CHECK-NEXT:    [[EXT:%.*]] = zext i8 [[IV]] to i32
+; CHECK-NEXT:    ret i32 [[EXT]]
 ;
 entry:
   br label %loop
Index: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
===================================================================
--- llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -1177,10 +1177,11 @@
       if (cast<Instruction>(InVal)->getParent() == NonConstBB)
         return nullptr;
 
-    // If the incoming non-constant value is in I's block, we will remove one
-    // instruction, but insert another equivalent one, leading to infinite
-    // instcombine.
-    if (isPotentiallyReachable(I.getParent(), NonConstBB, nullptr, &DT, LI))
+    // If the incoming non-constant value is reachable from the phis block,
+    // we'll push the operation across a loop backedge. This could result in
+    // an infinite combine loop, and is generally non-profitable (especially
+    // if the operation was originally outside the loop).
+    if (isPotentiallyReachable(PN->getParent(), NonConstBB, nullptr, &DT, LI))
       return nullptr;
   }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D127499.435907.patch
Type: text/x-patch
Size: 3171 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220610/ecbd7f41/attachment.bin>