[GVN, OptDiag] Include the value that is forwarded in load elimination

[recommitting after the fix in r288307]

This requires some changes to the opt-diag API.  Hal and I have
discussed this at the Dev Meeting and came up with a streaming delimiter
(setExtraArgs) to solve this.

Arguments after this delimiter are only included in the optimization
records and not in the remarks printed in the compiler output.  (Note,
how in the test the content of the YAML file changes but the remarks on
the compiler output don't.)

This implements the green GVN message with a bug fix at line
http://lab.llvm.org:8080/artifacts/opt-view_test-suite/build/SingleSource/Benchmarks/Dhrystone/CMakeFiles/dry.dir/html/_org_test-suite_SingleSource_Benchmarks_Dhrystone_dry.c.html#L446

The fix is that now we properly include the constant value in the
message: "load of type i32 eliminated in favor of 7"

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

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@288380 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/Analysis/OptimizationDiagnosticInfo.h b/include/llvm/Analysis/OptimizationDiagnosticInfo.h
index b0653367c0cdec261b843fb7075255c0bc82b50d..fa0b3eede67ce2b6468c58daf8634ffddb94a1b9 100644 (file)
--- a/include/llvm/Analysis/OptimizationDiagnosticInfo.h
+++ b/include/llvm/Analysis/OptimizationDiagnosticInfo.h
@@ -223,6 +223,7 @@ private:
 namespace ore {
 using NV = DiagnosticInfoOptimizationBase::Argument;
 using setIsVerbose = DiagnosticInfoOptimizationBase::setIsVerbose;
+using setExtraArgs = DiagnosticInfoOptimizationBase::setExtraArgs;
 }
 
 /// OptimizationRemarkEmitter legacy analysis pass

diff --git a/include/llvm/IR/DiagnosticInfo.h b/include/llvm/IR/DiagnosticInfo.h
index e739dfbd673e2226d02983d126f799b983a956f3..a93c180df1b5e373cd0a35df45415017753a9741 100644 (file)
--- a/include/llvm/IR/DiagnosticInfo.h
+++ b/include/llvm/IR/DiagnosticInfo.h
@@ -382,6 +382,12 @@ public:
   /// \brief Used to set IsVerbose via the stream interface.
   struct setIsVerbose {};
 
+  /// \brief When an instance of this is inserted into the stream, the arguments
+  /// following will not appear in the remark printed in the compiler output
+  /// (-Rpass) but only in the optimization record file
+  /// (-fsave-optimization-record).
+  struct setExtraArgs {};
+
   /// \brief Used in the streaming interface as the general argument type.  It
   /// internally converts everything into a key-value pair.
   struct Argument {
@@ -452,6 +458,7 @@ public:
   DiagnosticInfoOptimizationBase &operator<<(StringRef S);
   DiagnosticInfoOptimizationBase &operator<<(Argument A);
   DiagnosticInfoOptimizationBase &operator<<(setIsVerbose V);
+  DiagnosticInfoOptimizationBase &operator<<(setExtraArgs EA);
 
   /// \see DiagnosticInfo::print.
   void print(DiagnosticPrinter &DP) const override;
@@ -501,6 +508,11 @@ private:
   /// The remark is expected to be noisy.
   bool IsVerbose = false;
 
+  /// \brief If positive, the index of the first argument that only appear in
+  /// the optimization records and not in the remark printed in the compiler
+  /// output.
+  int FirstExtraArgIndex = -1;
+
   friend struct yaml::MappingTraits<DiagnosticInfoOptimizationBase *>;
 };
 

diff --git a/lib/IR/DiagnosticInfo.cpp b/lib/IR/DiagnosticInfo.cpp
index 6a3fdcff233d44b70d9cf6c80baa9573e4c5d6ca..ea71fde26e0e3d5657f7bb82147777855803f34f 100644 (file)
--- a/lib/IR/DiagnosticInfo.cpp
+++ b/lib/IR/DiagnosticInfo.cpp
@@ -170,14 +170,25 @@ const std::string DiagnosticInfoWithDebugLocBase::getLocationStr() const {
     getLocation(&Filename, &Line, &Column);
   return (Filename + ":" + Twine(Line) + ":" + Twine(Column)).str();
 }
+
 DiagnosticInfoOptimizationBase::Argument::Argument(StringRef Key, Value *V)
-    : Key(Key), Val(GlobalValue::getRealLinkageName(V->getName())) {
+    : Key(Key) {
   if (auto *F = dyn_cast<Function>(V)) {
     if (DISubprogram *SP = F->getSubprogram())
       DLoc = DebugLoc::get(SP->getScopeLine(), 0, SP);
   }
   else if (auto *I = dyn_cast<Instruction>(V))
     DLoc = I->getDebugLoc();
+
+  // Only include names that correspond to user variables.  FIXME: we should use
+  // debug info if available to get the name of the user variable.
+  if (isa<llvm::Argument>(V) || isa<GlobalValue>(V))
+    Val = GlobalValue::getRealLinkageName(V->getName());
+  else if (isa<Constant>(V)) {
+    raw_string_ostream OS(Val);
+    V->printAsOperand(OS, /*PrintType=*/false);
+  } else if (auto *I = dyn_cast<Instruction>(V))
+    Val = I->getOpcodeName();
 }
 
 DiagnosticInfoOptimizationBase::Argument::Argument(StringRef Key, Type *T)
@@ -359,10 +370,19 @@ operator<<(setIsVerbose V) {
   return *this;
 }
 
+DiagnosticInfoOptimizationBase &DiagnosticInfoOptimizationBase::
+operator<<(setExtraArgs EA) {
+  FirstExtraArgIndex = Args.size();
+  return *this;
+}
+
 std::string DiagnosticInfoOptimizationBase::getMsg() const {
   std::string Str;
   raw_string_ostream OS(Str);
-  for (const DiagnosticInfoOptimizationBase::Argument &Arg : Args)
+  for (const DiagnosticInfoOptimizationBase::Argument &Arg :
+       make_range(Args.begin(), FirstExtraArgIndex == -1
+                                    ? Args.end()
+                                    : Args.begin() + FirstExtraArgIndex))
     OS << Arg.Val;
   return OS.str();
 }

diff --git a/lib/Transforms/Scalar/GVN.cpp b/lib/Transforms/Scalar/GVN.cpp
index 050cc2ecc89c27423bff7d099defef91501f990a..9a14fcb07f932e702c6c00c7b1dfea80002c093e 100644 (file)
--- a/lib/Transforms/Scalar/GVN.cpp
+++ b/lib/Transforms/Scalar/GVN.cpp
@@ -1591,10 +1591,13 @@ bool GVN::PerformLoadPRE(LoadInst *LI, AvailValInBlkVect &ValuesPerBlock,
   return true;
 }
 
-static void reportLoadElim(LoadInst *LI, OptimizationRemarkEmitter *ORE) {
+static void reportLoadElim(LoadInst *LI, Value *AvailableValue,
+                           OptimizationRemarkEmitter *ORE) {
+  using namespace ore;
   ORE->emit(OptimizationRemark(DEBUG_TYPE, "LoadElim", LI)
-            << "load of type " << ore::NV("Type", LI->getType())
-            << " eliminated");
+            << "load of type " << NV("Type", LI->getType()) << " eliminated"
+            << setExtraArgs() << " in favor of "
+            << NV("InfavorOfValue", AvailableValue));
 }
 
 /// Attempt to eliminate a load whose dependencies are
@@ -1667,7 +1670,7 @@ bool GVN::processNonLocalLoad(LoadInst *LI) {
       MD->invalidateCachedPointerInfo(V);
     markInstructionForDeletion(LI);
     ++NumGVNLoad;
-    reportLoadElim(LI, ORE);
+    reportLoadElim(LI, V, ORE);
     return true;
   }
 
@@ -1814,7 +1817,7 @@ bool GVN::processLoad(LoadInst *L) {
     patchAndReplaceAllUsesWith(L, AvailableValue);
     markInstructionForDeletion(L);
     ++NumGVNLoad;
-    reportLoadElim(L, ORE);
+    reportLoadElim(L, AvailableValue, ORE);
     // Tell MDA to rexamine the reused pointer since we might have more
     // information after forwarding it.
     if (MD && AvailableValue->getType()->getScalarType()->isPointerTy())

diff --git a/test/Transforms/GVN/opt-remarks.ll b/test/Transforms/GVN/opt-remarks.ll
index c451833f765375cd6fe94d7674c09b2487dd1c85..14c1feff4a17c0a80059cf4de27915e72e69913c 100644 (file)
--- a/test/Transforms/GVN/opt-remarks.ll
+++ b/test/Transforms/GVN/opt-remarks.ll
@@ -15,6 +15,8 @@
 ; YAML-NEXT:   - String:          'load of type '
 ; YAML-NEXT:   - Type:            i32
 ; YAML-NEXT:   - String:          ' eliminated'
+; YAML-NEXT:   - String:          ' in favor of '
+; YAML-NEXT:   - InfavorOfValue:  i
 ; YAML-NEXT: ...
 ; YAML-NEXT: --- !Passed
 ; YAML-NEXT: Pass:            gvn
@@ -24,6 +26,8 @@
 ; YAML-NEXT:   - String:          'load of type '
 ; YAML-NEXT:   - Type:            i32
 ; YAML-NEXT:   - String:          ' eliminated'
+; YAML-NEXT:   - String:          ' in favor of '
+; YAML-NEXT:   - InfavorOfValue:  '4'
 ; YAML-NEXT: ...
 ; YAML-NEXT: --- !Passed
 ; YAML-NEXT: Pass:            gvn
@@ -33,6 +37,8 @@
 ; YAML-NEXT:   - String:          'load of type '
 ; YAML-NEXT:   - Type:            i32
 ; YAML-NEXT:   - String:          ' eliminated'
+; YAML-NEXT:   - String:          ' in favor of '
+; YAML-NEXT:   - InfavorOfValue:  load
 ; YAML-NEXT: ...