Skip to content

Conversation

@artagnon
Copy link
Contributor

The Worklist used by computeMinimumValueSizes is wastefully a Value-vector, when the top of the loop attempts to cast the popped value to an Instruction anyway. Improve the algorithm by cutting this wasteful work, refining the type of Worklist, Visited, and Roots from Value-vectors to Instruction-vectors.

The Worklist used by computeMinimumValueSizes is wastefully a
Value-vector, when the top of the loop attempts to cast the popped value
to an Instruction anyway. Improve the algorithm by cutting this wasteful
work, refining the type of Worklist, Visited, and Roots from
Value-vectors to Instruction-vectors.
@artagnon artagnon requested review from RKSimon, fhahn and nikic April 28, 2025 19:11
@llvmbot llvmbot added the llvm:analysis Includes value tracking, cost tables and constant folding label Apr 28, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 28, 2025

@llvm/pr-subscribers-llvm-analysis

Author: Ramkumar Ramachandra (artagnon)

Changes

The Worklist used by computeMinimumValueSizes is wastefully a Value-vector, when the top of the loop attempts to cast the popped value to an Instruction anyway. Improve the algorithm by cutting this wasteful work, refining the type of Worklist, Visited, and Roots from Value-vectors to Instruction-vectors.


Full diff: https://github.com/llvm/llvm-project/pull/137692.diff

1 Files Affected:

  • (modified) llvm/lib/Analysis/VectorUtils.cpp (+10-14)
diff --git a/llvm/lib/Analysis/VectorUtils.cpp b/llvm/lib/Analysis/VectorUtils.cpp
index 6448c372f5d5d..02f93d3fd1b4c 100644
--- a/llvm/lib/Analysis/VectorUtils.cpp
+++ b/llvm/lib/Analysis/VectorUtils.cpp
@@ -750,9 +750,9 @@ llvm::computeMinimumValueSizes(ArrayRef<BasicBlock *> Blocks, DemandedBits &DB,
   // to ensure no extra casts would need to be inserted, so every DAG
   // of connected values must have the same minimum bitwidth.
   EquivalenceClasses<Value *> ECs;
-  SmallVector<Value *, 16> Worklist;
-  SmallPtrSet<Value *, 4> Roots;
-  SmallPtrSet<Value *, 16> Visited;
+  SmallVector<Instruction *, 16> Worklist;
+  SmallPtrSet<Instruction *, 4> Roots;
+  SmallPtrSet<Instruction *, 16> Visited;
   DenseMap<Value *, uint64_t> DBits;
   SmallPtrSet<Instruction *, 4> InstructionSet;
   MapVector<Instruction *, uint64_t> MinBWs;
@@ -786,17 +786,12 @@ llvm::computeMinimumValueSizes(ArrayRef<BasicBlock *> Blocks, DemandedBits &DB,
 
   // Now proceed breadth-first, unioning values together.
   while (!Worklist.empty()) {
-    Value *Val = Worklist.pop_back_val();
-    Value *Leader = ECs.getOrInsertLeaderValue(Val);
+    Instruction *I = Worklist.pop_back_val();
+    Value *Leader = ECs.getOrInsertLeaderValue(I);
 
-    if (!Visited.insert(Val).second)
+    if (!Visited.insert(I).second)
       continue;
 
-    // Non-instructions terminate a chain successfully.
-    if (!isa<Instruction>(Val))
-      continue;
-    Instruction *I = cast<Instruction>(Val);
-
     // If we encounter a type that is larger than 64 bits, we can't represent
     // it so bail out.
     if (DB.getDemandedBits(I).getBitWidth() > 64)
@@ -831,9 +826,10 @@ llvm::computeMinimumValueSizes(ArrayRef<BasicBlock *> Blocks, DemandedBits &DB,
       // All bits demanded, no point continuing.
       continue;
 
-    for (Value *O : cast<User>(I)->operands()) {
+    for (Value *O : I->operands()) {
       ECs.unionSets(Leader, O);
-      Worklist.push_back(O);
+      if (auto *OI = dyn_cast<Instruction>(O))
+        Worklist.push_back(OI);
     }
   }
 
@@ -874,7 +870,7 @@ llvm::computeMinimumValueSizes(ArrayRef<BasicBlock *> Blocks, DemandedBits &DB,
       if (!MI)
         continue;
       Type *Ty = M->getType();
-      if (Roots.count(M))
+      if (Roots.count(MI))
         Ty = MI->getOperand(0)->getType();
 
       if (MinBW >= Ty->getScalarSizeInBits())

Copy link
Contributor

@fhahn fhahn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks

@artagnon artagnon merged commit f6b6fb8 into llvm:main Apr 29, 2025
13 checks passed
@artagnon artagnon deleted the vu-minvalsz-improve-nfc branch April 29, 2025 09:36
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
The Worklist used by computeMinimumValueSizes is wastefully a
Value-vector, when the top of the loop attempts to cast the popped value
to an Instruction anyway. Improve the algorithm by cutting this wasteful
work, refining the type of Worklist, Visited, and Roots from
Value-vectors to Instruction-vectors.
GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this pull request May 7, 2025
The Worklist used by computeMinimumValueSizes is wastefully a
Value-vector, when the top of the loop attempts to cast the popped value
to an Instruction anyway. Improve the algorithm by cutting this wasteful
work, refining the type of Worklist, Visited, and Roots from
Value-vectors to Instruction-vectors.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

llvm:analysis Includes value tracking, cost tables and constant folding

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants