[llvm] e46d74b - [CVP] Allow two transforms in one invocation

[Philip Reames via llvm-commits]

Author: Philip Reames
Date: 2020-09-28T15:11:42-07:00
New Revision: e46d74b58922a427562552464d798448520e4928

URL: https://github.com/llvm/llvm-project/commit/e46d74b58922a427562552464d798448520e4928
DIFF: https://github.com/llvm/llvm-project/commit/e46d74b58922a427562552464d798448520e4928.diff

LOG: [CVP] Allow two transforms in one invocation

For a call site which had both constant deopt operands and nonnull arguments, we were missing the opportunity to recognize the later by bailing early.

This is somewhat of a speculative fix.  Months ago, I'd had a private report of performance and compile time regressions from the deopt operand folding.  I never received a test case.  However, the only possibility I see was that after that change CVP missed the nonnull fold, and we end up with a pass ordering/missed simplification issue.  So, since it's a real issue, fix it and hope.

Added: 
    

Modified: 
    llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
    llvm/test/Transforms/CorrelatedValuePropagation/deopt.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp b/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
index d06caa6be78a..b7b1b9c6aa58 100644
--- a/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
+++ b/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
@@ -524,8 +524,6 @@ static void processSaturatingInst(SaturatingInst *SI, LazyValueInfo *LVI) {
 
 /// Infer nonnull attributes for the arguments at the specified callsite.
 static bool processCallSite(CallBase &CB, LazyValueInfo *LVI) {
-  SmallVector<unsigned, 4> ArgNos;
-  unsigned ArgNo = 0;
 
   if (auto *WO = dyn_cast<WithOverflowInst>(&CB)) {
     if (WO->getLHS()->getType()->isIntegerTy() && willNotOverflow(WO, LVI)) {
@@ -541,6 +539,8 @@ static bool processCallSite(CallBase &CB, LazyValueInfo *LVI) {
     }
   }
 
+  bool Changed = false;
+
   // Deopt bundle operands are intended to capture state with minimal
   // perturbance of the code otherwise.  If we can find a constant value for
   // any such operand and remove a use of the original value, that's
@@ -549,7 +549,6 @@ static bool processCallSite(CallBase &CB, LazyValueInfo *LVI) {
   // idiomatically, appear along rare conditional paths, it's reasonable likely
   // we may have a conditional fact with which LVI can fold.
   if (auto DeoptBundle = CB.getOperandBundle(LLVMContext::OB_deopt)) {
-    bool Progress = false;
     for (const Use &ConstU : DeoptBundle->Inputs) {
       Use &U = const_cast<Use&>(ConstU);
       Value *V = U.get();
@@ -559,12 +558,13 @@ static bool processCallSite(CallBase &CB, LazyValueInfo *LVI) {
       Constant *C = LVI->getConstant(V, &CB);
       if (!C) continue;
       U.set(C);
-      Progress = true;
+      Changed = true;
     }
-    if (Progress)
-      return true;
   }
 
+  SmallVector<unsigned, 4> ArgNos;
+  unsigned ArgNo = 0;
+
   for (Value *V : CB.args()) {
     PointerType *Type = dyn_cast<PointerType>(V->getType());
     // Try to mark pointer typed parameters as non-null.  We skip the
@@ -582,7 +582,7 @@ static bool processCallSite(CallBase &CB, LazyValueInfo *LVI) {
   assert(ArgNo == CB.arg_size() && "sanity check");
 
   if (ArgNos.empty())
-    return false;
+    return Changed;
 
   AttributeList AS = CB.getAttributes();
   LLVMContext &Ctx = CB.getContext();

diff  --git a/llvm/test/Transforms/CorrelatedValuePropagation/deopt.ll b/llvm/test/Transforms/CorrelatedValuePropagation/deopt.ll
index affd456182d3..af4bf6f90c77 100644
--- a/llvm/test/Transforms/CorrelatedValuePropagation/deopt.ll
+++ b/llvm/test/Transforms/CorrelatedValuePropagation/deopt.ll
@@ -2,6 +2,8 @@
 ; RUN: opt -correlated-propagation -S < %s | FileCheck %s
 
 declare void @use()
+declare void @use_ptr(i8*)
+
 ; test requires a mix of context sensative refinement, and analysis
 ; of the originating IR pattern.  Neither part is enough in isolation.
 define void @test1(i1 %c, i1 %c2) {
@@ -140,3 +142,23 @@ taken:
 untaken:
   ret void
 }
+
+
+define void @test5(i64 %a, i8* nonnull %p) {
+; CHECK-LABEL: @test5(
+; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i64 [[A:%.*]], 0
+; CHECK-NEXT:    br i1 [[CMP]], label [[TAKEN:%.*]], label [[UNTAKEN:%.*]]
+; CHECK:       taken:
+; CHECK-NEXT:    call void @use_ptr(i8* nonnull [[P:%.*]]) [ "deopt"(i64 0) ]
+; CHECK-NEXT:    ret void
+; CHECK:       untaken:
+; CHECK-NEXT:    ret void
+;
+  %cmp = icmp eq i64 %a, 0
+  br i1 %cmp, label %taken, label %untaken
+taken:
+  call void @use_ptr(i8* %p) ["deopt" (i64 %a)]
+  ret void
+untaken:
+  ret void
+}