[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