[llvm] [SimplifyCFG] Treat umul + extract pattern as cheap single instruction. (PR #124933)

Gábor Spaits via llvm-commits llvm-commits at lists.llvm.org
Sat Feb 1 00:24:58 PST 2025


================
@@ -3329,20 +3331,29 @@ bool SimplifyCFGOpt::speculativelyExecuteBB(BranchInst *BI,
     else
       ++SpeculatedInstructions;
 
-    if (SpeculatedInstructions > 1)
-      return false;
-
     // Don't hoist the instruction if it's unsafe or expensive.
     if (!IsSafeCheapLoadStore &&
         !isSafeToSpeculativelyExecute(&I, BI, Options.AC) &&
         !(HoistCondStores && !SpeculatedStoreValue &&
           (SpeculatedStoreValue =
                isSafeToSpeculateStore(&I, BB, ThenBB, EndBB))))
       return false;
-    if (!IsSafeCheapLoadStore && !SpeculatedStoreValue &&
-        computeSpeculationCost(&I, TTI) >
+
+    if (match(&I,
+              m_ExtractValue<1>(m_OneUse(
+                  m_Intrinsic<Intrinsic::umul_with_overflow>(m_Value())))) &&
+        ThenBB->size() <= 3)
+      PatternFound = true;
+
+    BlockCostSoFar += computeSpeculationCost(&I, TTI);
+    if (!PatternFound && !IsSafeCheapLoadStore && !SpeculatedStoreValue &&
+        BlockCostSoFar >
             PHINodeFoldingThreshold * TargetTransformInfo::TCC_Basic)
       return false;
+    // If we don't find any pattern, that must be cheap, then only speculatively
+    // execute a single instruction (not counting the terminator).
+    if (!PatternFound && SpeculatedInstructions > 1)
+      return false;
----------------
spaits wrote:

I have removed the accumulating part of the PR. It was not needed. I have just added it because it looked more generic to me(https://github.com/llvm/llvm-project/pull/124933#discussion_r1934099948)(If later we would want to hoist more then one instruction but we would still care about the cost then we would need it, but not now). I know it is a generally bad practice to include code in a commit, that is not related to the main goal of the commit. Sorry for that.

If we don't have the`PartialInst` variable then the hoisting would not be done, because of the high cost.
On rv64, the value of ` PHINodeFoldingThreshold * TargetTransformInfo::TCC_Basic` is 2 and the cost of the pattern is 5. So one of the instructions cost must be 3 and that would be over the threshold.

I was also thinking of extending the pattern. Maybe we should check, if the condition is truly a zero checking for one of the operand of the overflow operator. This hoist is probably beneficial in that case only, because later on the redundant check can be dismissed.
What do you think?

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


More information about the llvm-commits mailing list