Skip to content

Conversation

@RKSimon
Copy link
Collaborator

@RKSimon RKSimon commented Aug 18, 2025

isOperationLegal is much cheaper than value tracking

…mputeKnownBits/ComputeNumSignBits calls. NFC.
@RKSimon RKSimon requested a review from woruyu August 18, 2025 09:38
@llvmbot llvmbot added the llvm:SelectionDAG SelectionDAGISel as well label Aug 18, 2025
@llvmbot
Copy link
Member

llvmbot commented Aug 18, 2025

@llvm/pr-subscribers-llvm-selectiondag

Author: Simon Pilgrim (RKSimon)

Changes

isOperationLegal is much cheaper than value tracking


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

1 Files Affected:

  • (modified) llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (+4-3)
diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 6eb8468e2573e..785245b2d9e74 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -16329,8 +16329,9 @@ SDValue DAGCombiner::visitTRUNCATE(SDNode *N) {
     break;
   case ISD::ABDU:
   case ISD::ABDS:
-    // (trunc (abdu/abds a, b)) → (abdu/abds (trunc a), (trunc b))
-    if (!LegalOperations || N0.hasOneUse()) {
+    // (trunc (abdu/abds a, b)) -> (abdu/abds (trunc a), (trunc b))
+    if ((!LegalOperations || N0.hasOneUse()) &&
+        TLI.isOperationLegal(N0.getOpcode(), VT)) {
       EVT SrcVT = N0.getValueType();
       EVT TruncVT = VT;
       unsigned SrcBits = SrcVT.getScalarSizeInBits();
@@ -16352,7 +16353,7 @@ SDValue DAGCombiner::visitTRUNCATE(SDNode *N) {
         CanFold = SignBitsA > NeededBits && SignBitsB > NeededBits;
       }
 
-      if (CanFold && TLI.isOperationLegal(N0.getOpcode(), VT)) {
+      if (CanFold) {
         SDValue NewA = DAG.getNode(ISD::TRUNCATE, DL, TruncVT, A);
         SDValue NewB = DAG.getNode(ISD::TRUNCATE, DL, TruncVT, B);
         return DAG.getNode(N0.getOpcode(), DL, TruncVT, NewA, NewB);

Copy link
Member

@woruyu woruyu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, the earlier legality check looks reasonable.

@RKSimon RKSimon merged commit 681ecae into llvm:main Aug 18, 2025
11 checks passed
@RKSimon RKSimon deleted the dag-trunc-abg-legalop branch August 18, 2025 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

llvm:SelectionDAG SelectionDAGISel as well

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants