[PATCH] D141579: [PGO] incorrect classof in InstrProfIncrementInst

Teresa Johnson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 12 09:50:48 PST 2023


tejohnson requested changes to this revision.
tejohnson added a comment.
This revision now requires changes to proceed.

In D141579#4047994 <https://reviews.llvm.org/D141579#4047994>, @ellis wrote:

> Ah, I see the `dyn_cast` you are talking about is here <https://github.com/llvm/llvm-project/blob/9f1bb307da6fec6fc8123bdec79ecec28fced874/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp#L548-L549>. I think this is only a problem if `InstrProfIncrementInstStep` is the only InstrProf instruction in a function **and** it has an `llvm.instrprof.value.profile` instruction, which explains why we didn't catch this bug for so long. In that case, the later call to `getOrCreateRegionCounters()` will be skipped and, according to the comment, this is required for value profiling. Is that what you observed? If so it should be easy to add a small test case, probably similar to `llvm/test/Instrumentation/InstrProfiling/early-exit.ll`, but this could be followed up in another patch. Thanks for fixing!

Why not include the suggested test in this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141579



More information about the llvm-commits mailing list