[llvm] [Instrumentation] Fix EdgeCounts vector size in SetBranchWeights (PR #99064)
Mircea Trofin via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 14 15:20:48 PDT 2024
mtrofin wrote:
> > > Anyway, I suspect it might be better to fix OutEdges in PGOUseBBInfo rather than using a different value in this one location.
> >
> >
> > If you want me to do that, you'll have to provide (unfortunately detailed) guidance, because I don't know how.
>
> Looking.
OK, here's what happens:
refer to https://github.com/llvm/llvm-project/pull/71262. We have a reason to mark certain edges like in this case as "removed". If they are, they don't get added to a node's out edges (see `PGOInstrumentation.cpp`, `setupBBInfoEdges`). The code @avikivity fixed works if the numbering of the removed edges is such that they end up at the end of the successors list.
I think we have 2 alternatives: add `Removed` edges; or @avikivity's fix. I like the latter because the former would require going over a lot of places and checking `if (E->Removed) continue;`, and missing some spots would lead to :shrug: - basically quiet/action at a distance failures, likely; vs keeping the status quo (i.e. skipping over `Removed` edges) which at least leads to prompt failures.
WDYT? @xur-llvm also.
@avikivity so all you'd need (pending other folks' feedback on the above) is to add the file @ellishg pasted above - probably place it in `llvm/test/Transforms/Coroutines` or something like that - and I'd add a comment where you get the various Sizes as to why this way (the explanation above).
Thanks for the fix, too!
https://github.com/llvm/llvm-project/pull/99064
More information about the llvm-commits
mailing list