[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

paulkirth updated this revision to Diff 502875. paulkirth added a comment. Remove redunant assert Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131306/new/ https://reviews.llvm.org/D131306 Files: clang/test/CodeGenCXX/attr-likelihood-if-branch-w

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

paulkirth updated this revision to Diff 502873. paulkirth marked 4 inline comments as done. paulkirth added a comment. Rebase and address comments. - update doc comments and describe `IsExpected` parameter - add assert when scaling branch weights on calls - remove redundant check, now done in `ge

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

paulkirth added inline comments. Comment at: llvm/include/llvm/IR/MDBuilder.h:61 /// Return metadata containing two branch weights. + MDNode *createBranchWeights(uint32_t TrueWeight, uint32_t FalseWeight, tejohnson wrote: > Update comment to mention new par

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

tejohnson added a comment. Sorry for the slow response. Looks pretty good, just a few minor suggestions and questions. Comment at: llvm/include/llvm/IR/MDBuilder.h:61 /// Return metadata containing two branch weights. + MDNode *createBranchWeights(uint32_t TrueWeight, ui

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

paulkirth updated this revision to Diff 492493. paulkirth edited the summary of this revision. paulkirth added a comment. Update summary Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131306/new/ https://reviews.llvm.org/D131306 Files: clang/test

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

paulkirth updated this revision to Diff 492492. paulkirth added a comment. Rebase and update lang ref Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131306/new/ https://reviews.llvm.org/D131306 Files: clang/test/CodeGenCXX/attr-likelihood-if-bran

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

paulkirth added a comment. In D131306#4081864 , @jdoerfert wrote: > Drive by: There should be a lang ref component to this. Thanks for pointing out that we need to update the langref. Offhand, do you recall if there are places other than `BranchWeightM

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

jdoerfert added a comment. Drive by: There should be a lang ref component to this. Also, changing branch weight metadata might impact downstream users, an RFC seems in order (assuming I didn't miss one). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

paulkirth updated this revision to Diff 492246. paulkirth added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131306/new/ https://reviews.llvm.org/D131306 Files: clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp cla

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

paulkirth updated this revision to Diff 489176. paulkirth added a comment. Rebase. - fix test cases - improve code swaping branch weights in instruction.cpp - clean up some dead metadata types left over from an old version of this patch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

tejohnson added a comment. In D131306#4052878 , @paulkirth wrote: > In D131306#4052782 , @tejohnson > wrote: > >> In D131306#4037037 , @paulkirth >> wrote: >> >>> @tejoh

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

paulkirth added a comment. In D131306#4052782 , @tejohnson wrote: > In D131306#4037037 , @paulkirth > wrote: > >> @tejohnson @xur I kind of dropped the ball on these patches, but what are >> your thoughts on thi

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

tejohnson added a comment. In D131306#4037037 , @paulkirth wrote: > @tejohnson @xur I kind of dropped the ball on these patches, but what are > your thoughts on this approach over the old(more invasive) change to the > profdata format I had prototyped b

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

paulkirth added a comment. @tejohnson @xur I kind of dropped the ball on these patches, but what are your thoughts on this approach over the old(more invasive) change to the profdata format I had prototyped before? the patch will obviously need to be rebased, but other than that, do we see a do

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

paulkirth updated this revision to Diff 464332. paulkirth added a comment. rebase and change implementation to include provenance information directly in the MD_prof metadata Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131306/new/ https://review

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

xur added a comment. I agreed with Teresa: adding an optional string is better than using a separated metadata here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131306/new/ https://reviews.llvm.org/D131306 __

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

paulkirth added a comment. I think that may be a better approach. Let me investigate a bit and see what the exact tradeoffs are. If anyone else has a suggestion or thought on another way to handle this, I'm all ears. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

tejohnson added a comment. In D131306#3756153 , @paulkirth wrote: > In D131306#3756087 , @tejohnson > wrote: > >> Well I was thinking the extra field would be optional as well and could be >> removed. But unders

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

paulkirth added a comment. In D131306#3756087 , @tejohnson wrote: > Well I was thinking the extra field would be optional as well and could be > removed. But understood that this requires more changes (although maybe not > if it is optional, and after y

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

tejohnson added a comment. In D131306#3756074 , @paulkirth wrote: > In D131306#3756009 , @tejohnson > wrote: > >> @davidxl @xur for review and thoughts. >> >> I'm a little wary of requiring that both pieces of me

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

paulkirth added a comment. In D131306#3756009 , @tejohnson wrote: > @davidxl @xur for review and thoughts. > > I'm a little wary of requiring that both pieces of metadata be carried > together, as that seems very brittle to maintain in the compiler. Wh

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

tejohnson added subscribers: xur, davidxl. tejohnson added a comment. @davidxl @xur for review and thoughts. I'm a little wary of requiring that both pieces of metadata be carried together, as that seems very brittle to maintain in the compiler. What would happen if the MD_expected didn't get p

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

paulkirth created this revision. paulkirth added a reviewer: tejohnson. Herald added subscribers: ormris, okura, kuter, hiraditya. Herald added a project: All. paulkirth requested review of this revision. Herald added a reviewer: jdoerfert. Herald added a reviewer: sstefan1. Herald added projects: