[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