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

[Francis Visoiu Mistrih via Phabricator via llvm-commits]

thegameg added inline comments.


================
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())
----------------
fhahn wrote:
> nit: most code in LLVM seems to use regular () for constructors.
I use it mostly so I don't have to add a constructor, but I can add one if needed.


================
Comment at: llvm/lib/Transforms/Utils/AutoInitRemark.cpp:201
+  R << "\nVariables: ";
+  for (uint32_t i = 0; i < VIs.size(); ++i) {
+    const VariableInfo &VI = VIs[i];
----------------
paquette wrote:
> Why `uint32_t`?
🤷🏻‍♂️


================
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
----------------
fhahn wrote:
> Not sure I missed a test case, but is there one with multiple variables?
>From `known_call_with_size_alloca_phi` to the end of the file they should all be with multiple variables.


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

https://reviews.llvm.org/D97734