GVN-hoist: disable hoisting stores past calls (PR30499)

[sebpop]

Summary

MemoryLocation::get() does not support CallInst and it errors with llvm_unreachable("unsupported memory instruction");
The current patch avoids calling get() with CallInst.

[• dberlin]

It also will not work for InvokeInst.

This is what the whole MemoryLocOrCall stuff does in MemorySSA.cpp.
You also need to handle fenceinst, etc

You can resolve this by using getModRefInfo, which has versions for calls and not calls

Just explose MemoryLocOrCall and InstructionClobbersQuery from MemorySSA for now and use that.

[hfinkel]

llvm/lib/Transforms/Scalar/GVNHoist.cpp

368

You could do something like:

if (AA->getModRef(Insn, DefLoc) != MRI_NoModRef)
  return true;

and that should handle calls.

llvm/lib/Transforms/Scalar/GVNHoist.cpp

bool hasMemoryUseOnPath(const Instruction *NewPt, MemoryDef *Def, const BasicBlock *BB) {
     const MemorySSA::AccessList *Acc = MSSA->getBlockAccesses(BB);
     if (!Acc)
       return false;
 
     for (const MemoryAccess &MA : *Acc) {
       auto *MU = dyn_cast<MemoryUse>(&MA);
       if (!MU)
         continue;
+      Instruction *Insn = MU->getMemoryInst();
 
       // Do not check whether MU aliases Def when MU occurs after OldPt.
-      if (BB == OldBB && firstInBB(OldPt, MU->getMemoryInst()))
+      if (BB == OldBB && firstInBB(OldPt, Insn))
         break;
 
       // Do not check whether MU aliases Def when MU occurs before NewPt.
       if (BB == NewBB) {
         if (!ReachedNewPt) {
-          if (firstInBB(MU->getMemoryInst(), NewPt))
+          if (firstInBB(Insn, NewPt))
             continue;
           ReachedNewPt = true;
         }
       }
 
-      if (!AA->isNoAlias(DefLoc, MemoryLocation::get(MU->getMemoryInst())))
+      // Do not hoist stores past calls because MemoryLocation::get() does not
+      // work for CallInst.
+      if (isa<CallInst>(Insn))
+        return true;
+
+      if (!AA->isNoAlias(DefLoc, MemoryLocation::get(Insn)))
         return true;
     }
 
     return false;
   }
 
   // Return true when there are exception handling or loads of memory Def
   // between Def and NewPt.  This function is only called for stores: Def is

llvm/test/Transforms/GVNHoist/pr30499.ll

+; RUN: opt -S -gvn-hoist < %s
+
+define void @_Z3fn2v() #0 {
+entry:
+  %a = alloca i8*, align 8
+  %b = alloca i32, align 4
+  %0 = load i8*, i8** %a, align 8
+  store i8 0, i8* %0, align 1
+  %1 = load i32, i32* %b, align 4
+  %tobool = icmp ne i32 %1, 0
+  br i1 %tobool, label %if.then, label %if.end
+
+if.then:                                          ; preds = %entry
+  %call = call i64 @_Z3fn1v() #2
+  %conv = trunc i64 %call to i32
+  store i32 %conv, i32* %b, align 4
+  br label %if.end
+
+if.end:                                           ; preds = %if.then, %entry
+  %2 = load i8*, i8** %a, align 8
+  store i8 0, i8* %2, align 1
+  ret void
+}
+
+; Function Attrs: nounwind readonly
+declare i64 @_Z3fn1v() #1
+
+attributes #0 = { nounwind "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { nounwind readonly "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #2 = { nounwind readonly }
+