[PATCH] D114565: [InstrProf] Attach debug info to counters

Ellis Hoag via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 30 15:27:37 PST 2021

ellis added inline comments.

Comment at: compiler-rt/lib/profile/InstrProfilingWriter.c:273
-  if (!DataSize)
+  if (!CountersSize)
     return 0;
kyulee wrote:
> Is it no diff change? I wonder if there is a case where `CountersSize` is 0 but not `DataSize`  from a value probe.
> If not, should we make a separate change?
> I wonder if there is a case where CountersSize is 0 but not DataSize from a value probe.
Ah, yes I believe this is possible. I'll fix this here.

Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp:851
+      addString(AnnotationDie, dwarf::DW_AT_const_value, Value->getString());
+    else if (const auto *Expr = dyn_cast<DIExpression>(ValueOp))
+      addConstantValue(
kyulee wrote:
> It checks an expression but appears to apply the constant case below. Should it check a constant expression, instead?
Oh we actually don't need to use `DIExpression` at all. Instead we can emit `ConstantAsMetadata` to simplify it quite a bit.

Comment at: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp:647
+  PD.NumValueSites[ValueKind] =
+      std::max(PD.NumValueSites[ValueKind], (uint32_t)(Index + 1));
kyulee wrote:
> It appears this is a NFC. Should we make a separate change?
Yeah I can move this out of this patch.

  rG LLVM Github Monorepo



More information about the cfe-commits mailing list