[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)
> 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))
> 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));
> 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
CHANGES SINCE LAST ACTION
More information about the cfe-commits