[PATCH] D25542: GVN-hoist: avoid calling MemoryLocation::get() on a call (PR30499)

[Sebastian Pop via llvm-commits]

On Wed, Oct 12, 2016 at 10:05 PM, Daniel Berlin <dberlin at dberlin.org> wrote:
> There is actually a much simpler fix you sholdl use.
>
> return instructionClobbersQuery(MD, MU MemoryLocOrCall(MU), AA);
>
> You need to move that version of instructionClobbersQuery up, but it was
> built to always do the right thing.
>
> Also note it will properly handle a case you are not: Fences do not have
> memorylocations, and calling get on them will also crash.
>

Right. I will commit the attached patch after testing.

Thanks,
Sebastian
-------------- next part --------------
diff --git a/lib/Transforms/Utils/MemorySSA.cpp b/lib/Transforms/Utils/MemorySSA.cpp
index abcfc5a..c48c460 100644
--- a/lib/Transforms/Utils/MemorySSA.cpp
+++ b/lib/Transforms/Utils/MemorySSA.cpp
@@ -254,16 +254,23 @@ static bool instructionClobbersQuery(MemoryDef *MD,
   return AA.getModRefInfo(DefInst, UseLoc) & MRI_Mod;
 }
 
+static bool instructionClobbersQuery(MemoryDef *MD, const MemoryUseOrDef *MU,
+                                     const MemoryLocOrCall &UseMLOC,
+                                     AliasAnalysis &AA) {
+  // FIXME: This is a temporary hack to allow a single instructionClobbersQuery
+  // to exist while MemoryLocOrCall is pushed through places.
+  if (UseMLOC.IsCall)
+    return instructionClobbersQuery(MD, MemoryLocation(), MU->getMemoryInst(),
+                                    AA);
+  return instructionClobbersQuery(MD, UseMLOC.getLoc(), MU->getMemoryInst(),
+                                  AA);
+}
+
 // Return true when MD may alias MU, return false otherwise.
 bool defClobbersUseOrDef(MemoryDef *MD, const MemoryUseOrDef *MU,
                          AliasAnalysis &AA) {
   Instruction *UseInst = MU->getMemoryInst();
-  MemoryLocation UseLoc;
-  if (ImmutableCallSite(UseInst))
-    UseLoc = MemoryLocation();
-  else
-    UseLoc = MemoryLocation::get(UseInst);
-  return instructionClobbersQuery(MD, UseLoc, UseInst, AA);
+  return instructionClobbersQuery(MD, MU, MemoryLocOrCall(UseInst), AA);
 }
 }
 
@@ -315,18 +322,6 @@ static bool isUseTriviallyOptimizableToLiveOnEntry(AliasAnalysis &AA,
                               AA.pointsToConstantMemory(I));
 }
 
-static bool instructionClobbersQuery(MemoryDef *MD, MemoryUse *MU,
-                                     const MemoryLocOrCall &UseMLOC,
-                                     AliasAnalysis &AA) {
-  // FIXME: This is a temporary hack to allow a single instructionClobbersQuery
-  // to exist while MemoryLocOrCall is pushed through places.
-  if (UseMLOC.IsCall)
-    return instructionClobbersQuery(MD, MemoryLocation(), MU->getMemoryInst(),
-                                    AA);
-  return instructionClobbersQuery(MD, UseMLOC.getLoc(), MU->getMemoryInst(),
-                                  AA);
-}
-
 /// Cache for our caching MemorySSA walker.
 class WalkerCache {
   DenseMap<ConstMemoryAccessPair, MemoryAccess *> Accesses;