hoy added inline comments.
================
Comment at: llvm/include/llvm/IR/PseudoProbe.h:41
// [18:3] - probe id
- // [25:19] - reserved
+ // [25:19] - probe distribution factor
// [28:26] - probe type, see PseudoProbeType
----------------
wmi wrote:
> The bits in discriminator is a scare resource. Have you considered using less
> bits to represent probe distribution factor? I guess it is possible that
> using a little more coarse grain distribution factor won't affect performance.
That's a good point. We are using seven bits to represent [0, 100] so that
integral numbers can be distinguished. Yes, we could use fewer bits to
represent, say 4 bits to represent only even numbers. We could also not use any
bits here but instead use the distribution factor of the outer block probes
when the competition of those bits are high. I can do an experiment to see how
well that works.
================
Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:273
IRChangedPrinter PrintChangedIR;
+ PseudoProbeVerifier PseudoProbeVerification;
VerifyInstrumentation Verify;
----------------
wmi wrote:
> Before PseudoProbeUpdate pass, there is no need to verify because
> PseudoProbeUpdate will make distribution factor consistent. PseudoProbeUpdate
> run in a late stage in the lto/thinlto prelink pipeline, and no many passes
> need the verification, so what is the major usage of PseudoProbeVerifier?
Yeah, there's no need to verify intermediate passes. The verifier pass is just
a handy utility that tracks those passes that do code duplication for
debugging. Perhaps I should give it a better name like PseudoCloningTracker?
================
Comment at: llvm/lib/Transforms/IPO/SampleProfileProbe.cpp:133-134
+ float PrevProbeFactor = PrevProbeFactors[I.first];
+ if (std::abs(CurProbeFactor - PrevProbeFactor) >
+ DistributionFactorVariance) {
+ if (!BannerPrinted) {
----------------
wmi wrote:
> Why not issue warning/error message when verification fails? That will make
> enabling the verification in release compiler possible.
The verifier is for debugging only. It doesn't really do any verification. It
just helps to track code duplication. Sorry for the naming confusion.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D93264/new/
https://reviews.llvm.org/D93264
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
