[PATCH] D151807: [InstCombine] Revisit user of newly one-use instructions

[Nikita Popov via Phabricator via llvm-commits]

nikic created this revision.
nikic added reviewers: goldstein.w.n, RKSimon, fhahn.
Herald added subscribers: StephenFan, hiraditya.
Herald added a project: All.
nikic requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Many folds in InstCombine are limited to one-use instructions. For that reason, if the use-count of an instruction drops to one, it makes sense to revisit that one user. This is one of the most common reasons why InstCombine fails to finish in a single iteration.

Doing this revisit actually slightly improves compile-time (http://llvm-compile-time-tracker.com/compare.php?from=97f0e7b06e6b76fd85fb81b8c12eba2255ff1742&to=fc740dee13f42a948b378d731e666d0a80d16061&stat=instructions:u), because we save an extra InstCombine iteration in enough cases to make a visible difference.

This is conceptually NFC, but not NFC in practice, because differences in worklist order can result in slightly different folding behavior.


https://reviews.llvm.org/D151807

Files:
  llvm/include/llvm/Transforms/Utils/InstructionWorklist.h
  llvm/lib/Transforms/InstCombine/InstCombineInternal.h
  llvm/test/Other/print-debug-counter.ll
  llvm/test/Transforms/InstCombine/or-shifted-masks.ll


Index: llvm/test/Transforms/InstCombine/or-shifted-masks.ll
===================================================================
--- llvm/test/Transforms/InstCombine/or-shifted-masks.ll
+++ llvm/test/Transforms/InstCombine/or-shifted-masks.ll
@@ -54,10 +54,13 @@
 
 define i32 @multiuse1(i32 %x) {
 ; CHECK-LABEL: @multiuse1(
-; CHECK-NEXT:    [[I21:%.*]] = shl i32 [[X:%.*]], 6
+; CHECK-NEXT:    [[I:%.*]] = lshr i32 [[X:%.*]], 1
+; CHECK-NEXT:    [[I3:%.*]] = and i32 [[I]], 1
+; CHECK-NEXT:    [[I1:%.*]] = lshr i32 [[X]], 1
+; CHECK-NEXT:    [[I5:%.*]] = and i32 [[I1]], 2
+; CHECK-NEXT:    [[I21:%.*]] = shl i32 [[X]], 6
 ; CHECK-NEXT:    [[I6:%.*]] = and i32 [[I21]], 384
-; CHECK-NEXT:    [[I32:%.*]] = lshr i32 [[X]], 1
-; CHECK-NEXT:    [[I7:%.*]] = and i32 [[I32]], 3
+; CHECK-NEXT:    [[I7:%.*]] = or i32 [[I3]], [[I5]]
 ; CHECK-NEXT:    [[I8:%.*]] = or i32 [[I7]], [[I6]]
 ; CHECK-NEXT:    ret i32 [[I8]]
 ;
@@ -77,13 +80,15 @@
 ; CHECK-LABEL: @multiuse2(
 ; CHECK-NEXT:    [[I:%.*]] = shl i32 [[X:%.*]], 1
 ; CHECK-NEXT:    [[I2:%.*]] = and i32 [[I]], 12
-; CHECK-NEXT:    [[I6:%.*]] = shl i32 [[X]], 8
-; CHECK-NEXT:    [[I7:%.*]] = and i32 [[I6]], 24576
+; CHECK-NEXT:    [[I3:%.*]] = shl i32 [[X]], 1
+; CHECK-NEXT:    [[I5:%.*]] = and i32 [[I3]], 48
+; CHECK-NEXT:    [[I6:%.*]] = and i32 [[X]], 96
+; CHECK-NEXT:    [[I7:%.*]] = shl nuw nsw i32 [[I6]], 8
+; CHECK-NEXT:    [[I8:%.*]] = shl nuw nsw i32 [[I6]], 1
 ; CHECK-NEXT:    [[I14:%.*]] = shl i32 [[X]], 8
 ; CHECK-NEXT:    [[I9:%.*]] = and i32 [[I14]], 7680
 ; CHECK-NEXT:    [[I10:%.*]] = or i32 [[I7]], [[I9]]
-; CHECK-NEXT:    [[I85:%.*]] = shl i32 [[X]], 1
-; CHECK-NEXT:    [[I11:%.*]] = and i32 [[I85]], 240
+; CHECK-NEXT:    [[I11:%.*]] = or i32 [[I8]], [[I5]]
 ; CHECK-NEXT:    [[I12:%.*]] = or i32 [[I2]], [[I11]]
 ; CHECK-NEXT:    [[I13:%.*]] = or i32 [[I10]], [[I12]]
 ; CHECK-NEXT:    ret i32 [[I13]]
Index: llvm/test/Other/print-debug-counter.ll
===================================================================
--- llvm/test/Other/print-debug-counter.ll
+++ llvm/test/Other/print-debug-counter.ll
@@ -8,7 +8,7 @@
 ; CHECK:       early-cse
 ; CHECK-SAME:  {4,1,1}
 ; CHECK:       instcombine-visit
-; CHECK-SAME:  {12,0,-1}
+; CHECK-SAME:  {13,0,-1}
 ; CHECK:       newgvn-vn
 ; CHECK-SAME:  {9,1,2}
 define i32 @f1(i32 %a, i32 %b) {
Index: llvm/lib/Transforms/InstCombine/InstCombineInternal.h
===================================================================
--- llvm/lib/Transforms/InstCombine/InstCombineInternal.h
+++ llvm/lib/Transforms/InstCombine/InstCombineInternal.h
@@ -411,12 +411,11 @@
 
     // Make sure that we reprocess all operands now that we reduced their
     // use counts.
-    for (Use &Operand : I.operands())
-      if (auto *Inst = dyn_cast<Instruction>(Operand))
-        Worklist.add(Inst);
-
+    SmallVector<Value *> Ops(I.operands());
     Worklist.remove(&I);
     I.eraseFromParent();
+    for (Value *Op : Ops)
+      Worklist.handleUseCountDecrement(Op);
     MadeIRChange = true;
     return nullptr; // Don't do anything with FI
   }
Index: llvm/include/llvm/Transforms/Utils/InstructionWorklist.h
===================================================================
--- llvm/include/llvm/Transforms/Utils/InstructionWorklist.h
+++ llvm/include/llvm/Transforms/Utils/InstructionWorklist.h
@@ -108,6 +108,17 @@
       push(cast<Instruction>(U));
   }
 
+  /// Should be called *after* decrementing the use-count on V.
+  void handleUseCountDecrement(Value *V) {
+    if (auto *I = dyn_cast<Instruction>(V)) {
+      add(I);
+      // Many folds have one-use limitations. If there's only one use left,
+      // revisit that use.
+      if (I->hasOneUse())
+        add(cast<Instruction>(*I->user_begin()));
+    }
+  }
+
   /// Check that the worklist is empty and nuke the backing store for the map.
   void zap() {
     assert(WorklistMap.empty() && "Worklist empty, but map not?");


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D151807.527040.patch
Type: text/x-patch
Size: 3911 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230531/db25fd5d/attachment.bin>