[MemorySSA] Small fix for the clobber limit.

Summary:
After introducing the limit for clobber walking, `walkToPhiOrClobber` would assert that the limit is at least 1 on entry.
The test included triggered that assert.

The callsite in `tryOptimizePhi` making the calls to `walkToPhiOrClobber` is structured like this:
```
while (true) {
   if (getBlockingAccess()) { // calls walkToPhiOrClobber
   }
   for (...) {
     walkToPhiOrClobber();
   }
}
```

The cleanest fix is to check if the limit was reached inside `walkToPhiOrClobber`, and give an allowence of 1.
This approach not make any alias() calls (no calls to instructionClobbersQuery), so the performance condition is enforced.
The limit is set back to 0 if not used, as this provides info on the fact that we stopped before reaching a true clobber.

Reviewers: george.burgess.iv

Subscribers: jlebar, Prazek, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D60479

llvm-svn: 358303

Author: alinas

llvm/lib/Analysis/MemorySSA.cpp

@@ -544,10 +544,15 @@ template <class AliasAnalysisType> class ClobberWalker {
                      const MemoryAccess *SkipStopAt = nullptr) const {
     assert(!isa<MemoryUse>(Desc.Last) && "Uses don't exist in my world");
     assert(UpwardWalkLimit && "Need a valid walk limit");
-    // This will not do any alias() calls. It returns in the first iteration in
-    // the loop below.
-    if (*UpwardWalkLimit == 0)
-      (*UpwardWalkLimit)++;
+    bool LimitAlreadyReached = false;
+    // (*UpwardWalkLimit) may be 0 here, due to the loop in tryOptimizePhi. Set
+    // it to 1. This will not do any alias() calls. It either returns in the
+    // first iteration in the loop below, or is set back to 0 if all def chains
+    // are free of MemoryDefs.
+    if (!*UpwardWalkLimit) {
+      *UpwardWalkLimit = 1;
+      LimitAlreadyReached = true;
+    }
 
     for (MemoryAccess *Current : def_chain(Desc.Last)) {
       Desc.Last = Current;
@@ -568,6 +573,9 @@ template <class AliasAnalysisType> class ClobberWalker {
       }
     }
 
+    if (LimitAlreadyReached)
+      *UpwardWalkLimit = 0;
+
     assert(isa<MemoryPhi>(Desc.Last) &&
            "Ended at a non-clobber that's not a phi?");
     return {Desc.Last, false, MayAlias};

llvm/test/Analysis/MemorySSA/reduce_clobber_limit.ll

@@ -0,0 +1,131 @@
+; RUN: opt -S -memoryssa %s | FileCheck %s
+; REQUIRES: asserts
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK-LABEL: @func()
+; Function Attrs: noinline
+define dso_local void @func() unnamed_addr #0 align 2 {
+entry:
+  %NoFinalize.addr = alloca i8, align 1
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  %call8 = call zeroext i1 @foo()
+  br i1 %call8, label %if.then9, label %while.cond
+
+if.then9:                                         ; preds = %entry
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  br label %while.cond
+
+while.cond:                                       ; preds = %cleanup, %if.then9, %entry
+  %call34 = call zeroext i1 @foo()
+  call void @blah()
+  br i1 %call34, label %while.body, label %while.end
+
+while.body:                                       ; preds = %while.cond
+  %call35 = call zeroext i1 @foo()
+  br i1 %call35, label %if.end37, label %if.then36
+
+if.then36:                                        ; preds = %while.body
+  store i32 2, i32* undef, align 4
+  br label %cleanup
+
+if.end37:                                         ; preds = %while.body
+  %call38 = call zeroext i1 @foo()
+  br i1 %call38, label %if.end46, label %land.lhs.true
+
+land.lhs.true:                                    ; preds = %if.end37
+  call void @blah()
+  %call41 = call zeroext i1 @foo()
+  br i1 %call41, label %if.then42, label %if.end46
+
+if.then42:                                        ; preds = %land.lhs.true
+  call void @blah()
+  br label %if.end46
+
+if.end46:                                         ; preds = %if.then42, %land.lhs.true, %if.end37
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  br label %cleanup
+
+cleanup:                                          ; preds = %if.end46, %if.then36
+  call void @blah()
+  br label %while.cond
+
+while.end:                                        ; preds = %while.cond
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  %call93 = call zeroext i1 @foo()
+  br i1 %call93, label %if.end120, label %if.then94
+
+if.then94:                                        ; preds = %while.end
+  store i32 0, i32* undef, align 4
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  br label %for.cond
+
+for.cond:                                         ; preds = %for.body, %if.then94
+  br i1 undef, label %for.body, label %if.end120
+
+for.body:                                         ; preds = %for.cond
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  call void @blah()
+  br label %for.cond
+
+if.end120:                                        ; preds = %for.cond, %while.end
+  %val = load i8, i8* %NoFinalize.addr, align 1
+  ret void
+}
+
+; Function Attrs: noinline
+declare hidden void @blah() unnamed_addr #0 align 2
+
+; Function Attrs: noinline
+declare hidden i1 @foo() local_unnamed_addr #0 align 2
+
+attributes #0 = { noinline }
+