VT: teach a special-case optz about samesign

[artagnon]

There is a narrow special-case in isImpliedCondICmps that can benefit from being taught about samesign. Since it costs us nothing to implement it, teach it about samesign, for completeness. This patch marks the completion of the effort to teach ValueTracking about samesign.

[llvmbot]

@llvm/pr-subscribers-llvm-analysis

Author: Ramkumar Ramachandra (artagnon)

Changes

There is a narrow special-case in isImpliedCondICmps that can benefit from being taught about samesign. Since it costs us nothing to implement it, teach it about samesign, for completeness. This patch marks the completion of the effort to teach ValueTracking about samesign.

---

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

2 Files Affected:

• (modified) llvm/lib/Analysis/ValueTracking.cpp (+4-2)
• (modified) llvm/test/Analysis/ValueTracking/implied-condition-samesign.ll (+2-8)

diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 92338d33b27a43..2491493c993cb5 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -9495,7 +9495,8 @@ isImpliedCondICmps(const ICmpInst *LHS, CmpPredicate RPred, const Value *R0,
   // must be positive if X >= Y and no overflow".
   // Take SGT as an example:  L0:x > L1:y and C >= 0
   //                      ==> R0:(x -nsw y) < R1:(-C) is false
-  if ((LPred == ICmpInst::ICMP_SGT || LPred == ICmpInst::ICMP_SGE) &&
+  if ((ICmpInst::isSigned(LPred) || LPred.hasSameSign()) &&
+      (ICmpInst::isGE(LPred) || ICmpInst::isGT(LPred)) &&
       match(R0, m_NSWSub(m_Specific(L0), m_Specific(L1)))) {
     if (match(R1, m_NonPositive()) &&
         isImpliedCondMatchingOperands(LPred, RPred) == false)
@@ -9504,7 +9505,8 @@ isImpliedCondICmps(const ICmpInst *LHS, CmpPredicate RPred, const Value *R0,
 
   // Take SLT as an example:  L0:x < L1:y and C <= 0
   //                      ==> R0:(x -nsw y) < R1:(-C) is true
-  if ((LPred == ICmpInst::ICMP_SLT || LPred == ICmpInst::ICMP_SLE) &&
+  if ((ICmpInst::isSigned(LPred) || LPred.hasSameSign()) &&
+      (ICmpInst::isLE(LPred) || ICmpInst::isLT(LPred)) &&
       match(R0, m_NSWSub(m_Specific(L0), m_Specific(L1)))) {
     if (match(R1, m_NonNegative()) &&
         isImpliedCondMatchingOperands(LPred, RPred) == true)
diff --git a/llvm/test/Analysis/ValueTracking/implied-condition-samesign.ll b/llvm/test/Analysis/ValueTracking/implied-condition-samesign.ll
index 546ff2d77d86e3..35cfadaa2965a7 100644
--- a/llvm/test/Analysis/ValueTracking/implied-condition-samesign.ll
+++ b/llvm/test/Analysis/ValueTracking/implied-condition-samesign.ll
@@ -207,10 +207,7 @@ define i32 @gt_sub_nsw(i32 %x, i32 %y) {
 ; CHECK:       [[TAKEN]]:
 ; CHECK-NEXT:    [[SUB:%.*]] = sub nsw i32 [[X]], [[Y]]
 ; CHECK-NEXT:    [[ADD:%.*]] = add nsw i32 [[SUB]], 1
-; CHECK-NEXT:    [[NEG:%.*]] = xor i32 [[SUB]], -1
-; CHECK-NEXT:    [[ABSCOND:%.*]] = icmp samesign ult i32 [[SUB]], -1
-; CHECK-NEXT:    [[ABS:%.*]] = select i1 [[ABSCOND]], i32 [[NEG]], i32 [[ADD]]
-; CHECK-NEXT:    ret i32 [[ABS]]
+; CHECK-NEXT:    ret i32 [[ADD]]
 ; CHECK:       [[END]]:
 ; CHECK-NEXT:    ret i32 0
 ;
@@ -239,10 +236,7 @@ define i32 @ge_sub_nsw(i32 %x, i32 %y) {
 ; CHECK:       [[TAKEN]]:
 ; CHECK-NEXT:    [[SUB:%.*]] = sub nsw i32 [[X]], [[Y]]
 ; CHECK-NEXT:    [[ADD:%.*]] = add nsw i32 [[SUB]], 1
-; CHECK-NEXT:    [[NEG:%.*]] = xor i32 [[SUB]], -1
-; CHECK-NEXT:    [[ABSCOND:%.*]] = icmp samesign ult i32 [[SUB]], -1
-; CHECK-NEXT:    [[ABS:%.*]] = select i1 [[ABSCOND]], i32 [[NEG]], i32 [[ADD]]
-; CHECK-NEXT:    ret i32 [[ABS]]
+; CHECK-NEXT:    ret i32 [[ADD]]
 ; CHECK:       [[END]]:
 ; CHECK-NEXT:    ret i32 0
 ;

[dtcxzyw]

LG

[llvm-ci]

LLVM Buildbot has detected a new failure on builder llvm-clang-x86_64-expensive-checks-debian running on gribozavr4 while building llvm at step 6 "test-build-unified-tree-check-all".

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

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

Step 6 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'LLVM :: tools/llvm-gsymutil/ARM_AArch64/macho-merged-funcs-dwarf.yaml' FAILED ********************
Exit Code: 1

Command Output (stdout):
--
Input file: /b/1/llvm-clang-x86_64-expensive-checks-debian/build/test/tools/llvm-gsymutil/ARM_AArch64/Output/macho-merged-funcs-dwarf.yaml.tmp.dSYM
Output file (aarch64): /b/1/llvm-clang-x86_64-expensive-checks-debian/build/test/tools/llvm-gsymutil/ARM_AArch64/Output/macho-merged-funcs-dwarf.yaml.tmp.default.gSYM
Loaded 3 functions from DWARF.
Loaded 3 functions from symbol table.
warning: same address range contains different debug info. Removing:
[0x0000000000000248 - 0x0000000000000270): Name=0x00000047
addr=0x0000000000000248, file=  3, line=  5
addr=0x0000000000000254, file=  3, line=  7
addr=0x0000000000000258, file=  3, line=  9
addr=0x000000000000025c, file=  3, line=  8
addr=0x0000000000000260, file=  3, line= 11
addr=0x0000000000000264, file=  3, line= 10
addr=0x0000000000000268, file=  3, line=  6


In favor of this one:
[0x0000000000000248 - 0x0000000000000270): Name=0x00000030
addr=0x0000000000000248, file=  2, line=  5
addr=0x0000000000000254, file=  2, line=  7
addr=0x0000000000000258, file=  2, line=  9
addr=0x000000000000025c, file=  2, line=  8
addr=0x0000000000000260, file=  2, line= 11
addr=0x0000000000000264, file=  2, line= 10
addr=0x0000000000000268, file=  2, line=  6


warning: same address range contains different debug info. Removing:
[0x0000000000000248 - 0x0000000000000270): Name=0x00000030
addr=0x0000000000000248, file=  2, line=  5
addr=0x0000000000000254, file=  2, line=  7
addr=0x0000000000000258, file=  2, line=  9
addr=0x000000000000025c, file=  2, line=  8
addr=0x0000000000000260, file=  2, line= 11
addr=0x0000000000000264, file=  2, line= 10
addr=0x0000000000000268, file=  2, line=  6


In favor of this one:
[0x0000000000000248 - 0x0000000000000270): Name=0x00000001
addr=0x0000000000000248, file=  1, line=  5
addr=0x0000000000000254, file=  1, line=  7
addr=0x0000000000000258, file=  1, line=  9
addr=0x000000000000025c, file=  1, line=  8
addr=0x0000000000000260, file=  1, line= 11
addr=0x0000000000000264, file=  1, line= 10
...

[nikic]

This caused a compile-time regression: https://llvm-compile-time-tracker.com/compare.php?from=26b4a0ac7ed3f04f10bd1c043e2cf9c52da7fc47&to=66badf224ade6e78d5da005f6a9819092fd8767b&stat=instructions:u

I think we should add a more efficient variant of getMatching() for the case where we just want to check for equality. getMatching() does this in a really inefficient way.

[nikic]

Though in this particular case, the most efficient way would probably be something like getSignedPredicate() because then samesign doesn't have to be considered four times for four predicate comparisons.