[llvm] [NFC][Sink] Change runtime checks to asserts (PR #137354)

via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 25 09:12:26 PDT 2025


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-transforms

Author: None (LU-JOHN)

<details>
<summary>Changes</summary>

Candidate block for sinking must be dominated by current location.  This is true based on how the candidate block was selected.  A runtime checks are not necessary and has been changed to an assertion.

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


1 Files Affected:

- (modified) llvm/lib/Transforms/Scalar/Sink.cpp (+9-7) 


``````````diff
diff --git a/llvm/lib/Transforms/Scalar/Sink.cpp b/llvm/lib/Transforms/Scalar/Sink.cpp
index 1a48a59c4189e..d6fd485aa5374 100644
--- a/llvm/lib/Transforms/Scalar/Sink.cpp
+++ b/llvm/lib/Transforms/Scalar/Sink.cpp
@@ -82,10 +82,11 @@ static bool IsAcceptableTarget(Instruction *Inst, BasicBlock *SuccToSinkTo,
         !Inst->hasMetadata(LLVMContext::MD_invariant_load))
       return false;
 
-    // We don't want to sink across a critical edge if we don't dominate the
-    // successor. We could be introducing calculations to new code paths.
-    if (!DT.dominates(Inst->getParent(), SuccToSinkTo))
-      return false;
+    // The current location of Inst dominates all uses, thus it must dominate
+    // SuccToSinkTo, which is on the IDom chain between the nearest common
+    // dominator to all uses and the current location.
+    assert(DT.dominates(Inst->getParent(), SuccToSinkTo) &&
+           "SuccToSinkTo must be dominated by current Inst location!");
 
     // Don't sink instructions into a loop.
     Loop *succ = LI.getLoopFor(SuccToSinkTo);
@@ -144,9 +145,10 @@ static bool SinkInstruction(Instruction *Inst,
       SuccToSinkTo = DT.findNearestCommonDominator(SuccToSinkTo, UseBlock);
     else
       SuccToSinkTo = UseBlock;
-    // The current basic block needs to dominate the candidate.
-    if (!DT.dominates(BB, SuccToSinkTo))
-      return false;
+    // The current basic block dominates all uses, thus it must dominate
+    // SuccToSinkTo, the nearest common dominator of all uses.
+    assert(DT.dominates(BB, SuccToSinkTo) &&
+           "SuccToSinkTo must be dominated by current basic block!");
   }
 
   if (SuccToSinkTo) {

``````````

</details>


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


More information about the llvm-commits mailing list