[PATCH] D26488: [GVN] Basic optimization remark support

[Adam Nemet via llvm-commits]

anemet added a comment.

Hi Davide,

In https://reviews.llvm.org/D26488#591495, @davide wrote:

> You're mentioning that more interesting cases will be added in later patches, can you please elaborate what else do you have in mind (other than load elim?)


Currently it is load-elim in patch 2 and 3.  There are probably other interesting things we can do with load-pre but I don't have those implemented.  Also not directly GVN-specific, but it would also be nice to somehow expose the aliasing information via opt-viewer.  (Let me know if you're interested working on any of this.)



================
Comment at: lib/IR/DiagnosticInfo.cpp:183-188
+DiagnosticInfoOptimizationBase::Argument::Argument(StringRef Key, Type *T)
+    : Key(Key) {
+  raw_string_ostream OS(Val);
+  OS << *T;
+}
+
----------------
davide wrote:
> I assume you're using this but it's not strictly tied to the patch, i.e. maybe can be committed separately? (up to you)
Yeah the first use is in this patch and this allows writing the test case for the feature.


================
Comment at: lib/Transforms/Scalar/GVN.cpp:1807-1808
     ++NumGVNLoad;
+    DEBUG(
+        dbgs() << "GVN: load eliminated: " << *L << "\n");
+    ORE->emit(OptimizationRemark(DEBUG_TYPE, "LoadElim", L)
----------------
davide wrote:
> I see here you're adding this `DEBUG` statement before emitting the remark but you don't in the other two cases. Is this intended?
Thanks for bringing this up, I haven't so far thought about the relationship of debug output and opt remarks.  I think that we shouldn't have debug output if we already have opt remarks.  It makes the code less readable and with "opt -pass-remarks*=" you can get them printed.

Let me remove this.

Perhaps in the long run we can imply -pass-remarks*=<pass> if "-debug-only=<pass>" is used.


https://reviews.llvm.org/D26488