Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DAG] Extend the computeOverflowForSignedSub/computeOverflowForUnsignedSub implementations with ConstantRange #67890

Merged
merged 2 commits into from
Oct 1, 2023

Conversation

elhewaty
Copy link
Contributor

  • Add tests for computeOverflowFor*Sub functions
  • extend the computeOverflowForSignedSub/computeOverflowForUnsignedSub implementations with ConstantRange

@llvmbot llvmbot added backend:X86 llvm:SelectionDAG SelectionDAGISel as well labels Sep 30, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 30, 2023

@llvm/pr-subscribers-llvm-selectiondag

@llvm/pr-subscribers-backend-x86

Changes
  • Add tests for computeOverflowFor*Sub functions
  • extend the computeOverflowForSignedSub/computeOverflowForUnsignedSub implementations with ConstantRange

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

3 Files Affected:

  • (modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp (+10-4)
  • (modified) llvm/test/CodeGen/X86/combine-subo.ll (+81)
  • (modified) llvm/test/CodeGen/X86/or-with-overflow.ll (+3-9)
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index cd21af770e1a4d9..0a61920b7c079ba 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -4091,8 +4091,11 @@ SelectionDAG::computeOverflowForSignedSub(SDValue N0, SDValue N1) const {
   if (ComputeNumSignBits(N0) > 1 && ComputeNumSignBits(N1) > 1)
     return OFK_Never;
 
-  // TODO: Add ConstantRange::signedSubMayOverflow handling.
-  return OFK_Sometime;
+  KnownBits N0Known = computeKnownBits(N0);
+  KnownBits N1Known = computeKnownBits(N1);
+  ConstantRange N0Range = ConstantRange::fromKnownBits(N0Known, true);
+  ConstantRange N1Range = ConstantRange::fromKnownBits(N1Known, true);
+  return mapOverflowResult(N0Range.signedSubMayOverflow(N1Range));
 }
 
 SelectionDAG::OverflowKind
@@ -4101,8 +4104,11 @@ SelectionDAG::computeOverflowForUnsignedSub(SDValue N0, SDValue N1) const {
   if (isNullConstant(N1))
     return OFK_Never;
 
-  // TODO: Add ConstantRange::unsignedSubMayOverflow handling.
-  return OFK_Sometime;
+  KnownBits N0Known = computeKnownBits(N0);
+  KnownBits N1Known = computeKnownBits(N1);
+  ConstantRange N0Range = ConstantRange::fromKnownBits(N0Known, false);
+  ConstantRange N1Range = ConstantRange::fromKnownBits(N1Known, false);
+  return mapOverflowResult(N0Range.unsignedSubMayOverflow(N1Range));
 }
 
 SelectionDAG::OverflowKind
diff --git a/llvm/test/CodeGen/X86/combine-subo.ll b/llvm/test/CodeGen/X86/combine-subo.ll
index 6965f6d7af27b53..99f26525d49e5b5 100644
--- a/llvm/test/CodeGen/X86/combine-subo.ll
+++ b/llvm/test/CodeGen/X86/combine-subo.ll
@@ -4,9 +4,14 @@
 
 declare {i32, i1} @llvm.ssub.with.overflow.i32(i32, i32) nounwind readnone
 declare {i32, i1} @llvm.usub.with.overflow.i32(i32, i32) nounwind readnone
+declare { i8, i1 } @llvm.ssub.with.overflow.i8(i8, i8) nounwind readnone
+declare { i8, i1 } @llvm.usub.with.overflow.i8(i8, i8) nounwind readnone
+
 
 declare {<4 x i32>, <4 x i1>} @llvm.ssub.with.overflow.v4i32(<4 x i32>, <4 x i32>) nounwind readnone
 declare {<4 x i32>, <4 x i1>} @llvm.usub.with.overflow.v4i32(<4 x i32>, <4 x i32>) nounwind readnone
+declare { <4 x i8>, <4 x i1> } @llvm.ssub.with.overflow.v4i8(<4 x i8>, <4 x i8>) nounwind readnone
+declare { <4 x i8>, <4 x i1> } @llvm.usub.with.overflow.v4i8(<4 x i8> , <4 x i8>) nounwind readnone
 
 ; fold (ssub x, 0) -> x
 define i32 @combine_ssub_zero(i32 %a0, i32 %a1) {
@@ -148,3 +153,79 @@ define <4 x i32> @combine_vec_usub_negone(<4 x i32> %a0, <4 x i32> %a1) {
   %4 = select <4 x i1> %3, <4 x i32> %a1, <4 x i32> %2
   ret <4 x i32> %4
 }
+
+define { i32, i1 } @combine_usub_nuw(i32 %a, i32 %b) {
+; CHECK-LABEL: combine_usub_nuw:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    movl %edi, %eax
+; CHECK-NEXT:    orl $-2147483648, %eax # imm = 0x80000000
+; CHECK-NEXT:    andl $2147483647, %esi # imm = 0x7FFFFFFF
+; CHECK-NEXT:    subl %esi, %eax
+; CHECK-NEXT:    xorl %edx, %edx
+; CHECK-NEXT:    retq
+  %aa = or i32 %a, 2147483648
+  %bb = and i32 %b, 2147483647
+  %x = call { i32, i1 } @llvm.usub.with.overflow.i32(i32 %aa, i32 %bb)
+  ret { i32, i1 } %x
+}
+
+define { i8, i1 } @usub_always_overflow(i8 %x) nounwind {
+; CHECK-LABEL: usub_always_overflow:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    orb $64, %dil
+; CHECK-NEXT:    movb $63, %al
+; CHECK-NEXT:    subb %dil, %al
+; CHECK-NEXT:    setb %dl
+; CHECK-NEXT:    retq
+  %y = or i8 %x, 64
+  %a = call { i8, i1 } @llvm.usub.with.overflow.i8(i8 63, i8 %y)
+  ret { i8, i1 } %a
+}
+
+define { i8, i1 } @ssub_always_overflow(i8 %x) nounwind {
+; CHECK-LABEL: ssub_always_overflow:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    cmpb $30, %dil
+; CHECK-NEXT:    movl $29, %ecx
+; CHECK-NEXT:    cmovgel %edi, %ecx
+; CHECK-NEXT:    movb $-100, %al
+; CHECK-NEXT:    subb %cl, %al
+; CHECK-NEXT:    seto %dl
+; CHECK-NEXT:    retq
+  %c = icmp sgt i8 %x, 29
+  %y = select i1 %c, i8 %x, i8 29
+  %a = call { i8, i1 } @llvm.ssub.with.overflow.i8(i8 -100, i8 %y)
+  ret { i8, i1 } %a
+}
+
+define { <4 x i8>, <4 x i1> } @always_usub_const_vector() nounwind {
+; SSE-LABEL: always_usub_const_vector:
+; SSE:       # %bb.0:
+; SSE-NEXT:    pcmpeqd %xmm0, %xmm0
+; SSE-NEXT:    pcmpeqd %xmm1, %xmm1
+; SSE-NEXT:    retq
+;
+; AVX-LABEL: always_usub_const_vector:
+; AVX:       # %bb.0:
+; AVX-NEXT:    vpcmpeqd %xmm0, %xmm0, %xmm0
+; AVX-NEXT:    vpcmpeqd %xmm1, %xmm1, %xmm1
+; AVX-NEXT:    retq
+  %x = call { <4 x i8>, <4 x i1> } @llvm.usub.with.overflow.v4i8(<4 x i8> <i8 0, i8 0, i8 0, i8 0>, <4 x i8> <i8 1, i8 1, i8 1, i8 1>)
+  ret { <4 x i8>, <4 x i1> } %x
+}
+
+define { <4 x i8>, <4 x i1> } @never_usub_const_vector() nounwind {
+; SSE-LABEL: never_usub_const_vector:
+; SSE:       # %bb.0:
+; SSE-NEXT:    movaps {{.*#+}} xmm0 = <127,255,0,254,u,u,u,u,u,u,u,u,u,u,u,u>
+; SSE-NEXT:    xorps %xmm1, %xmm1
+; SSE-NEXT:    retq
+;
+; AVX-LABEL: never_usub_const_vector:
+; AVX:       # %bb.0:
+; AVX-NEXT:    vbroadcastss {{.*#+}} xmm0 = [127,255,0,254,127,255,0,254,127,255,0,254,127,255,0,254]
+; AVX-NEXT:    vxorps %xmm1, %xmm1, %xmm1
+; AVX-NEXT:    retq
+  %x = call { <4 x i8>, <4 x i1> } @llvm.usub.with.overflow.v4i8(<4 x i8> <i8 255, i8 255, i8 255, i8 255>, <4 x i8> <i8 128, i8 0, i8 255, i8 1>)
+  ret { <4 x i8>, <4 x i1> } %x
+}
diff --git a/llvm/test/CodeGen/X86/or-with-overflow.ll b/llvm/test/CodeGen/X86/or-with-overflow.ll
index 4440485af54bbaa..b3ffa209bc7004e 100644
--- a/llvm/test/CodeGen/X86/or-with-overflow.ll
+++ b/llvm/test/CodeGen/X86/or-with-overflow.ll
@@ -161,19 +161,13 @@ define i32 @or_i32_rr(i32 %0, i32 %1) {
 define i64 @or_i64_ri(i64 %0, i64 %1) nounwind {
 ; X86-LABEL: or_i64_ri:
 ; X86:       # %bb.0:
-; X86-NEXT:    pushl %esi
 ; X86-NEXT:    movl {{[0-9]+}}(%esp), %eax
 ; X86-NEXT:    movl {{[0-9]+}}(%esp), %edx
-; X86-NEXT:    movl %eax, %ecx
-; X86-NEXT:    orl $17, %ecx
-; X86-NEXT:    cmpl $1, %ecx
-; X86-NEXT:    movl %edx, %esi
-; X86-NEXT:    sbbl $0, %esi
-; X86-NEXT:    jl .LBB6_2
+; X86-NEXT:    testl %edx, %edx
+; X86-NEXT:    js .LBB6_2
 ; X86-NEXT:  # %bb.1:
-; X86-NEXT:    movl %ecx, %eax
+; X86-NEXT:    orl $17, %eax
 ; X86-NEXT:  .LBB6_2:
-; X86-NEXT:    popl %esi
 ; X86-NEXT:    retl
 ;
 ; X64-LABEL: or_i64_ri:

@RKSimon RKSimon changed the title subOverflow [DAG] Extend the computeOverflowForSignedSub/computeOverflowForUnsignedSub implementations with ConstantRange Oct 1, 2023
@RKSimon RKSimon self-requested a review October 1, 2023 13:25
Copy link
Collaborator

@RKSimon RKSimon left a comment

Choose a reason for hiding this comment

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

LGTM - thanks

@elhewaty
Copy link
Contributor Author

elhewaty commented Oct 1, 2023

@RKSimon Do you have more issues/projects I can work on?

@RKSimon
Copy link
Collaborator

RKSimon commented Oct 1, 2023

@RKSimon Do you have more issues/projects I can work on?

#66101 might be interesting - hopefully this can be addressed as a generic DAGCombine fold and not x86-specific

@elhewaty
Copy link
Contributor Author

elhewaty commented Oct 1, 2023

Will this be merged soon?

@RKSimon RKSimon merged commit 9103b1d into llvm:main Oct 1, 2023
4 of 5 checks passed
@elhewaty elhewaty deleted the subOverflow branch October 2, 2023 07:20
@ajarmusch
Copy link
Contributor

This change seemed to have broken this build bot yesterday.

https://gitlab.e4s.io/uo-public/llvm-sollve/-/pipelines/9388/

@RKSimon
Copy link
Collaborator

RKSimon commented Oct 2, 2023

@ajarmusch Please can you provide any more information? I can't even find a reference to 9103b1d on the log

@elhewaty
Copy link
Contributor Author

elhewaty commented Oct 2, 2023

@RKSimon: I received this link by the mail yesterday:
https://lab.llvm.org/buildbot/#/builders/238/builds/5908

@RKSimon
Copy link
Collaborator

RKSimon commented Oct 2, 2023

@RKSimon: I received this link by the mail yesterday: https://lab.llvm.org/buildbot/#/builders/238/builds/5908

That was fixed by 632022e

@ajarmusch
Copy link
Contributor

@RKSimon sure thing

the commit 632022e shows up in the artifacts

@RKSimon
Copy link
Collaborator

RKSimon commented Oct 2, 2023

632022e looks ok: https://alive2.llvm.org/ce/z/V33G3f

Please can you confirm why -lFortranRuntime is missing?

FAILED: bin/external-hello-world 
: && /usr/local/llvm/16.0.6/bin/clang++ -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -Wno-deprecated-copy -Wno-string-conversion -Wno-ctad-maybe-unsupported -Wno-unused-command-line-argument -Wstring-conversion           -Wcovered-switch-default -Wno-nested-anon-types -O3 -DNDEBUG -Wl,-rpath-link,/home/gitlab-runner-sollve/builds/yP7whQwj9/0/uo-public/llvm-sollve/build/./lib  -Wl,--gc-sections tools/flang/examples/ExternalHelloWorld/CMakeFiles/external-hello-world.dir/external-hello.cpp.o -o bin/external-hello-world  -Wl,-rpath,"\$ORIGIN/../lib:"  -lpthread  -lFortranRuntime && :
/opt/rh/gcc-toolset-12/root/usr/lib/gcc/x86_64-redhat-linux/12/../../../../bin/ld: cannot find -lFortranRuntime: No such file or directory

@ajarmusch
Copy link
Contributor

thanks for checking - I'll ask the CI admin

@RKSimon
Copy link
Collaborator

RKSimon commented Oct 4, 2023

@ajarmusch Any update?

@ajarmusch
Copy link
Contributor

It was a different commit that caused the failure. Trying to prevent another false blame

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
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants