[MemorySSA] Don't check def set when cloning memoryaccesses

[StephenFan]

Summary

For internal functions, globals-aa returns different ModRefInfo
results if they are inlined and are no longer called by external
functions. This causes an assertion failure when cloning memoryssa
accesses.

Fixes: https://github.com/llvm/llvm-project/issues/59546

[nikic]

llvm/lib/Analysis/MemorySSA.cpp

1764

We could do something like assert(Def == DefCheck || !DefCheck), to check that we only reduce, but not increase possible accesses. (Though I suspect that we'll hit cases like that as well...)

llvm/test/Transforms/SimpleLoopUnswitch/pr59546.ll

42

Is this needed to prevent inlining? Can we mark it noinline instead?

[asbirlea]

llvm/lib/Analysis/MemorySSA.cpp

1764

+1 to add this check now.
If we encounter the opposite it'll be worth understanding how something became obfuscated enough to miss that the instruction cannot write.

[StephenFan]

Add an assertion.

[StephenFan]

Replace attribute to noinline.

[nikic]

LG

llvm/lib/Analysis/MemorySSA.cpp

1764

Can you please add () around the expression? I think it still does the right thing in the end, but mostly by accident. It will evaluate Def == DefCheck || (!DefCheck && "")

llvm/lib/Analysis/MemorySSA.cpp

MemoryUseOrDef *MemorySSA::createNewAccess(Instruction *I,
   if (Template) {
     Def = isa<MemoryDef>(Template);
     Use = isa<MemoryUse>(Template);
 #if !defined(NDEBUG)
     ModRefInfo ModRef = AAP->getModRefInfo(I, std::nullopt);
     bool DefCheck, UseCheck;
     DefCheck = isModSet(ModRef) || isOrdered(I);
     UseCheck = isRefSet(ModRef);
-    // Use set is not checked since AA may return better results as a result of
-    // other transforms.
-    // FIXME: Would Def value always be consistent after transforms?
-    assert(Def == DefCheck && "Invalid template");
+    // Memory accesses should only be reduced and can not be increased since AA
+    // just might return better results as a result of some transformations.
+    assert((Def == DefCheck || !DefCheck) &&
+           "Memory accesses should only be reduced");
     if (!Def && Use != UseCheck) {
       // New Access should not have more power than template access
       assert(!UseCheck && "Invalid template");
     }
 #endif
   } else {
     // Find out what affect this instruction has on memory.
     ModRefInfo ModRef = AAP->getModRefInfo(I, std::nullopt);

llvm/test/Transforms/SimpleLoopUnswitch/pr59546.ll

+; RUN: opt -passes="scc-oz-module-inliner,function(loop-mssa(no-op-loop)),recompute-globalsaa,function(loop-mssa(simple-loop-unswitch<nontrivial>))" -disable-output < %s
+; Check that don't crash if the Alias Analysis returns better results than
+; before when cloning loop's memoryssa.
+
+@a = internal global i16 0
+
+define void @h() {
+entry:
+  br label %end
+
+body:                                       ; No predecessors!
+  call void @g(ptr null)
+  br label %end
+
+end:                                        ; preds = %while.body, %entry
+  ret void
+}
+
+define internal void @g(ptr %a) #0 {
+entry:
+  br label %while.cond
+
+while.cond:                                       ; preds = %while.body, %entry
+  %0 = load i16, ptr %a, align 1
+  %tobool.not = icmp eq i16 %0, 0
+  br i1 %tobool.not, label %while.end, label %while.body
+
+while.body:                                       ; preds = %while.cond
+  call void @f()
+  br label %while.cond
+
+while.end:                                        ; preds = %while.cond
+  ret void
+}
+
+define internal void @f() #0 {
+  store i16 0, ptr @a, align 1
+  ret void
+}
+
+attributes #0 = { noinline }