[llvm] r309641 - Allow None as a MemoryLocation to getModRefInfo

Alina Sbirlea via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 31 17:28:29 PDT 2017


Author: asbirlea
Date: Mon Jul 31 17:28:29 2017
New Revision: 309641

URL: http://llvm.org/viewvc/llvm-project?rev=309641&view=rev
Log:
Allow None as a MemoryLocation to getModRefInfo

Summary:
Adding part of the changes in D30369 (needed to make progress):
Current patch updates AliasAnalysis and MemoryLocation, but does _not_ clean up MemorySSA.

Original summary from D30369, by dberlin:
Currently, we have instructions which affect memory but have no memory
location. If you call, for example, MemoryLocation::get on a fence,
it asserts. This means things specifically have to avoid that. It
also means we end up with a copy of each API, one taking a memory
location, one not.

This starts to fix that.

We add MemoryLocation::getOrNone as a new call, and reimplement the
old asserting version in terms of it.

We make MemoryLocation optional in the (Instruction, MemoryLocation)
version of getModRefInfo, and kill the old one argument version in
favor of passing None (it had one caller). Now both can handle fences
because you can just use MemoryLocation::getOrNone on an instruction
and it will return a correct answer.

We use all this to clean up part of MemorySSA that had to handle this difference.

Note that literally every actual getModRefInfo interface we have could be made private and replaced with:

getModRefInfo(Instruction, Optional<MemoryLocation>)
and
getModRefInfo(Instruction, Optional<MemoryLocation>, Instruction, Optional<MemoryLocation>)

and delegating to the right ones, if we wanted to.

I have not attempted to do this yet.

Reviewers: dberlin, davide, dblaikie

Subscribers: sanjoy, hfinkel, chandlerc, llvm-commits

Differential Revision: https://reviews.llvm.org/D35441

Modified:
    llvm/trunk/include/llvm/Analysis/AliasAnalysis.h
    llvm/trunk/include/llvm/Analysis/MemoryLocation.h
    llvm/trunk/lib/Analysis/MemorySSA.cpp
    llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp
    llvm/trunk/unittests/Analysis/AliasAnalysisTest.cpp

Modified: llvm/trunk/include/llvm/Analysis/AliasAnalysis.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/AliasAnalysis.h?rev=309641&r1=309640&r2=309641&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Analysis/AliasAnalysis.h (original)
+++ llvm/trunk/include/llvm/Analysis/AliasAnalysis.h Mon Jul 31 17:28:29 2017
@@ -38,6 +38,7 @@
 #ifndef LLVM_ANALYSIS_ALIASANALYSIS_H
 #define LLVM_ANALYSIS_ALIASANALYSIS_H
 
+#include "llvm/ADT/Optional.h"
 #include "llvm/Analysis/MemoryLocation.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/IR/CallSite.h"
@@ -500,43 +501,33 @@ public:
     return getModRefInfo(I, MemoryLocation(P, Size));
   }
 
-  /// Check whether or not an instruction may read or write memory (without
-  /// regard to a specific location).
+  /// Check whether or not an instruction may read or write the optionally
+  /// specified memory location.
   ///
-  /// For function calls, this delegates to the alias-analysis specific
-  /// call-site mod-ref behavior queries. Otherwise it delegates to the generic
-  /// mod ref information query without a location.
-  ModRefInfo getModRefInfo(const Instruction *I) {
-    if (auto CS = ImmutableCallSite(I)) {
-      auto MRB = getModRefBehavior(CS);
-      if ((MRB & MRI_ModRef) == MRI_ModRef)
-        return MRI_ModRef;
-      if (MRB & MRI_Ref)
-        return MRI_Ref;
-      if (MRB & MRI_Mod)
-        return MRI_Mod;
-      return MRI_NoModRef;
-    }
-
-    return getModRefInfo(I, MemoryLocation());
-  }
-
-  /// Check whether or not an instruction may read or write the specified
-  /// memory location.
-  ///
-  /// Note explicitly that getModRefInfo considers the effects of reading and
-  /// writing the memory location, and not the effect of ordering relative to
-  /// other instructions.  Thus, a volatile load is considered to be Ref,
-  /// because it does not actually write memory, it just can't be reordered
-  /// relative to other volatiles (or removed).  Atomic ordered loads/stores are
-  /// considered ModRef ATM because conservatively, the visible effect appears
-  /// as if memory was written, not just an ordering constraint.
   ///
   /// An instruction that doesn't read or write memory may be trivially LICM'd
   /// for example.
   ///
