- User Since
- May 12 2020, 12:18 PM (123 w, 6 d)
Fri, Sep 16
Tue, Sep 6
@davidxl Gentle ping :)
Mon, Aug 29
Thank you all for the help!
@paulkirth, your earlier comments really helps and I'm improving the code now.
@davidxl, I also found it's difficult to add BFI in SimplifyCFG and may I ask could you please help me check the high level of this patch and whether this updating method makes sense?
@nikic, anyway thank you for the suggestions. I think it looks harder than JumpThreading because our final goal is to keep block frequency the same after CFG change. Jumpthreading knows BFI and need to update branch(just like already know the result and want to find the reason), but SimplifyCFG cannot use BFI so it's like doing something in a reverse manner. That's just my guess.
Aug 25 2022
Hi @paulkirth, I summarized all the issues about BFI and bring it up in discourse: https://discourse.llvm.org/t/is-that-blockfrequencyinfo-can-only-be-used-with-profile-data/64855. By David's response, I think I can start to try to add BFI in SimplifyCFG to see if it's a feasible way.
Aug 24 2022
Aug 23 2022
Hi @paulkirth, sorry for the late reply. If I understand it correctly, BlockFrequencyInfo can only be used with profile information. I have found the original author of updateBlockFreqAndEdgeWeight(https://reviews.llvm.org/D10979 and https://reviews.llvm.org/D15519), that's why the author added if (HasProfileData) and only do the fix when have profile data.
However, this code is 7 years ago and we added __builtin_expect_with_probability in 2020. This probability issue(that example at the beginning of my description) can also happen even without profile data, the user will find the probability of executing bar is not expected. So it’s not sufficient for us to only do this with profile data. This is our concern about using BlockFrequencyInfo.
And again thank you very much for the kind and useful comments! I think all of them are reasonable and is working on it.
Aug 20 2022
Hi Paul, thank you very much for the suggestions! I'm working on improving the code according to your comments.
For your question about JumpThreading, since I'm not an expert on this, here is my understeading:
(1) I agree that using blockfrequency(like the function that nikic suggested) is the best way to solve this issue. I was trying to use BlockFrequencyInfo at the beginning but I found this information is not maintained and no otherwhere else use this in SimplifyCFG. May I ask what do we usually do for this situation? Is that adding this in SimplifyCFG?
(2) Jumpthreading seems is doing the same thing as FoldCondBranchOnValueKnownInPredecessor in SimplifyCFG, may I ask do you know(or someone else know) why we need both of them? And jumpthreading seems is not in -O1 optimization. I guess if we can use jumpthreading, this issue would not exist at the beginning.
Any way, thank you for the comments!
Aug 19 2022
Gentle ping :) @nikic
Aug 14 2022
Hello guys, sorry for bothering again. May I ask could anyone help me look at this change or give suggestions to my previous question?
Because fixing this issue is important to us. Thank you very much!
Aug 11 2022
Hello : )
Aug 9 2022
Hello, may I ask do anyone have comments on this change? or any suggestions of fixing this in better ways? Thanks!
Aug 5 2022
Jul 9 2020
Thank you very much!
Hi, could you please help me commit this change? Thank you very much!
Jul 8 2020
Thank you for the new comment. I've modified my test cases
(1) improve test case
@erichkeane Thank you very much for the comments. I have added test case to make sure it is evaluated during compile time. Previous code could not pass these test cases.
(1) improve test case
Jul 7 2020
Jun 23 2020
Fixed. If it looks good to you, could you please help me commit it? Thank you very much!
Jun 22 2020
Thank you very much for fixing! It looks good to me, while it seems to fail build for some other reason..
Thank you very much!
Hi, I do not have access to commit, could anybody help me to commit it? Thank you!
Jun 16 2020
@rsmith Hi, could you please take a look at my revision since last comment? Thank you!
Jun 15 2020
Added: fix comment
(1) fix minor variable name and doc word
(1) minor change of assertion and cast
For clang side is there any suggestions? Thank you!
Jun 9 2020
(1) improve code in LowerExpectIntrinsic
(2) update and simplify test
Jun 8 2020
(1) update llvm side test for intrinsic llvm.expect.with.probability, which mimics test for llvm.expect
Jun 6 2020
(1) updated llvm.expect.with.probability intrinsic document in LangRef.rst
(1) convert argument to IEEEdouble for different target
(2) update tests, add edge cases and llvm test
(3) minor change
Jun 4 2020
Jun 3 2020
(1) fix bug of range checking in SemaChecking
Jun 1 2020
May 28 2020
(1) remove redundant "else" according to coding standard
(2) change a few document words
May 26 2020
Fixed in latest update as well as adding test. Thank you!
(1) add Sema test
(2) improve assert
(3) format change
May 21 2020
- add assertion after evaluate probability
- remove value dependent check in SemaChecking
- add test case handling value-dependent template
May 15 2020
(1) add documents about __builtin_expect_with_probability
(2) modify SemaChecking to handle evaluate constant floating-point expression
(3) modify CGBuiltin to evaluate constant floating-point argument before passing it
May 14 2020
May 13 2020
- fix code format
- move probability type and value check from codegen to semacheck
- update patch with full context