Skip to content

Commit 0a5347f

Browse files
committed
[DAG] SimplifyDemandedBits - Use DemandedBits intead of OriginalDemandedBits to when simplifying UMIN/UMAX to AND/OR.
DemandedBits is forced to all ones if there are multiple users. The changes X86 test cases looks like they were miscompiles before. The value of eax/rax from the cmov is returned from the function in addition to being used by the sar. That usage needs all bits even though the sar doesn't.
1 parent 770be43 commit 0a5347f

File tree

3 files changed

+12
-6
lines changed

3 files changed

+12
-6
lines changed

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2139,7 +2139,7 @@ bool TargetLowering::SimplifyDemandedBits(
21392139
SDValue Op0 = Op.getOperand(0);
21402140
SDValue Op1 = Op.getOperand(1);
21412141
// If we're only wanting the msb, then we can simplify to AND node.
2142-
if (OriginalDemandedBits.isSignMask())
2142+
if (DemandedBits.isSignMask())
21432143
return TLO.CombineTo(Op, TLO.DAG.getNode(ISD::AND, dl, VT, Op0, Op1));
21442144
// Check if one arg is always less than (or equal) to the other arg.
21452145
KnownBits Known0 = TLO.DAG.computeKnownBits(Op0, DemandedElts, Depth + 1);
@@ -2155,7 +2155,7 @@ bool TargetLowering::SimplifyDemandedBits(
21552155
SDValue Op0 = Op.getOperand(0);
21562156
SDValue Op1 = Op.getOperand(1);
21572157
// If we're only wanting the msb, then we can simplify to OR node.
2158-
if (OriginalDemandedBits.isSignMask())
2158+
if (DemandedBits.isSignMask())
21592159
return TLO.CombineTo(Op, TLO.DAG.getNode(ISD::OR, dl, VT, Op0, Op1));
21602160
// Check if one arg is always greater than (or equal) to the other arg.
21612161
KnownBits Known0 = TLO.DAG.computeKnownBits(Op0, DemandedElts, Depth + 1);

llvm/test/CodeGen/X86/umax.ll

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1287,7 +1287,9 @@ define i64 @test_signbits_i64(i64 %a, i64 %b) nounwind {
12871287
; X86-LABEL: test_signbits_i64:
12881288
; X86: # %bb.0:
12891289
; X86-NEXT: movl {{[0-9]+}}(%esp), %eax
1290-
; X86-NEXT: orl {{[0-9]+}}(%esp), %eax
1290+
; X86-NEXT: movl {{[0-9]+}}(%esp), %ecx
1291+
; X86-NEXT: cmpl %eax, %ecx
1292+
; X86-NEXT: cmoval %ecx, %eax
12911293
; X86-NEXT: movl %eax, %edx
12921294
; X86-NEXT: sarl $31, %edx
12931295
; X86-NEXT: retl
@@ -1302,7 +1304,8 @@ define i128 @test_signbits_i128(i128 %a, i128 %b) nounwind {
13021304
; X64: # %bb.0:
13031305
; X64-NEXT: movq %rcx, %rax
13041306
; X64-NEXT: sarq $28, %rax
1305-
; X64-NEXT: orq %rsi, %rax
1307+
; X64-NEXT: cmpq %rax, %rsi
1308+
; X64-NEXT: cmovaq %rsi, %rax
13061309
; X64-NEXT: movq %rax, %rdx
13071310
; X64-NEXT: sarq $63, %rdx
13081311
; X64-NEXT: retq

llvm/test/CodeGen/X86/umin.ll

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -702,7 +702,9 @@ define i64 @test_signbits_i64(i64 %a, i64 %b) nounwind {
702702
; X86-LABEL: test_signbits_i64:
703703
; X86: # %bb.0:
704704
; X86-NEXT: movl {{[0-9]+}}(%esp), %eax
705-
; X86-NEXT: andl {{[0-9]+}}(%esp), %eax
705+
; X86-NEXT: movl {{[0-9]+}}(%esp), %ecx
706+
; X86-NEXT: cmpl %eax, %ecx
707+
; X86-NEXT: cmovbl %ecx, %eax
706708
; X86-NEXT: movl %eax, %edx
707709
; X86-NEXT: sarl $31, %edx
708710
; X86-NEXT: retl
@@ -717,7 +719,8 @@ define i128 @test_signbits_i128(i128 %a, i128 %b) nounwind {
717719
; X64: # %bb.0:
718720
; X64-NEXT: movq %rcx, %rax
719721
; X64-NEXT: sarq $28, %rax
720-
; X64-NEXT: andq %rsi, %rax
722+
; X64-NEXT: cmpq %rax, %rsi
723+
; X64-NEXT: cmovbq %rsi, %rax
721724
; X64-NEXT: movq %rax, %rdx
722725
; X64-NEXT: sarq $63, %rdx
723726
; X64-NEXT: retq

0 commit comments

Comments
 (0)