[llvm] [IR] Return bool from replaceUsesWithIf() (PR #184107)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-ir

Author: Nikita Popov (nikic)

<details>
<summary>Changes</summary>

We regularly want to know whether replaceUsesWithIf() actually replaced any uses for the purpose of change tracking. It's possible to do this by maintaining a Changed flag inside the callback, but it's cleaner if the method directly returns whether it modified anything.

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


4 Files Affected:

- (modified) llvm/include/llvm/IR/Value.h (+2-1) 
- (modified) llvm/lib/IR/Value.cpp (+6-1) 
- (modified) llvm/lib/Transforms/InstCombine/InstructionCombining.cpp (+5-12) 
- (modified) llvm/lib/Transforms/Scalar/ConstraintElimination.cpp (+2-6) 


``````````diff
diff --git a/llvm/include/llvm/IR/Value.h b/llvm/include/llvm/IR/Value.h
index a22c324189b9a..f50842f0a03d5 100644
--- a/llvm/include/llvm/IR/Value.h
+++ b/llvm/include/llvm/IR/Value.h
@@ -310,7 +310,8 @@ class Value {
   /// to "V" if the callback ShouldReplace returns true for the given Use.
   /// Unlike replaceAllUsesWith() this function does not support basic block
   /// values.
-  LLVM_ABI void
+  /// Returns whether any uses have been replaced.
+  LLVM_ABI bool
   replaceUsesWithIf(Value *New, llvm::function_ref<bool(Use &U)> ShouldReplace);
 
   /// replaceUsesOutsideBlock - Go through the uses list for this definition and
diff --git a/llvm/lib/IR/Value.cpp b/llvm/lib/IR/Value.cpp
index 2231ffba5e314..eba6810eb684d 100644
--- a/llvm/lib/IR/Value.cpp
+++ b/llvm/lib/IR/Value.cpp
@@ -558,7 +558,7 @@ void Value::replaceNonMetadataUsesWith(Value *New) {
   doRAUW(New, ReplaceMetadataUses::No);
 }
 
-void Value::replaceUsesWithIf(Value *New,
+bool Value::replaceUsesWithIf(Value *New,
                               llvm::function_ref<bool(Use &U)> ShouldReplace) {
   assert(New && "Value::replaceUsesWithIf(<null>) is invalid!");
   assert(New->getType() == getType() &&
@@ -567,9 +567,12 @@ void Value::replaceUsesWithIf(Value *New,
   SmallVector<TrackingVH<Constant>, 8> Consts;
   SmallPtrSet<Constant *, 8> Visited;
 
+  bool Changed = false;
   for (Use &U : llvm::make_early_inc_range(uses())) {
     if (!ShouldReplace(U))
       continue;
+    Changed = true;
+
     // Must handle Constants specially, we cannot call replaceUsesOfWith on a
     // constant because they are uniqued.
     if (auto *C = dyn_cast<Constant>(U.getUser())) {
@@ -587,6 +590,8 @@ void Value::replaceUsesWithIf(Value *New,
     //        not just the one passed to ShouldReplace
     Consts.pop_back_val()->handleOperandChange(this, New);
   }
+
+  return Changed;
 }
 
 /// Replace debug record uses of MetadataAsValue(ValueAsMetadata(V)) outside BB
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 0ca5da1bbf251..e5c750fc6b130 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -3516,13 +3516,9 @@ Instruction *InstCombinerImpl::visitGetElementPtrInst(GetElementPtrInst &GEP) {
             GEPType == Y->getType()) {
           bool HasNonAddressBits =
               DL.getAddressSizeInBits(AS) != DL.getPointerSizeInBits(AS);
-          bool Changed = false;
-          GEP.replaceUsesWithIf(Y, [&](Use &U) {
-            bool ShouldReplace =
-                isa<PtrToAddrInst, ICmpInst>(U.getUser()) ||
-                (!HasNonAddressBits && isa<PtrToIntInst>(U.getUser()));
-            Changed |= ShouldReplace;
-            return ShouldReplace;
+          bool Changed = GEP.replaceUsesWithIf(Y, [&](Use &U) {
+            return isa<PtrToAddrInst, ICmpInst>(U.getUser()) ||
+                   (!HasNonAddressBits && isa<PtrToIntInst>(U.getUser()));
           });
           return Changed ? &GEP : nullptr;
         }
@@ -5358,11 +5354,8 @@ bool InstCombinerImpl::freezeOtherUses(FreezeInst &FI) {
     Changed = true;
   }
 
-  Op->replaceUsesWithIf(&FI, [&](Use &U) -> bool {
-    bool Dominates = DT.dominates(&FI, U);
-    Changed |= Dominates;
-    return Dominates;
-  });
+  Changed |= Op->replaceUsesWithIf(
+      &FI, [&](Use &U) -> bool { return DT.dominates(&FI, U); });
 
   return Changed;
 }
diff --git a/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp b/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
index fc5ee2359a76f..6d33eae6a15d2 100644
--- a/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
+++ b/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
@@ -1512,9 +1512,7 @@ static bool checkAndReplaceCondition(
     generateReproducer(Cmp, ReproducerModule, ReproducerCondStack, Info, DT);
     Constant *ConstantC = ConstantInt::getBool(
         CmpInst::makeCmpResultType(Cmp->getType()), IsTrue);
-    bool Changed = false;
-    Cmp->replaceUsesWithIf(ConstantC, [&DT, NumIn, NumOut, ContextInst,
-                                       &Changed](Use &U) {
+    bool Changed = Cmp->replaceUsesWithIf(ConstantC, [&](Use &U) {
       auto *UserI = getContextInstForUse(U);
       auto *DTN = DT.getNode(UserI->getParent());
       if (!DTN || DTN->getDFSNumIn() < NumIn || DTN->getDFSNumOut() > NumOut)
@@ -1526,9 +1524,7 @@ static bool checkAndReplaceCondition(
       // Conditions in an assume trivially simplify to true. Skip uses
       // in assume calls to not destroy the available information.
       auto *II = dyn_cast<IntrinsicInst>(U.getUser());
-      bool ShouldReplace = !II || II->getIntrinsicID() != Intrinsic::assume;
-      Changed |= ShouldReplace;
-      return ShouldReplace;
+      return !II || II->getIntrinsicID() != Intrinsic::assume;
     });
     NumCondsRemoved++;
 

``````````

</details>


https://github.com/llvm/llvm-project/pull/184107