-  /// This primarily delegates to specific helpers above.
-  ModRefInfo getModRefInfo(const Instruction *I, const MemoryLocation &Loc) {
+  /// For function calls, this delegates to the alias-analysis specific
+  /// call-site mod-ref behavior queries. Otherwise it delegates to the specific
+  /// helpers above.
+  ModRefInfo getModRefInfo(const Instruction *I,
+                           const Optional<MemoryLocation> &OptLoc) {
+    if (OptLoc == None) {
+      if (auto CS = ImmutableCallSite(I)) {
+        auto MRB = getModRefBehavior(CS);
+        if ((MRB & MRI_ModRef) == MRI_ModRef)
+          return MRI_ModRef;
+        if (MRB & MRI_Ref)
+          return MRI_Ref;
+        if (MRB & MRI_Mod)
+          return MRI_Mod;
+        return MRI_NoModRef;
+      }
+    }
+
+    const MemoryLocation &Loc = OptLoc.getValueOr(MemoryLocation());
+
     switch (I->getOpcode()) {
     case Instruction::VAArg:  return getModRefInfo((const VAArgInst*)I, Loc);
     case Instruction::Load:   return getModRefInfo((const LoadInst*)I,  Loc);

Modified: llvm/trunk/include/llvm/Analysis/MemoryLocation.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/MemoryLocation.h?rev=309641&r1=309640&r2=309641&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Analysis/MemoryLocation.h (original)
+++ llvm/trunk/include/llvm/Analysis/MemoryLocation.h Mon Jul 31 17:28:29 2017
@@ -16,6 +16,7 @@
 #ifndef LLVM_ANALYSIS_MEMORYLOCATION_H
 #define LLVM_ANALYSIS_MEMORYLOCATION_H
 
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/DenseMapInfo.h"
 #include "llvm/IR/CallSite.h"
 #include "llvm/IR/Metadata.h"
@@ -68,17 +69,23 @@ public:
   static MemoryLocation get(const AtomicCmpXchgInst *CXI);
   static MemoryLocation get(const AtomicRMWInst *RMWI);
   static MemoryLocation get(const Instruction *Inst) {
-    if (auto *I = dyn_cast<LoadInst>(Inst))
-      return get(I);
-    else if (auto *I = dyn_cast<StoreInst>(Inst))
-      return get(I);
-    else if (auto *I = dyn_cast<VAArgInst>(Inst))
-      return get(I);
-    else if (auto *I = dyn_cast<AtomicCmpXchgInst>(Inst))
-      return get(I);
-    else if (auto *I = dyn_cast<AtomicRMWInst>(Inst))
-      return get(I);
-    llvm_unreachable("unsupported memory instruction");
+    return *MemoryLocation::getOrNone(Inst);
+  }
+  static Optional<MemoryLocation> getOrNone(const Instruction *Inst) {
+    switch (Inst->getOpcode()) {
+    case Instruction::Load:
+      return get(cast<LoadInst>(Inst));
+    case Instruction::Store:
+      return get(cast<StoreInst>(Inst));
+    case Instruction::VAArg:
+      return get(cast<VAArgInst>(Inst));
+    case Instruction::AtomicCmpXchg:
+      return get(cast<AtomicCmpXchgInst>(Inst));
+    case Instruction::AtomicRMW:
+      return get(cast<AtomicRMWInst>(Inst));
+    default:
+      return None;
+    }
   }
 
   /// Return a location representing the source of a memory transfer.

Modified: llvm/trunk/lib/Analysis/MemorySSA.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/MemorySSA.cpp?rev=309641&r1=309640&r2=309641&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/MemorySSA.cpp (original)
+++ llvm/trunk/lib/Analysis/MemorySSA.cpp Mon Jul 31 17:28:29 2017
@@ -1473,7 +1473,7 @@ MemoryUseOrDef *MemorySSA::createNewAcce
       return nullptr;
 
   // Find out what affect this instruction has on memory.
-  ModRefInfo ModRef = AA->getModRefInfo(I);
+  ModRefInfo ModRef = AA->getModRefInfo(I, None);
   // The isOrdered check is used to ensure that volatiles end up as defs
   // (atomics end up as ModRef right now anyway).  Until we separate the
   // ordering chain from the memory chain, this enables people to see at least

Modified: llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp?rev=309641&r1=309640&r2=309641&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp Mon Jul 31 17:28:29 2017
@@ -535,7 +535,7 @@ static bool moveUp(AliasAnalysis &AA, St
   for (auto I = --SI->getIterator(), E = P->getIterator(); I != E; --I) {
     auto *C = &*I;
 
-    bool MayAlias = AA.getModRefInfo(C) != MRI_NoModRef;
+    bool MayAlias = AA.getModRefInfo(C, MemoryLocation()) != MRI_NoModRef;
 
     bool NeedLift = false;
     if (Args.erase(C))

Modified: llvm/trunk/unittests/Analysis/AliasAnalysisTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Analysis/AliasAnalysisTest.cpp?rev=309641&r1=309640&r2=309641&view=diff
==============================================================================
--- llvm/trunk/unittests/Analysis/AliasAnalysisTest.cpp (original)
+++ llvm/trunk/unittests/Analysis/AliasAnalysisTest.cpp Mon Jul 31 17:28:29 2017
@@ -192,17 +192,17 @@ TEST_F(AliasAnalysisTest, getModRefInfo)
 
   // Check basic results
   EXPECT_EQ(AA.getModRefInfo(Store1, MemoryLocation()), MRI_Mod);
-  EXPECT_EQ(AA.getModRefInfo(Store1), MRI_Mod);
+  EXPECT_EQ(AA.getModRefInfo(Store1, None), MRI_Mod);
   EXPECT_EQ(AA.getModRefInfo(Load1, MemoryLocation()), MRI_Ref);
-  EXPECT_EQ(AA.getModRefInfo(Load1), MRI_Ref);
+  EXPECT_EQ(AA.getModRefInfo(Load1, None), MRI_Ref);
   EXPECT_EQ(AA.getModRefInfo(Add1, MemoryLocation()), MRI_NoModRef);
-  EXPECT_EQ(AA.getModRefInfo(Add1), MRI_NoModRef);
+  EXPECT_EQ(AA.getModRefInfo(Add1, None), MRI_NoModRef);
   EXPECT_EQ(AA.getModRefInfo(VAArg1, MemoryLocation()), MRI_ModRef);
-  EXPECT_EQ(AA.getModRefInfo(VAArg1), MRI_ModRef);
+  EXPECT_EQ(AA.getModRefInfo(VAArg1, None), MRI_ModRef);
   EXPECT_EQ(AA.getModRefInfo(CmpXChg1, MemoryLocation()), MRI_ModRef);
-  EXPECT_EQ(AA.getModRefInfo(CmpXChg1), MRI_ModRef);
+  EXPECT_EQ(AA.getModRefInfo(CmpXChg1, None), MRI_ModRef);
   EXPECT_EQ(AA.getModRefInfo(AtomicRMW, MemoryLocation()), MRI_ModRef);
-  EXPECT_EQ(AA.getModRefInfo(AtomicRMW), MRI_ModRef);
+  EXPECT_EQ(AA.getModRefInfo(AtomicRMW, None), MRI_ModRef);
 }
 
 class AAPassInfraTest : public testing::Test {




More information about the llvm-commits mailing list