[ValueTracking] Enabling the patch 'an improvement for non-negative phi bits' by default

[olga.chupina]

Summary

Enabling the patch 'an improvement for non-negative phi bits' by default.
The patch was originally checked in under a flag: https://reviews.llvm.org/D18777
Performance issues were resolved. Enabling the patch as is.

[reames]

LGTM.

[olga.chupina]

Commit was reverted due to failing clang test.
After commit was reverted the test that was supposed to check my change was deleted from testing system.

So I'm uploading a new path.
The changes in lib/Analysis/ValueTracking.cpp are the same.
New changes:
I've fixed clang test: tools/clang/test/Frontend/optimization-remark-options.c
And added a new test in test/Analysis/ValueTracking folder, that checks my change.

Please review the updated patch.

Thanks in advance.

[olga.chupina]

updated patch is attached.

[olga.chupina]

Kindly ping.
Please, review updated patch and tests. Thanks.

[olga.chupina]

Please, review updated patch and tests. Thanks.

[olga.chupina]

Adding Hal Finkel as he was the last person updating llvm/tools/clang/test/Frontend/optimization-remark-options.c

[hfinkel]

In D34101#833685, @olga.chupina wrote:

Adding Hal Finkel as he was the last person updating llvm/tools/clang/test/Frontend/optimization-remark-options.c

Looks reasonable to me; go ahead and recommit.

lib/Analysis/ValueTracking.cpp

 
 const unsigned MaxDepth = 6;
 
 // Controls the number of uses of the value searched for possible
 // dominating comparisons.
 static cl::opt<unsigned> DomConditionsMaxUses("dom-conditions-max-uses",
                                               cl::Hidden, cl::init(20));
 
-// This optimization is known to cause performance regressions is some cases,
-// keep it under a temporary flag for now.
-static cl::opt<bool>
-DontImproveNonNegativePhiBits("dont-improve-non-negative-phi-bits",
-                              cl::Hidden, cl::init(true));
-
 /// Returns the bitwidth of the given scalar or pointer type. For vector types,
 /// returns the element type's bitwidth.
 static unsigned getBitWidth(Type *Ty, const DataLayout &DL) {
   if (unsigned BitWidth = Ty->getScalarSizeInBits())
     return BitWidth;
 
   return DL.getPointerTypeSizeInBits(Ty);
 }
if (P->getNumIncomingValues() == 2) {
 
           // We need to take the minimum number of known bits
           KnownBits Known3(Known);
           computeKnownBits(L, Known3, Depth + 1, Q);
 
           Known.Zero.setLowBits(std::min(Known2.countMinTrailingZeros(),
                                          Known3.countMinTrailingZeros()));
 
-          if (DontImproveNonNegativePhiBits)
-            break;
-
           auto *OverflowOp = dyn_cast<OverflowingBinaryOperator>(LU);
           if (OverflowOp && OverflowOp->hasNoSignedWrap()) {
             // If initial value of recurrence is nonnegative, and we are adding
             // a nonnegative number with nsw, the result can only be nonnegative
             // or poison value regardless of the number of times we execute the
             // add in phi recurrence. If initial value is negative and we are
             // adding a negative number with nsw, the result can only be
             // negative or poison value. Similar arguments apply to sub and mul.

test/Analysis/ValueTracking/non-negative-phi-bits.ll

+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -indvars < %s -S | FileCheck %s
+
+define void @test() #0 {
+; CHECK-LABEL: @test(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[FOR_BODY:%.*]]
+; CHECK:       for.body:
+; CHECK-NEXT:    [[INDVARS_IV:%.*]] = phi i64 [ 0, [[ENTRY:%.*]] ], [ [[INDVARS_IV_NEXT:%.*]], [[FOR_BODY]] ]
+; CHECK-NEXT:    [[INDVARS_IV_NEXT]] = add nuw nsw i64 [[INDVARS_IV]], 1
+; CHECK-NEXT:    [[EXITCOND:%.*]] = icmp eq i64 [[INDVARS_IV_NEXT]], 40
+; CHECK-NEXT:    br i1 [[EXITCOND]], label [[FOR_END:%.*]], label [[FOR_BODY]]
+; CHECK:       for.end:
+; CHECK-NEXT:    ret void
+;
+entry:
+  br label %for.body
+
+for.body:                                         ; preds = %for.body, %entry
+  %indvars.iv = phi i64 [ 0, %entry ], [ %indvars.iv.next, %for.body ]
+  %indvars.iv.next = add i64 %indvars.iv, 1
+  %exitcond = icmp eq i64 %indvars.iv.next, 40
+  br i1 %exitcond, label %for.end, label %for.body
+
+for.end:                                          ; preds = %for.body
+  ret void
+}
+

test/Frontend/optimization-remark-options.c

 // RUN: %clang -O1 -fvectorize -target x86_64-unknown-unknown -Rpass-analysis=loop-vectorize -emit-llvm -S %s -o - 2>&1 | FileCheck %s
 
 // CHECK: {{.*}}:9:11: remark: loop not vectorized: cannot prove it is safe to reorder floating-point operations; allow reordering by specifying '#pragma clang loop vectorize(enable)' before the loop or by providing the compiler option '-ffast-math'.
 
 double foo(int N) {
   double v = 0.0;
 
   for (int i = 0; i < N; i++)
     v = v + 1.0;
 
   return v;
 }
 
 // CHECK: {{.*}}:17:3: remark: loop not vectorized: cannot prove it is safe to reorder memory operations; allow reordering by specifying '#pragma clang loop vectorize(enable)' before the loop. If the arrays will always be independent specify '#pragma clang loop vectorize(assume_safety)' before the loop or provide the '__restrict__' qualifier with the independent array arguments. Erroneous results will occur if these options are incorrectly applied!
 
 void foo2(int *dw, int *uw, int *A, int *B, int *C, int *D, int N) {
-  for (int i = 0; i < N; i++) {
+  for (long i = 0; i < N; i++) {
     dw[i] = A[i] + B[i - 1] + C[i - 2] + D[i - 3];
     uw[i] = A[i] + B[i + 1] + C[i + 2] + D[i + 3];
   }
 }