Print OptimizedAccessType in MemorySSA if it is set

[aqjune]

Summary

This patch makes MemoryUse/MemoryDef of MemorySSA print OptimizedAccessType if they're set.

Any comments are welcome.

[george.burgess.iv]

Hi, and thanks for this patch!

I'm unsure if we want to print this out for Defs without adding a bit more information to MemoryDef::print. This is because MemoryUses and MemoryDefs both print out their defining accesses, and getOptimizedAccessType() returns information about the optimized access of a Use or Def.

For Uses, the optimized access *is* the defining access, so I think this is a great addition. For Defs, we don't really print whether or not it's optimized at all right now; it seems a bit awkward to say "the optimized Access type is MayAlias, but we're not going to tell you what the optimized access is".

So, please either back out the change to MemoryDef::print, or have MemoryDef::print print out where the Def is optimized to, if it's been optimized. I'm happy with either of these. :)

lib/Analysis/MemorySSA.cpp

1848

Hm. Does it break anything to make this function a raw_ostream &operator<<(raw_ostream &, AliasResult) that we stick into lib/Analysis/AliasAnalysis.cpp (with a declaration in include/llvm/Analysis/AliasAnalysis.cpp)?

I ask because this seems useful to just about anyone who wants to print an AliasResult. :)

1851

I suspect this isn't clang-format'ed. If not, please do so.

1858

Do we need the default case here? If not, please remove it: I'd rather have compilers warn about missing cases if we ever add something else to AliasResult (unlikely, but...)

1880

Style nit: Please fold this into the if, like so:

if (Optional<AliasResult> AR = getOptimizedAccessType()) {
  // ...
}

1921

(Same style nit)

[aqjune]

• Define << operator for AliasResult & let AliasAnalysisEvaluator.cpp use it as well
• Update printing format of MemoryDef so it prints optimized def as well

[aqjune]

Comments applied :)

[george.burgess.iv]

This LGTM after one last ultra-small nitpick is addressed. I don't see you in the commit logs, so will you need me to commit this for you?

include/llvm/Analysis/AliasAnalysis.h

95 ↗

