MemorySSA: Revert r269678 and r268068; replace with special casing in MemorySSA.

[pcc]

Summary

It turns out that too many passes are relying on alias analysis results
for control dependencies. Until we fix that by introducing a more accurate
modelling of control dependencies, special case assume in MemorySSA instead.

Also introduce tests to ensure we don't regress the FunctionAttrs or LICM
passes.

[• dberlin]

LGTM

[• dberlin]

(and thanks for taking the time to do this)

[aaboud]

llvm/trunk/test/Transforms/LICM/assume.ll

17 ↗

(On Diff #58566)

I am not sure what this test it checking?
Can you explain it ?

Thanks

[pcc]

llvm/trunk/test/Transforms/LICM/assume.ll

17 ↗

(On Diff #58566)

This test is checking that the licm pass does not remove calls to llvm.assume when hoisting code out of a loop. This was one of the regressions caused by r268068.

lib/Analysis/BasicAliasAnalysis.cpp

if (const Function *F = CS.getCalledFunction())
       Min =
           FunctionModRefBehavior(Min & getBestAAResults().getModRefBehavior(F));
 
   return Min;
 }
 
 /// Returns the behavior when calling the given function. For use when the call
 /// site is not known.
-/// NOTE: Because of the special case handling of llvm.assume below, the result
-/// of this function may not match similar results derived from function
-/// attributes (e.g. "readnone").
 FunctionModRefBehavior BasicAAResult::getModRefBehavior(const Function *F) {
   // If the function declares it doesn't access memory, we can't do better.
   if (F->doesNotAccessMemory())
     return FMRB_DoesNotAccessMemory;
 
-  // While the assume intrinsic is marked as arbitrarily writing so that
-  // proper control dependencies will be maintained, it never aliases any
-  // actual memory locations.
-  if (F->getIntrinsicID() == Intrinsic::assume)
-    return FMRB_DoesNotAccessMemory;
-
   FunctionModRefBehavior Min = FMRB_UnknownModRefBehavior;
 
   // If the function declares it only reads memory, go with that.
   if (F->onlyReadsMemory())
     Min = FMRB_OnlyReadsMemory;
 
   if (F->onlyAccessesArgMemory())
     Min = FunctionModRefBehavior(Min & FMRB_OnlyAccessesArgumentPointees);

lib/Transforms/Utils/MemorySSA.cpp

for (auto &BB : F)
     if (!Visited.count(&BB))
       markUnreachableAsLiveOnEntry(&BB);
 
   return Walker;
 }
 
 /// \brief Helper function to create new memory accesses
 MemoryUseOrDef *MemorySSA::createNewAccess(Instruction *I) {
+  // The assume intrinsic has a control dependency which we model by claiming
+  // that it writes arbitrarily. Ignore that fake memory dependency here.
+  // FIXME: Replace this special casing with a more accurate modelling of
+  // assume's control dependency.
+  if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(I))
+    if (II->getIntrinsicID() == Intrinsic::assume)
+      return nullptr;
+
   // Find out what affect this instruction has on memory.
   ModRefInfo ModRef = AA->getModRefInfo(I);
   bool Def = bool(ModRef & MRI_Mod);
   bool Use = bool(ModRef & MRI_Ref);
 
   // It's possible for an instruction to not modify memory at all. During
   // construction, we ignore them.
   if (!Def && !Use)

test/Transforms/FunctionAttrs/assume.ll

+; RUN: opt -S -o - -functionattrs %s | FileCheck %s
+
+; CHECK-NOT: readnone
+declare void @llvm.assume(i1)

test/Transforms/LICM/assume.ll

+; RUN: opt -licm -basicaa < %s -S | FileCheck %s
+
+define void @f(i1 %p) nounwind ssp {
+entry:
+  br label %for.body
+
+for.body:
+  br i1 undef, label %if.then, label %for.cond.backedge
+
+for.cond.backedge:
+  br i1 undef, label %for.end104, label %for.body
+
+if.then:
+  br i1 undef, label %if.then27, label %if.end.if.end.split_crit_edge.critedge
+
+if.then27:
+; CHECK: tail call void @llvm.assume
+  tail call void @llvm.assume(i1 %p)
+  br label %for.body61.us
+
+if.end.if.end.split_crit_edge.critedge:
+  br label %for.body61
+
+for.body61.us:
+  br i1 undef, label %for.cond.backedge, label %for.body61.us
+
+for.body61:
+  br i1 undef, label %for.cond.backedge, label %for.body61
+
+for.end104:
+  ret void
+}
+
+declare void @llvm.assume(i1)

test/Transforms/Util/MemorySSA/assume.ll

 ; RUN: opt -basicaa -print-memoryssa -verify-memoryssa -analyze < %s 2>&1 | FileCheck %s
 ;
 ; Ensures that assumes are treated as not reading or writing memory.
 
 declare void @llvm.assume(i1)
 
 define i32 @foo(i32* %a, i32* %b, i1 %c) {
 ; CHECK: 1 = MemoryDef(liveOnEntry)
 ; CHECK-NEXT: store i32 4
   store i32 4, i32* %a, align 4
+; CHECK-NOT: MemoryDef
 ; CHECK: call void @llvm.assume
   call void @llvm.assume(i1 %c)
 ; CHECK: MemoryUse(1)
 ; CHECK-NEXT: %1 = load i32
   %1 = load i32, i32* %a, align 4
   ret i32 %1
 }