[ValueTracking][NFC] Early exit when enumerating guaranteed well-defined/non-poison operands.

[dtcxzyw]

According to the coverage result on my benchmark, llvm::mustTriggerUB returns true with an average of 35.0M/12.3M=2.85 matches. I think we can stop enumerating when one of the matches succeeds to avoid filling the temporary buffer NonPoisonOps.

This patch introduces two template functions handleGuaranteedWellDefinedOps/handleGuaranteedNonPoisonOps. They will pass well-defined/non-poison operands to inlinable callbacks Handle. If the callback returns true, stop processing and return true. Otherwise, return false.

Compile-time improvement: https://llvm-compile-time-tracker.com/compare.php?from=13acb3af5ad48e850cf37dcf02270ede3f267bd4&to=2b55f513c1b6dd2732cb79a25f3eaf6c5e4d6619&stat=instructions:u

stage1-O3	stage1-ReleaseThinLTO	stage1-ReleaseLTO-g	stage1-O0-g	stage2-O3	stage2-O0-g	stage2-clang
-0.03%	-0.04%	-0.06%	-0.03%	-0.05%	+0.03%	-0.02%

[llvmbot]

@llvm/pr-subscribers-llvm-analysis

Author: Yingwei Zheng (dtcxzyw)

Changes

According to the coverage result on my benchmark, llvm::mustTriggerUB returns true with an average of 35.0M/12.3M=2.85 matches. I think we can stop enumerating when one of the matches succeeds to avoid filling the temporary buffer NonPoisonOps.

This patch introduces two template functions foreachGuaranteedWellDefinedOps/foreachGuaranteedNonPoisonOps. They will pass well-defined/non-poison operands to inlinable callbacks Handle. If the callback returns true, stop iterating and return true. Otherwise, return false.

Compile-time improvement: https://llvm-compile-time-tracker.com/compare.php?from=13acb3af5ad48e850cf37dcf02270ede3f267bd4&to=2b55f513c1b6dd2732cb79a25f3eaf6c5e4d6619&stat=instructions:u

stage1-O3	stage1-ReleaseThinLTO	stage1-ReleaseLTO-g	stage1-O0-g	stage2-O3	stage2-O0-g	stage2-clang
-0.03%	-0.04%	-0.06%	-0.03%	-0.05%	+0.03%	-0.02%

---

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

1 Files Affected:

• (modified) llvm/lib/Analysis/ValueTracking.cpp (+60-36)

diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 04f317228b3ea7..41ab63dcacfc2b 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -7211,84 +7211,108 @@ bool llvm::propagatesPoison(const Use &PoisonOp) {
   }
 }
 
