[SelectionDAG] Move sign pattern check from AArch64 and ARM to general SelectionDAG #151736

AZero13 · 2025-08-01T17:21:57Z

This works on all cases much like the XOR case above it in SelectionDAG.

…l SelectionDAG

llvmbot · 2025-08-01T17:22:28Z

@llvm/pr-subscribers-llvm-selectiondag

@llvm/pr-subscribers-backend-aarch64

Author: AZero13 (AZero13)

Changes

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

4 Files Affected:

(modified) llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (+13)
(modified) llvm/lib/Target/AArch64/AArch64ISelLowering.cpp (-12)
(modified) llvm/lib/Target/ARM/ARMISelLowering.cpp (-12)
(modified) llvm/test/CodeGen/ARM/cmp-select-sign.ll (+16-16)

diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index a43020ee62281..0097e8e1aa1a3 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -28972,6 +28972,19 @@ SDValue DAGCombiner::SimplifySelectCC(const SDLoc &DL, SDValue N0, SDValue N1,
                        DAG.getSExtOrTrunc(CC == ISD::SETLT ? N3 : N2, DL, VT));
   }
 
+  // Check for sign pattern (SELECT_CC setgt, iN X, -1, 1, -1) and transform
+  // into (or (ashr X, BW-1), 1).
+  if (CC == ISD::SETGT && N1C && N2C && N3C && N1C->isAllOnes() &&
+      N2C->isOne() && N3C->isAllOnes() &&
+      !TLI.shouldAvoidTransformToShift(CmpOpVT,
+                                       CmpOpVT.getScalarSizeInBits() - 1)) {
+    SDValue ASR = DAG.getNode(
+        ISD::SRA, DL, CmpOpVT, N0,
+        DAG.getConstant(CmpOpVT.getScalarSizeInBits() - 1, DL, CmpOpVT));
+    return DAG.getNode(ISD::OR, DL, VT, DAG.getSExtOrTrunc(ASR, DL, VT),
+                       DAG.getConstant(1, DL, VT));
+  }
+
   if (SDValue S = PerformMinMaxFpToSatCombine(N0, N1, N2, N3, CC, DAG))
     return S;
   if (SDValue S = PerformUMinFpToSatCombine(N0, N1, N2, N3, CC, DAG))
diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
index 4f6e3ddd18def..d1aedae0b09e6 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -11360,18 +11360,6 @@ SDValue AArch64TargetLowering::LowerSELECT_CC(
     ConstantSDNode *CFVal = dyn_cast<ConstantSDNode>(FVal);
     ConstantSDNode *CTVal = dyn_cast<ConstantSDNode>(TVal);
     ConstantSDNode *RHSC = dyn_cast<ConstantSDNode>(RHS);
-    // Check for sign pattern (SELECT_CC setgt, iN lhs, -1, 1, -1) and transform
-    // into (OR (ASR lhs, N-1), 1), which requires less instructions for the
-    // supported types.
-    if (CC == ISD::SETGT && RHSC && RHSC->isAllOnes() && CTVal && CFVal &&
-        CTVal->isOne() && CFVal->isAllOnes() &&
-        LHS.getValueType() == TVal.getValueType()) {
-      EVT VT = LHS.getValueType();
-      SDValue Shift =
-          DAG.getNode(ISD::SRA, DL, VT, LHS,
-                      DAG.getConstant(VT.getSizeInBits() - 1, DL, VT));
-      return DAG.getNode(ISD::OR, DL, VT, Shift, DAG.getConstant(1, DL, VT));
-    }
 
     // Check for SMAX(lhs, 0) and SMIN(lhs, 0) patterns.
     // (SELECT_CC setgt, lhs, 0, lhs, 0) -> (BIC lhs, (SRA lhs, typesize-1))
diff --git a/llvm/lib/Target/ARM/ARMISelLowering.cpp b/llvm/lib/Target/ARM/ARMISelLowering.cpp
index bd4b75fd3c167..936625606e315 100644
--- a/llvm/lib/Target/ARM/ARMISelLowering.cpp
+++ b/llvm/lib/Target/ARM/ARMISelLowering.cpp
@@ -5521,18 +5521,6 @@ SDValue ARMTargetLowering::LowerSELECT_CC(SDValue Op, SelectionDAG &DAG) const {
   ConstantSDNode *CTVal = dyn_cast<ConstantSDNode>(TrueVal);
   ConstantSDNode *RHSC = dyn_cast<ConstantSDNode>(RHS);
   if (Op.getValueType().isInteger()) {
-    // Check for sign pattern (SELECT_CC setgt, iN lhs, -1, 1, -1) and transform
-    // into (OR (ASR lhs, N-1), 1), which requires less instructions for the
-    // supported types.
-    if (CC == ISD::SETGT && RHSC && RHSC->isAllOnes() && CTVal && CFVal &&
-        CTVal->isOne() && CFVal->isAllOnes() &&
-        LHS.getValueType() == TrueVal.getValueType()) {
-      EVT VT = LHS.getValueType();
-      SDValue Shift =
-          DAG.getNode(ISD::SRA, dl, VT, LHS,
-                      DAG.getConstant(VT.getSizeInBits() - 1, dl, VT));
-      return DAG.getNode(ISD::OR, dl, VT, Shift, DAG.getConstant(1, dl, VT));
-    }
 
     // Check for SMAX(lhs, 0) and SMIN(lhs, 0) patterns.
     // (SELECT_CC setgt, lhs, 0, lhs, 0) -> (BIC lhs, (SRA lhs, typesize-1))
diff --git a/llvm/test/CodeGen/ARM/cmp-select-sign.ll b/llvm/test/CodeGen/ARM/cmp-select-sign.ll
index 298a623ed1d87..61cdc3bdcd277 100644
--- a/llvm/test/CodeGen/ARM/cmp-select-sign.ll
+++ b/llvm/test/CodeGen/ARM/cmp-select-sign.ll
@@ -75,31 +75,31 @@ define i4 @sign_i4(i4 %a) {
 define i8 @sign_i8(i8 %a) {
 ; ARM-LABEL: sign_i8:
 ; ARM:       @ %bb.0:
-; ARM-NEXT:    lsl r0, r0, #24
+; ARM-NEXT:    sxtb r0, r0
 ; ARM-NEXT:    mov r1, #1
-; ARM-NEXT:    orr r0, r1, r0, asr #31
+; ARM-NEXT:    orr r0, r1, r0, asr #7
 ; ARM-NEXT:    bx lr
 ;
 ; THUMB-LABEL: sign_i8:
 ; THUMB:       @ %bb.0:
-; THUMB-NEXT:    lsls r0, r0, #24
-; THUMB-NEXT:    asrs r1, r0, #31
+; THUMB-NEXT:    sxtb r0, r0
+; THUMB-NEXT:    asrs r1, r0, #7
 ; THUMB-NEXT:    movs r0, #1
 ; THUMB-NEXT:    orrs r0, r1
 ; THUMB-NEXT:    bx lr
 ;
 ; THUMB2-LABEL: sign_i8:
 ; THUMB2:       @ %bb.0:
-; THUMB2-NEXT:    lsls r0, r0, #24
+; THUMB2-NEXT:    sxtb r0, r0
 ; THUMB2-NEXT:    movs r1, #1
-; THUMB2-NEXT:    orr.w r0, r1, r0, asr #31
+; THUMB2-NEXT:    orr.w r0, r1, r0, asr #7
 ; THUMB2-NEXT:    bx lr
 ;
 ; THUMBV8-LABEL: sign_i8:
 ; THUMBV8:       @ %bb.0:
-; THUMBV8-NEXT:    lsls r0, r0, #24
+; THUMBV8-NEXT:    sxtb r0, r0
 ; THUMBV8-NEXT:    movs r1, #1
-; THUMBV8-NEXT:    orr.w r0, r1, r0, asr #31
+; THUMBV8-NEXT:    orr.w r0, r1, r0, asr #7
 ; THUMBV8-NEXT:    bx lr
   %c = icmp sgt i8 %a, -1
   %res = select i1 %c, i8 1, i8 -1
@@ -109,31 +109,31 @@ define i8 @sign_i8(i8 %a) {
 define i16 @sign_i16(i16 %a) {
 ; ARM-LABEL: sign_i16:
 ; ARM:       @ %bb.0:
-; ARM-NEXT:    lsl r0, r0, #16
+; ARM-NEXT:    sxth r0, r0
 ; ARM-NEXT:    mov r1, #1
-; ARM-NEXT:    orr r0, r1, r0, asr #31
+; ARM-NEXT:    orr r0, r1, r0, asr #15
 ; ARM-NEXT:    bx lr
 ;
 ; THUMB-LABEL: sign_i16:
 ; THUMB:       @ %bb.0:
-; THUMB-NEXT:    lsls r0, r0, #16
-; THUMB-NEXT:    asrs r1, r0, #31
+; THUMB-NEXT:    sxth r0, r0
+; THUMB-NEXT:    asrs r1, r0, #15
 ; THUMB-NEXT:    movs r0, #1
 ; THUMB-NEXT:    orrs r0, r1
 ; THUMB-NEXT:    bx lr
 ;
 ; THUMB2-LABEL: sign_i16:
 ; THUMB2:       @ %bb.0:
-; THUMB2-NEXT:    lsls r0, r0, #16
+; THUMB2-NEXT:    sxth r0, r0
 ; THUMB2-NEXT:    movs r1, #1
-; THUMB2-NEXT:    orr.w r0, r1, r0, asr #31
+; THUMB2-NEXT:    orr.w r0, r1, r0, asr #15
 ; THUMB2-NEXT:    bx lr
 ;
 ; THUMBV8-LABEL: sign_i16:
 ; THUMBV8:       @ %bb.0:
-; THUMBV8-NEXT:    lsls r0, r0, #16
+; THUMBV8-NEXT:    sxth r0, r0
 ; THUMBV8-NEXT:    movs r1, #1
-; THUMBV8-NEXT:    orr.w r0, r1, r0, asr #31
+; THUMBV8-NEXT:    orr.w r0, r1, r0, asr #15
 ; THUMBV8-NEXT:    bx lr
   %c = icmp sgt i16 %a, -1
   %res = select i1 %c, i16 1, i16 -1

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

github-actions · 2025-08-01T18:08:39Z

✅ With the latest revision this PR passed the C/C++ code formatter.

AZero13 · 2025-08-01T18:16:51Z

@topperc What about now?

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

AZero13 · 2025-08-01T19:52:38Z

@topperc I did everything

topperc

LGTM

AZero13 · 2025-08-01T20:05:13Z

Thank you!

AZero13 · 2025-08-01T20:05:32Z

I do not have merge permissions. @topperc

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

AZero13 · 2025-08-01T20:20:57Z

@topperc fixed

llvm-ci · 2025-08-01T22:15:57Z

LLVM Buildbot has detected a new failure on builder arc-builder running on arc-worker while building llvm at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/3/builds/19945

Here is the relevant piece of the build log for the reference

Step 6 (test-build-unified-tree-check-all) failure: 1200 seconds without output running [b'ninja', b'check-all'], attempting to kill
...
11.353 [1/12/46] Linking CXX executable unittests/IR/IRTests
12.061 [1/11/47] Linking CXX executable unittests/Target/TargetMachineCTests
12.625 [1/10/48] Linking CXX executable unittests/Transforms/Instrumentation/InstrumentationTests
12.677 [1/9/49] Linking CXX executable unittests/Passes/PassBuilderBindings/PassesBindingsTests
12.818 [1/8/50] Linking CXX executable unittests/Transforms/Coroutines/CoroTests
13.139 [1/7/51] Linking CXX executable unittests/tools/llvm-mca/LLVMMCATests
13.228 [1/6/52] Linking CXX executable unittests/tools/llvm-exegesis/LLVMExegesisTests
13.336 [1/5/53] Linking CXX executable unittests/Target/X86/X86Tests
13.699 [1/4/54] Linking CXX executable unittests/Passes/Plugins/PluginsTests
17.032 [1/3/55] Linking CXX executable tools/clang/unittests/Interpreter/ExceptionTests/ClangReplInterpreterExceptionTests
command timed out: 1200 seconds without output running [b'ninja', b'check-all'], attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=1218.083118

[SelectionDAG] Move sign pattern check from AArch64 and ARM to genera…

8121a10

…l SelectionDAG

llvmbot added backend:ARM backend:AArch64 llvm:SelectionDAG SelectionDAGISel as well labels Aug 1, 2025

topperc reviewed Aug 1, 2025

View reviewed changes

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Show resolved Hide resolved

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Show resolved Hide resolved

Use ashr and getShiftAmountConstant

1c6e7dc

AZero13 requested a review from topperc August 1, 2025 18:05

clang-format

e5b5324

topperc reviewed Aug 1, 2025

View reviewed changes

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Show resolved Hide resolved

Fix sign pattern comment

17e29e0

AZero13 requested a review from topperc August 1, 2025 18:48

topperc approved these changes Aug 1, 2025

View reviewed changes

topperc reviewed Aug 1, 2025

View reviewed changes

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Show resolved Hide resolved

Fix comment

0fa1036

topperc merged commit 23022a4 into llvm:main Aug 1, 2025
9 checks passed

AZero13 deleted the asr branch August 1, 2025 23:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[SelectionDAG] Move sign pattern check from AArch64 and ARM to general SelectionDAG #151736

[SelectionDAG] Move sign pattern check from AArch64 and ARM to general SelectionDAG #151736

Uh oh!

AZero13 commented Aug 1, 2025 •

edited

Loading

Uh oh!

llvmbot commented Aug 1, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Aug 1, 2025 •

edited

Loading

Uh oh!

AZero13 commented Aug 1, 2025

Uh oh!

Uh oh!

AZero13 commented Aug 1, 2025

Uh oh!

topperc left a comment

Uh oh!

AZero13 commented Aug 1, 2025

Uh oh!

AZero13 commented Aug 1, 2025 •

edited

Loading

Uh oh!

Uh oh!

AZero13 commented Aug 1, 2025

Uh oh!

Uh oh!

llvm-ci commented Aug 1, 2025

Uh oh!

Uh oh!

[SelectionDAG] Move sign pattern check from AArch64 and ARM to general SelectionDAG #151736

[SelectionDAG] Move sign pattern check from AArch64 and ARM to general SelectionDAG #151736

Uh oh!

Conversation

AZero13 commented Aug 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Aug 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Aug 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AZero13 commented Aug 1, 2025

Uh oh!

Uh oh!

AZero13 commented Aug 1, 2025

Uh oh!

topperc left a comment

Choose a reason for hiding this comment

Uh oh!

AZero13 commented Aug 1, 2025

Uh oh!

AZero13 commented Aug 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

AZero13 commented Aug 1, 2025

Uh oh!

Uh oh!

llvm-ci commented Aug 1, 2025

Uh oh!

Uh oh!

AZero13 commented Aug 1, 2025 •

edited

Loading

llvmbot commented Aug 1, 2025 •

edited

Loading

github-actions bot commented Aug 1, 2025 •

edited

Loading

AZero13 commented Aug 1, 2025 •

edited

Loading