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

via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 25 09:11:50 PDT 2025


https://github.com/LU-JOHN created https://github.com/llvm/llvm-project/pull/137354

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.

>From d6914fb4e0ee3dca46b54482f2ae32949b2c587b Mon Sep 17 00:00:00 2001
From: John Lu <John.Lu at amd.com>
Date: Fri, 25 Apr 2025 11:09:15 -0500
Subject: [PATCH] Change dynamic checks to asserts

Signed-off-by: John Lu <John.Lu at amd.com>
---
 llvm/lib/Transforms/Scalar/Sink.cpp | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

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) {



More information about the llvm-commits mailing list