-void llvm::getGuaranteedWellDefinedOps(
-    const Instruction *I, SmallVectorImpl<const Value *> &Operands) {
+/// Enumerates all operands of \p I that are guaranteed to not be undef or
+/// poison. If the callback \p Handle returns true, stop iterating and return
+/// true. Otherwise, return false.
+template <typename CallableT>
+bool foreachGuaranteedWellDefinedOps(const Instruction *I,
+                                     const CallableT &Handle) {
   switch (I->getOpcode()) {
     case Instruction::Store:
-      Operands.push_back(cast<StoreInst>(I)->getPointerOperand());
+      if (Handle(cast<StoreInst>(I)->getPointerOperand()))
+        return true;
       break;
 
     case Instruction::Load:
-      Operands.push_back(cast<LoadInst>(I)->getPointerOperand());
+      if (Handle(cast<LoadInst>(I)->getPointerOperand()))
+        return true;
       break;
 
     // Since dereferenceable attribute imply noundef, atomic operations
     // also implicitly have noundef pointers too
     case Instruction::AtomicCmpXchg:
-      Operands.push_back(cast<AtomicCmpXchgInst>(I)->getPointerOperand());
+      if (Handle(cast<AtomicCmpXchgInst>(I)->getPointerOperand()))
+        return true;
       break;
 
     case Instruction::AtomicRMW:
-      Operands.push_back(cast<AtomicRMWInst>(I)->getPointerOperand());
+      if (Handle(cast<AtomicRMWInst>(I)->getPointerOperand()))
+        return true;
       break;
 
     case Instruction::Call:
     case Instruction::Invoke: {
       const CallBase *CB = cast<CallBase>(I);
-      if (CB->isIndirectCall())
-        Operands.push_back(CB->getCalledOperand());
-      for (unsigned i = 0; i < CB->arg_size(); ++i) {
-        if (CB->paramHasAttr(i, Attribute::NoUndef) ||
-            CB->paramHasAttr(i, Attribute::Dereferenceable) ||
-            CB->paramHasAttr(i, Attribute::DereferenceableOrNull))
-          Operands.push_back(CB->getArgOperand(i));
-      }
+      if (CB->isIndirectCall() && Handle(CB->getCalledOperand()))
+        return true;
+      for (unsigned i = 0; i < CB->arg_size(); ++i)
+        if ((CB->paramHasAttr(i, Attribute::NoUndef) ||
+             CB->paramHasAttr(i, Attribute::Dereferenceable) ||
+             CB->paramHasAttr(i, Attribute::DereferenceableOrNull)) &&
+            Handle(CB->getArgOperand(i)))
+          return true;
       break;
     }
     case Instruction::Ret:
-      if (I->getFunction()->hasRetAttribute(Attribute::NoUndef))
-        Operands.push_back(I->getOperand(0));
+      if (I->getFunction()->hasRetAttribute(Attribute::NoUndef) &&
+          Handle(I->getOperand(0)))
+        return true;
       break;
     case Instruction::Switch:
-      Operands.push_back(cast<SwitchInst>(I)->getCondition());
+      if (Handle(cast<SwitchInst>(I)->getCondition()))
+        return true;
       break;
     case Instruction::Br: {
       auto *BR = cast<BranchInst>(I);
-      if (BR->isConditional())
-        Operands.push_back(BR->getCondition());
+      if (BR->isConditional() && Handle(BR->getCondition()))
+        return true;
       break;
     }
     default:
       break;
   }
+
+  return false;
 }
 
-void llvm::getGuaranteedNonPoisonOps(const Instruction *I,
-                                     SmallVectorImpl<const Value *> &Operands) {
-  getGuaranteedWellDefinedOps(I, Operands);
+void llvm::getGuaranteedWellDefinedOps(
+    const Instruction *I, SmallVectorImpl<const Value *> &Operands) {
+  foreachGuaranteedWellDefinedOps(I, [&](const Value *V) {
+    Operands.push_back(V);
+    return false;
+  });
+}
+
+/// Enumerates all operands of \p I that are guaranteed to not be poison.
+template <typename CallableT>
+bool foreachGuaranteedNonPoisonOps(const Instruction *I,
+                                   const CallableT &Handle) {
+  if (foreachGuaranteedWellDefinedOps(I, Handle))
+    return true;
   switch (I->getOpcode()) {
   // Divisors of these operations are allowed to be partially undef.
   case Instruction::UDiv:
   case Instruction::SDiv:
   case Instruction::URem:
   case Instruction::SRem:
-    Operands.push_back(I->getOperand(1));
-    break;
+    return Handle(I->getOperand(1));
   default:
-    break;
+    return false;
   }
 }
 
+void llvm::getGuaranteedNonPoisonOps(const Instruction *I,
+                                     SmallVectorImpl<const Value *> &Operands) {
+  foreachGuaranteedNonPoisonOps(I, [&](const Value *V) {
+    Operands.push_back(V);
+    return false;
+  });
+}
+
 bool llvm::mustTriggerUB(const Instruction *I,
                          const SmallPtrSetImpl<const Value *> &KnownPoison) {
-  SmallVector<const Value *, 4> NonPoisonOps;
-  getGuaranteedNonPoisonOps(I, NonPoisonOps);
-
-  for (const auto *V : NonPoisonOps)
-    if (KnownPoison.count(V))
-      return true;
-
-  return false;
+  return foreachGuaranteedNonPoisonOps(
+      I, [&](const Value *V) { return KnownPoison.count(V); });
 }
 
 static bool programUndefinedIfUndefOrPoison(const Value *V,
@@ -7331,9 +7355,9 @@ static bool programUndefinedIfUndefOrPoison(const Value *V,
       if (--ScanLimit == 0)
         break;
 
-      SmallVector<const Value *, 4> WellDefinedOps;
-      getGuaranteedWellDefinedOps(&I, WellDefinedOps);
-      if (is_contained(WellDefinedOps, V))
+      if (foreachGuaranteedWellDefinedOps(&I, [V](const Value *WellDefinedOp) {
+            return WellDefinedOp == V;
+          }))
         return true;
 
       if (!isGuaranteedToTransferExecutionToSuccessor(&I))

[goldsteinn]

LGTM.

[nikic]

This looks fine, though I wonder whether the short-circuiting really plays any significant role here, vs just using the callback based approach that avoids populating a set. Not having to check the callback return value would make this a bit simpler...