(On Diff #150669)

Nit: I think we generally try to define these out-of-line unless their body is literally { Foo.print(OS); return OS; }. Please do so.

lib/Analysis/AliasAnalysisEvaluator.cpp

153 ↗

(On Diff #150669)

Ooh, nice :)

[george.burgess.iv]

(Sorry for the double-post; one more bit that I noticed scrolling up)

lib/Analysis/MemorySSA.cpp

1889

(This should probably go before the isOptimized check, so we're consistent with MemoryUses?)

[aqjune]

• Move definition of operator<< to AliasAnalysis.cpp
• Update format of MemoryDef print

[aqjune]

include/llvm/Analysis/AliasAnalysis.h

95 ↗

(On Diff #151094)

I was following the way operator<< is defined in Value.h but I'm not against this suggestion. I updated it

lib/Analysis/MemorySSA.cpp

1886

Makes sense, updated it

[aqjune]

Yes, you can commit this patch.

[george.burgess.iv]

This revision was not accepted when it landed; it landed in state Needs Review.

TIL phab will complain about this. To save any code archaeologists some time, this LGTM.

Committed as noted by Phab. Thanks again!

lib/Analysis/MemorySSA.cpp

if (MemoryPhi *MP = dyn_cast<MemoryPhi>(Dominatee.getUser())) {
     return locallyDominates(Dominator, cast<MemoryAccess>(Dominatee));
   }
   // If it's not a PHI node use, the normal dominates can already handle it.
   return dominates(Dominator, cast<MemoryAccess>(Dominatee.getUser()));
 }
 
 const static char LiveOnEntryStr[] = "liveOnEntry";
 
+static void printAliasResult(raw_ostream &OS, AliasResult AR) {
+  switch (AR) {
+  case NoAlias:
+  { OS << "NoAlias"; return; }
+  case MustAlias:
+  { OS << "MustAlias"; return; }
+  case MayAlias:
+  { OS << "MayAlias"; return; }
+  case PartialAlias:
+  { OS << "PartialAlias"; return; }
+  default: llvm_unreachable("invalid alias result");
+  }
+}
+
 void MemoryAccess::print(raw_ostream &OS) const {
   switch (getValueID()) {
   case MemoryPhiVal: return static_cast<const MemoryPhi *>(this)->print(OS);
   case MemoryDefVal: return static_cast<const MemoryDef *>(this)->print(OS);
   case MemoryUseVal: return static_cast<const MemoryUse *>(this)->print(OS);
   }
   llvm_unreachable("invalid value id");
 }
 
 void MemoryDef::print(raw_ostream &OS) const {
   MemoryAccess *UO = getDefiningAccess();
 
   OS << getID() << " = MemoryDef(";
   if (UO && UO->getID())
     OS << UO->getID();
   else
     OS << LiveOnEntryStr;
   OS << ')';
+  Optional<AliasResult> AR = getOptimizedAccessType();
+  if (AR) {
+    OS << " ";
+    printAliasResult(OS, *AR);
+  }
 }
 
 void MemoryPhi::print(raw_ostream &OS) const {
   bool First = true;
   OS << getID() << " = MemoryPhi(";
   for (const auto &Op : operands()) {
     BasicBlock *BB = getIncomingBlock(Op);
     MemoryAccess *MA = cast<MemoryAccess>(Op);
     if (!First)
       OS << ',';
     else
       First = false;
 

 void MemoryUse::print(raw_ostream &OS) const {
   MemoryAccess *UO = getDefiningAccess();
   OS << "MemoryUse(";
   if (UO && UO->getID())
     OS << UO->getID();
   else
     OS << LiveOnEntryStr;
   OS << ')';
+  Optional<AliasResult> AR = getOptimizedAccessType();
+  if (AR) {
+    OS << " ";
+    printAliasResult(OS, *AR);
+  }
 }
 
 void MemoryAccess::dump() const {
 // Cannot completely remove virtual function even in release mode.
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
   print(dbgs());
   dbgs() << "\n";
 #endif

test/Analysis/MemorySSA/optimize-use.ll

 ; RUN: opt -basicaa -print-memoryssa -verify-memoryssa -analyze < %s 2>&1 | FileCheck %s
 ; RUN: opt -aa-pipeline=basic-aa -passes='print<memoryssa>,verify<memoryssa>' -disable-output < %s 2>&1 | FileCheck %s
 
 ; Function Attrs: ssp uwtable
 define i32 @main() {
 entry:
-; CHECK: 1 = MemoryDef(liveOnEntry)
+; CHECK: 1 = MemoryDef(liveOnEntry) MayAlias
 ; CHECK-NEXT:   %call = call noalias i8* @_Znwm(i64 4)
   %call = call noalias i8* @_Znwm(i64 4)
   %0 = bitcast i8* %call to i32*
-; CHECK: 2 = MemoryDef(1)
+; CHECK: 2 = MemoryDef(1) MayAlias
 ; CHECK-NEXT:   %call1 = call noalias i8* @_Znwm(i64 4)
   %call1 = call noalias i8* @_Znwm(i64 4)
   %1 = bitcast i8* %call1 to i32*
-; CHECK: 3 = MemoryDef(2)
+; CHECK: 3 = MemoryDef(2) MayAlias
 ; CHECK-NEXT:   store i32 5, i32* %0, align 4
   store i32 5, i32* %0, align 4
-; CHECK: 4 = MemoryDef(3)
+; CHECK: 4 = MemoryDef(3) MayAlias
 ; CHECK-NEXT:   store i32 7, i32* %1, align 4
   store i32 7, i32* %1, align 4
-; CHECK: MemoryUse(3)
+; CHECK: MemoryUse(3) MustAlias
 ; CHECK-NEXT:   %2 = load i32, i32* %0, align 4
   %2 = load i32, i32* %0, align 4
-; CHECK: MemoryUse(4)
+; CHECK: MemoryUse(4) MustAlias
 ; CHECK-NEXT:   %3 = load i32, i32* %1, align 4
   %3 = load i32, i32* %1, align 4
-; CHECK: MemoryUse(3)
+; CHECK: MemoryUse(3) MustAlias
 ; CHECK-NEXT:   %4 = load i32, i32* %0, align 4
   %4 = load i32, i32* %0, align 4
-; CHECK: MemoryUse(4)
+; CHECK: MemoryUse(4) MustAlias
 ; CHECK-NEXT:   %5 = load i32, i32* %1, align 4
   %5 = load i32, i32* %1, align 4
   %add = add nsw i32 %3, %5
   ret i32 %add
 }
 
 declare noalias i8* @_Znwm(i64)