[PATCH] D97734: [Remarks] Emit variable info in auto-init remarks

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/include/llvm/Transforms/Utils/AutoInitRemark.h:57
+  struct VariableInfo {
+    Optional<std::string> Name;
+    Optional<uint64_t> Size;
----------------
This just stores references to existing strings, right? Could it just be a StringRef?


================
Comment at: llvm/include/llvm/Transforms/Utils/AutoInitRemark.h:61
+  };
+  void inspectVariable(const Value *V, SmallVectorImpl<VariableInfo> &Result);
+  void inspectDst(Value *Dst, OptimizationRemarkMissed &R);
----------------
might be helpful to have a quick comment describing what those do?


================
Comment at: llvm/lib/Transforms/Utils/AutoInitRemark.cpp:160
+      Optional<uint64_t> DISize = getSizeInBytes(DILV->getSizeInBits());
+      VariableInfo Var{DILV->getName().str(), DISize};
+      if (!Var.isEmpty())
----------------
nit: most code in LLVM seems to use regular () for constructors.


================
Comment at: llvm/test/Transforms/Util/trivial-auto-var-init-call.ll:293
 ; CHECK-NEXT: Call to memset inserted by -ftrivial-auto-var-init. Memory operation size: 1 bytes.
+; CHECK-NEXT: Variables: dst (1 bytes).
 ; YAML-LABEL: --- !Missed
----------------
Not sure I missed a test case, but is there one with multiple variables?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97734/new/

https://reviews.llvm.org/D97734