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.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114565/new/
https://reviews.llvm.org/D114565
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
