[PATCH] D136211: [llvm-profdata] Check for all duplicate entries in MemOpSize table

[Matthew Voss via Phabricator via llvm-commits]

ormris added inline comments.


================
Comment at: llvm/lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp:320
+    if (SeenSizeId.count(V)) {
       LLVM_DEBUG(dbgs() << "Invalid Profile Data in Function " << Func.getName()
+                        << ": Two identical values in MemOp value counts.\n");
----------------
ellis wrote:
> ormris wrote:
> > ellis wrote:
> > > Why not make this a full diagnostic error/warning? Is this something a user could encounter in the wild?
> > > Why not make this a full diagnostic error/warning?
> > 
> > Well, my understanding is that invalid profile data is usually skipped with a diagnostic. I don't want this error to be out of line with other profile data diagnostics. If something more severe is usually emitted, then we should change this error as well.
> > 
> > > Is this something a user could encounter in the wild?
> > 
> > This does occur in the wild, though it's very rare. We originally discovered this issue when a user sent us invalid bitcode which we determined had been generated by this pass.
> > This does occur in the wild, though it's very rare. We originally discovered this issue when a user sent us invalid bitcode which we determined had been generated by this pass.
> If the compiler generated invalid bitcode then IMO we should definitely emit an error. Or maybe we can turn this into an assert at least? Maybe there is something I'm missing because I'm not familiar with MemOP.
OK, that makes sense to me. I'll change it in the next update.


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

https://reviews.llvm.org/D136211