Fix PR45442: Bail out when MemorySSA information is not available

[hiraditya]

Self approving as it is a trivial change and fixes PR45442

[fhahn]

llvm/lib/Transforms/Scalar/GVNHoist.cpp

525

IMO I think it would preferable to check at the call site where we get the MemoryUseOrDef and not call safeToHoistLdSt without MemoryUseOrDef.

llvm/test/Transforms/GVNHoist/pr45442.ll

22

nit: branch on undef is UB, so it is probably best to avoid it and use a i1 parameter or something instead

[hiraditya]

llvm/lib/Transforms/Scalar/GVNHoist.cpp

525

Sure, i'll do that.

llvm/test/Transforms/GVNHoist/pr45442.ll

22

Makes sense, i'll amend the testcase. The test case was reported in the PR so i added as is. Thanks for pointing out.

[fhahn]

llvm/test/Transforms/GVNHoist/pr45442.ll

22

No worries! Also might be good to replace the unreachable with ret void, if it still reproduces the crash.

[hiraditya]

Updated test cases.

[fhahn]

Thanks, LGTM although the test appease to still use branch on undef. Would be good to adjust before committing.

llvm/lib/Transforms/Scalar/GVNHoist.cpp

614

nit: auto * T?

[hiraditya]

Fixed the bug in: https://bugs.llvm.org/show_bug.cgi?id=45442

llvm/lib/Transforms/Scalar/GVNHoist.cpp

private:
   // Return true when it is safe to hoist a memory load or store U from OldPt
   // to NewPt.
   bool safeToHoistLdSt(const Instruction *NewPt, const Instruction *OldPt,
                        MemoryUseOrDef *U, InsKind K, int &NBBsOnAllPaths) {
     // In place hoisting is safe.
     if (NewPt == OldPt)
       return true;
 
+    // MemoryUseDef information is not available, bail out.
+    if (!U)
+      return false;
+
     const BasicBlock *NewBB = NewPt->getParent();
     const BasicBlock *OldBB = OldPt->getParent();
     const BasicBlock *UBB = U->getBlock();
 
     // Check for dependences on the Memory SSA.
     MemoryAccess *D = U->getDefiningAccess();
     BasicBlock *DBB = D->getBlock();
     if (DT->properlyDominates(NewBB, DBB))
for (auto CHI : C) {
       if (!Insn) // No instruction was inserted in this CHI.
         continue;
       if (K == InsKind::Scalar) {
         if (safeToHoistScalar(BB, Insn->getParent(), NumBBsOnAllPaths))
           Safe.push_back(CHI);
       } else {
         MemoryUseOrDef *UD = MSSA->getMemoryAccess(Insn);
         if (safeToHoistLdSt(BB->getTerminator(), Insn, UD, K, NumBBsOnAllPaths))
           Safe.push_back(CHI);
       }
     }
   }
 
   using RenameStackType = DenseMap<VNType, SmallVector<Instruction *, 2>>;
 
   // Push all the VNs corresponding to BB into RenameStack.
   void fillRenameStack(BasicBlock *BB, InValuesType &ValueBBs,

llvm/test/Transforms/GVNHoist/pr45442.ll

+; RUN: opt < %s -gvn-hoist -S | FileCheck %s
+
+; gvn-hoist shouldn't crash in this case.
+; CHECK-LABEL: @func()
+; CHECK:       entry:
+; CHECK-NEXT:  br i1
+; CHECK-NEXT:  bb1
+; CHECK-NEXT:  unreachable
+; CHECK-NEXT:  bb2:
+; CHECK-NEXT:  call
+; CHECK-NEXT:  call
+; CHECK-NEXT:  unreachable
+
+define void @v_1_0() #0 {
+entry:
+  ret void
+}
+
+define void @func()  {
+entry:
+  br i1 undef, label %bb1, label %bb2
+
+bb1:
+  unreachable
+
+bb2:
+  call void @v_1_0()
+  call void @v_1_0()
+  unreachable
+}
+
+attributes #0 = { nounwind readonly }