Skip to content

release/23.x: [SDAG] Fix invalid sign bit condition for abs(sub) -> abdu fold (#215548) - #217203

Merged
dyung merged 1 commit into
llvm:release/23.xfrom
llvmbot:issue215548
Aug 20, 2026
Merged

release/23.x: [SDAG] Fix invalid sign bit condition for abs(sub) -> abdu fold (#215548)#217203
dyung merged 1 commit into
llvm:release/23.xfrom
llvmbot:issue215548

Conversation

@llvmbot

@llvmbot llvmbot commented Aug 19, 2026

Copy link
Copy Markdown
Member

Backport 93030c3

Requested by: @Benjins

@llvmbot

llvmbot commented Aug 19, 2026

Copy link
Copy Markdown
Member Author

@topperc What do you think about merging this PR to the release branch?

@github-actions

Copy link
Copy Markdown

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Developer Policy and LLVM Discourse for more information.

@llvmorg-github-actions

llvmorg-github-actions Bot commented Aug 19, 2026

Copy link
Copy Markdown

@llvm/pr-subscribers-llvm-selectiondag

@llvm/pr-subscribers-backend-x86

Author: llvmbot

Changes

Backport 93030c3

Requested by: @Benjins


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

2 Files Affected:

  • (modified) llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (+2-2)
  • (modified) llvm/test/CodeGen/X86/abdu.ll (+30)
diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 5fd1b10d97f35..67e16dcb41c7f 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -12126,8 +12126,8 @@ SDValue DAGCombiner::foldABSToABD(SDNode *N, const SDLoc &DL) {
       return CreateZextedAbd(ISD::ABDS);
 
     // fold (abs (sub x, y)) -> abdu(x, y)
-    bool Op1SignBitIsOne = DAG.computeKnownBits(Op1).isNegative();
-    bool AbsOpWillNUW = !IsAdd && DAG.SignBitIsZero(Op0) && Op1SignBitIsOne;
+    bool AbsOpWillNUW =
+        !IsAdd && DAG.SignBitIsZero(Op0) && DAG.SignBitIsZero(Op1);
 
     if (hasOperation(ISD::ABDU, VT) && AbsOpWillNUW)
       return CreateZextedAbd(ISD::ABDU);
diff --git a/llvm/test/CodeGen/X86/abdu.ll b/llvm/test/CodeGen/X86/abdu.ll
index b8bc3649773f2..520a6917aff53 100644
--- a/llvm/test/CodeGen/X86/abdu.ll
+++ b/llvm/test/CodeGen/X86/abdu.ll
@@ -949,6 +949,36 @@ define i128 @abd_select_i128(i128 %a, i128 %b) nounwind {
   ret i128 %sub
 }
 
+define i32 @abs_sub_abdu_sign_check(i32 %p0) {
+; X86-LABEL: abs_sub_abdu_sign_check:
+; X86:       # %bb.0: # %entry
+; X86-NEXT:    cmpl $0, {{[0-9]+}}(%esp)
+; X86-NEXT:    movl $1000000000, %eax # imm = 0x3B9ACA00
+; X86-NEXT:    movl $-2039640824, %ecx # imm = 0x866D8D08
+; X86-NEXT:    cmovel %eax, %ecx
+; X86-NEXT:    movl %ecx, %eax
+; X86-NEXT:    negl %eax
+; X86-NEXT:    cmovsl %ecx, %eax
+; X86-NEXT:    retl
+;
+; X64-LABEL: abs_sub_abdu_sign_check:
+; X64:       # %bb.0: # %entry
+; X64-NEXT:    testl %edi, %edi
+; X64-NEXT:    movl $1000000000, %eax # imm = 0x3B9ACA00
+; X64-NEXT:    movl $-2039640824, %ecx # imm = 0x866D8D08
+; X64-NEXT:    cmovel %eax, %ecx
+; X64-NEXT:    movl %ecx, %eax
+; X64-NEXT:    negl %eax
+; X64-NEXT:    cmovsl %ecx, %eax
+; X64-NEXT:    retq
+entry:
+  %cmp = icmp eq i32 %p0, 0
+  %v = select i1 %cmp, i32 0, i32 1255326472
+  %s = sub i32 %v, -1000000000
+  %a = call i32 @llvm.abs.i32(i32 %s, i1 false)
+  ret i32 %a
+}
+
 declare i8 @llvm.abs.i8(i8, i1)
 declare i16 @llvm.abs.i16(i16, i1)
 declare i32 @llvm.abs.i32(i32, i1)

@dyung dyung moved this from Needs Triage to Needs Review in LLVM Release Status Aug 19, 2026

@topperc topperc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@github-project-automation github-project-automation Bot moved this from Needs Review to Needs Merge in LLVM Release Status Aug 19, 2026
…#215548)

The fold here for (abs (sub x y)) -> (abdu x y) was proven in Alive,
assuming that both operands had a sign bit of zero. However, the code
was checking if x had a sign bit of zero and y had a sign bit of 1

Fixes llvm#214942

Original Alive proof from
llvm#186659 :
https://alive2.llvm.org/ce/z/HfPF5q
A variant that's explicitly (abs (sub x y)):
https://alive2.llvm.org/ce/z/QEgDaa
And changing the range to 32770 or higher there will break the
transformation

(cherry picked from commit 93030c3)
@dyung
dyung merged commit fb14f7f into llvm:release/23.x Aug 20, 2026
2 of 4 checks passed
@github-project-automation github-project-automation Bot moved this from Needs Merge to Done in LLVM Release Status Aug 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend:X86 llvm:SelectionDAG SelectionDAGISel as well

Projects

Development

Successfully merging this pull request may close these issues.

4 participants