Page MenuHomePhabricator

LukeZhuang (Zhi Zhuang)
User

Projects

User does not belong to any projects.

User Details

User Since
May 12 2020, 12:18 PM (123 w, 6 d)

Recent Activity

Fri, Sep 16

LukeZhuang added inline comments to D131287: Fix branch weight in FoldCondBranchOnValueKnownInPredecessor pass in SimplifyCFG.
Fri, Sep 16, 8:13 AM · Restricted Project, Restricted Project
LukeZhuang updated the diff for D131287: Fix branch weight in FoldCondBranchOnValueKnownInPredecessor pass in SimplifyCFG.

Sorry for the late reply. @paulkirth @davidxl I just updated the patch according to your comments

Fri, Sep 16, 8:12 AM · Restricted Project, Restricted Project

Tue, Sep 6

LukeZhuang added a comment to D131287: Fix branch weight in FoldCondBranchOnValueKnownInPredecessor pass in SimplifyCFG.

@davidxl Gentle ping :)

Tue, Sep 6, 7:25 AM · Restricted Project, Restricted Project

Mon, Aug 29

LukeZhuang updated subscribers of D131287: Fix branch weight in FoldCondBranchOnValueKnownInPredecessor pass in SimplifyCFG.
Mon, Aug 29, 3:18 PM · Restricted Project, Restricted Project
LukeZhuang updated subscribers of D131287: Fix branch weight in FoldCondBranchOnValueKnownInPredecessor pass in SimplifyCFG.

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.

Mon, Aug 29, 11:07 AM · Restricted Project, Restricted Project

Aug 25 2022

LukeZhuang added a comment to D131287: Fix branch weight in FoldCondBranchOnValueKnownInPredecessor pass in SimplifyCFG.

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 25 2022, 2:00 PM · Restricted Project, Restricted Project

Aug 24 2022

LukeZhuang removed reviewers for D131287: Fix branch weight in FoldCondBranchOnValueKnownInPredecessor pass in SimplifyCFG: lebedev.ri, lattner, danielcdh, manmanren, aeubanks, eraman, atrick, samparker, kazu, dmgreen, congh.
Aug 24 2022, 1:07 PM · Restricted Project, Restricted Project
LukeZhuang added inline comments to D131287: Fix branch weight in FoldCondBranchOnValueKnownInPredecessor pass in SimplifyCFG.
Aug 24 2022, 8:35 AM · Restricted Project, Restricted Project

Aug 23 2022

LukeZhuang added a reviewer for D131287: Fix branch weight in FoldCondBranchOnValueKnownInPredecessor pass in SimplifyCFG: congh.
Aug 23 2022, 5:13 PM · Restricted Project, Restricted Project
LukeZhuang added a comment to D131287: Fix branch weight in FoldCondBranchOnValueKnownInPredecessor pass in SimplifyCFG.

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 23 2022, 5:13 PM · Restricted Project, Restricted Project

Aug 20 2022

LukeZhuang added a comment to D131287: Fix branch weight in FoldCondBranchOnValueKnownInPredecessor pass in SimplifyCFG.

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 20 2022, 7:23 PM · Restricted Project, Restricted Project

Aug 19 2022

LukeZhuang added inline comments to D131287: Fix branch weight in FoldCondBranchOnValueKnownInPredecessor pass in SimplifyCFG.
Aug 19 2022, 7:06 AM · Restricted Project, Restricted Project
LukeZhuang added reviewers for D131287: Fix branch weight in FoldCondBranchOnValueKnownInPredecessor pass in SimplifyCFG: RKSimon, samparker, kazu, davidxl, dmgreen, paulkirth.

Gentle ping :) @nikic

Aug 19 2022, 6:55 AM · Restricted Project, Restricted Project

Aug 14 2022

LukeZhuang added a comment to D131287: Fix branch weight in FoldCondBranchOnValueKnownInPredecessor pass in SimplifyCFG.

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 14 2022, 4:32 PM · Restricted Project, Restricted Project

Aug 11 2022

LukeZhuang added a comment to D131287: Fix branch weight in FoldCondBranchOnValueKnownInPredecessor pass in SimplifyCFG.

Hello : )

Aug 11 2022, 6:13 AM · Restricted Project, Restricted Project

Aug 9 2022

LukeZhuang added a comment to D131287: Fix branch weight in FoldCondBranchOnValueKnownInPredecessor pass in SimplifyCFG.

This is a jump threading optimization -- is it possible to reuse the branch weight adjustment code from the JumpThreading pass? The updateBlockFreqAndEdgeWeight() method looks relevant.

Aug 9 2022, 11:37 AM · Restricted Project, Restricted Project
LukeZhuang added a comment to D131287: Fix branch weight in FoldCondBranchOnValueKnownInPredecessor pass in SimplifyCFG.

Hello, may I ask do anyone have comments on this change? or any suggestions of fixing this in better ways? Thanks!

Aug 9 2022, 9:03 AM · Restricted Project, Restricted Project

Aug 5 2022

LukeZhuang requested review of D131287: Fix branch weight in FoldCondBranchOnValueKnownInPredecessor pass in SimplifyCFG.
Aug 5 2022, 1:19 PM · Restricted Project, Restricted Project

Jul 9 2020

LukeZhuang added a comment to D83362: Fix warning caused by __builtin_expect_with_probability was not handled in places such as constant folding.

Thank you very much!

Jul 9 2020, 8:08 AM · Restricted Project
LukeZhuang added a comment to D83362: Fix warning caused by __builtin_expect_with_probability was not handled in places such as constant folding.

Hi, could you please help me commit this change? Thank you very much!

Jul 9 2020, 7:02 AM · Restricted Project

Jul 8 2020

LukeZhuang added a comment to D83362: Fix warning caused by __builtin_expect_with_probability was not handled in places such as constant folding.

Thank you for the new comment. I've modified my test cases

Jul 8 2020, 2:24 PM · Restricted Project
LukeZhuang updated the diff for D83362: Fix warning caused by __builtin_expect_with_probability was not handled in places such as constant folding.

updated: 07/08/2020
(1) improve test case

Jul 8 2020, 2:24 PM · Restricted Project
LukeZhuang added a comment to D83362: Fix warning caused by __builtin_expect_with_probability was not handled in places such as constant folding.

@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.

Jul 8 2020, 12:17 PM · Restricted Project
LukeZhuang updated the diff for D83362: Fix warning caused by __builtin_expect_with_probability was not handled in places such as constant folding.

updated: 07/08/2020
(1) improve test case

Jul 8 2020, 12:15 PM · Restricted Project

Jul 7 2020

LukeZhuang created D83362: Fix warning caused by __builtin_expect_with_probability was not handled in places such as constant folding.
Jul 7 2020, 5:27 PM · Restricted Project

Jun 23 2020

LukeZhuang updated the diff for D82403: fix test failure for clang/test/CodeGen/builtin-expect-with-probability.cpp.

Fixed. If it looks good to you, could you please help me commit it? Thank you very much!

Jun 23 2020, 1:26 PM · Restricted Project, Restricted Project
LukeZhuang added a comment to D79830: Add support of __builtin_expect_with_probability.

@LukeZhuang : This patch causes the buildbots to fail, as O1 means something slightly different with the new pass manager :
http://lab.llvm.org:8011/builders/clang-x86_64-debian-new-pass-manager-fast/builds/10542/steps/test-check-all/logs/FAIL%3A%20Clang%3A%3Abuiltin-expect-with-probability.cpp

I'm sorry I didn't notice it during review, but the test that is failing is a poorly written test. The CFE tests shouldn't be written in a way that depends on the actions of the optimizer, so testing the branch_weights is incorrect.

Please submit a new patch with a way to validate the clang codegen actions without depending on the optimization (that is, would work with -disable-llvm-passes), and if necessary, add a test to llvm to ensure the proper result is validated.

EDIT: To clarify, I've unblocked the buildbots by doing a temporary fix. But this test needs to be rewritten.

Jun 23 2020, 12:54 PM · Restricted Project, Restricted Project
LukeZhuang created D82403: fix test failure for clang/test/CodeGen/builtin-expect-with-probability.cpp.
Jun 23 2020, 12:54 PM · Restricted Project, Restricted Project
LukeZhuang updated the summary of D82403: fix test failure for clang/test/CodeGen/builtin-expect-with-probability.cpp.
Jun 23 2020, 12:54 PM · Restricted Project, Restricted Project

Jun 22 2020

LukeZhuang added a comment to D82325: Use std::make_tuple instead initializer list.

Thank you very much for fixing! It looks good to me, while it seems to fail build for some other reason..

Jun 22 2020, 1:26 PM · Restricted Project
LukeZhuang added a comment to D79830: Add support of __builtin_expect_with_probability.

Thank you very much!

Jun 22 2020, 1:26 PM · Restricted Project, Restricted Project
Herald added a reviewer for D79830: Add support of __builtin_expect_with_probability: jdoerfert.

Hi, I do not have access to commit, could anybody help me to commit it? Thank you!

Jun 22 2020, 10:12 AM · Restricted Project, Restricted Project

Jun 16 2020

LukeZhuang added a comment to D79830: Add support of __builtin_expect_with_probability.

@rsmith Hi, could you please take a look at my revision since last comment? Thank you!

Jun 16 2020, 11:32 AM · Restricted Project, Restricted Project

Jun 15 2020

LukeZhuang updated the diff for D79830: Add support of __builtin_expect_with_probability.

Added: fix comment

Jun 15 2020, 11:25 PM · Restricted Project, Restricted Project
LukeZhuang updated the diff for D79830: Add support of __builtin_expect_with_probability.

updated: 06/15/2020
(1) fix minor variable name and doc word

Jun 15 2020, 11:04 PM · Restricted Project, Restricted Project
LukeZhuang updated the diff for D79830: Add support of __builtin_expect_with_probability.

updated: 06/15/2020
(1) minor change of assertion and cast

Jun 15 2020, 6:46 PM · Restricted Project, Restricted Project
LukeZhuang added a comment to D79830: Add support of __builtin_expect_with_probability.

Please fix the 'nit' when committing.

Jun 15 2020, 6:46 PM · Restricted Project, Restricted Project
LukeZhuang added a comment to D79830: Add support of __builtin_expect_with_probability.

ping :)
For clang side is there any suggestions? Thank you!

Jun 15 2020, 10:55 AM · Restricted Project, Restricted Project

Jun 9 2020

LukeZhuang updated the diff for D79830: Add support of __builtin_expect_with_probability.

updated: 06/09/2020
(1) improve code in LowerExpectIntrinsic
(2) update and simplify test

Jun 9 2020, 1:14 PM · Restricted Project, Restricted Project

Jun 8 2020

LukeZhuang updated the diff for D79830: Add support of __builtin_expect_with_probability.

updated: 06/08/2020
(1) update llvm side test for intrinsic llvm.expect.with.probability, which mimics test for llvm.expect

Jun 8 2020, 9:00 PM · Restricted Project, Restricted Project

Jun 6 2020

LukeZhuang updated the diff for D79830: Add support of __builtin_expect_with_probability.

updated: 06/06/2020
(1) updated llvm.expect.with.probability intrinsic document in LangRef.rst

Jun 6 2020, 2:25 PM · Restricted Project, Restricted Project
LukeZhuang added a comment to D79830: Add support of __builtin_expect_with_probability.

Can you add some tests at the LLVM side?

Jun 6 2020, 2:06 AM · Restricted Project, Restricted Project
LukeZhuang updated the diff for D79830: Add support of __builtin_expect_with_probability.

updated: 06/06/2020
(1) convert argument to IEEEdouble for different target
(2) update tests, add edge cases and llvm test
(3) minor change

Jun 6 2020, 2:06 AM · Restricted Project, Restricted Project

Jun 4 2020

LukeZhuang added inline comments to D79830: Add support of __builtin_expect_with_probability.
Jun 4 2020, 8:23 PM · Restricted Project, Restricted Project
LukeZhuang added a reviewer for D79830: Add support of __builtin_expect_with_probability: void.
Jun 4 2020, 7:51 PM · Restricted Project, Restricted Project

Jun 3 2020

LukeZhuang added inline comments to D79830: Add support of __builtin_expect_with_probability.
Jun 3 2020, 9:18 AM · Restricted Project, Restricted Project
LukeZhuang updated the diff for D79830: Add support of __builtin_expect_with_probability.

updated 06/03/2020:
(1) fix bug of range checking in SemaChecking

Jun 3 2020, 9:18 AM · Restricted Project, Restricted Project

Jun 1 2020

LukeZhuang added a comment to D79830: Add support of __builtin_expect_with_probability.

ping :)

Jun 1 2020, 11:18 AM · Restricted Project, Restricted Project

May 28 2020

LukeZhuang updated the diff for D79830: Add support of __builtin_expect_with_probability.

updated: 05/28/2020
(1) remove redundant "else" according to coding standard
(2) change a few document words

May 28 2020, 9:15 AM · Restricted Project, Restricted Project

May 26 2020

LukeZhuang added a comment to D79830: Add support of __builtin_expect_with_probability.

Fixed in latest update as well as adding test. Thank you!

May 26 2020, 11:58 AM · Restricted Project, Restricted Project
LukeZhuang updated the diff for D79830: Add support of __builtin_expect_with_probability.

updated: 05/26/2020
(1) add Sema test
(2) improve assert
(3) format change

May 26 2020, 11:58 AM · Restricted Project, Restricted Project

May 21 2020

LukeZhuang added a comment to D79830: Add support of __builtin_expect_with_probability.

FYI: I'm more of a clang contributor, so I'm unable to review the LLVM code, hopefully someone will come along who can check on that.

May 21 2020, 2:38 PM · Restricted Project, Restricted Project
LukeZhuang updated the diff for D79830: Add support of __builtin_expect_with_probability.

Updated: 05/21/2020

  1. add assertion after evaluate probability
  2. remove value dependent check in SemaChecking
  3. add test case handling value-dependent template
May 21 2020, 2:38 PM · Restricted Project, Restricted Project
LukeZhuang added inline comments to D79830: Add support of __builtin_expect_with_probability.
May 21 2020, 11:21 AM · Restricted Project, Restricted Project
LukeZhuang added inline comments to D79830: Add support of __builtin_expect_with_probability.
May 21 2020, 10:15 AM · Restricted Project, Restricted Project

May 15 2020

LukeZhuang added inline comments to D79830: Add support of __builtin_expect_with_probability.
May 15 2020, 3:14 PM · Restricted Project, Restricted Project
LukeZhuang updated the diff for D79830: Add support of __builtin_expect_with_probability.

updated 05/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 15 2020, 3:14 PM · Restricted Project, Restricted Project

May 14 2020

LukeZhuang added inline comments to D79830: Add support of __builtin_expect_with_probability.
May 14 2020, 12:30 PM · Restricted Project, Restricted Project

May 13 2020

LukeZhuang added inline comments to D79830: Add support of __builtin_expect_with_probability.
May 13 2020, 10:19 PM · Restricted Project, Restricted Project
LukeZhuang added inline comments to D79830: Add support of __builtin_expect_with_probability.
May 13 2020, 4:24 PM · Restricted Project, Restricted Project
LukeZhuang added a comment to D79830: Add support of __builtin_expect_with_probability.

Thanks for working on this.
Please upload patch with full context.

Hi, sorry but I'm not sure what does full context means, is that means I need to show more lines(for example 10 lines) when generating patch or show the whole function? Thank you!

git diff -p -U99999

May 13 2020, 4:24 PM · Restricted Project, Restricted Project
LukeZhuang updated the diff for D79830: Add support of __builtin_expect_with_probability.
  1. fix code format
  2. move probability type and value check from codegen to semacheck
  3. update patch with full context
May 13 2020, 4:24 PM · Restricted Project, Restricted Project
LukeZhuang added a comment to D79830: Add support of __builtin_expect_with_probability.

Thanks for working on this.
Please upload patch with full context.

May 13 2020, 11:57 AM · Restricted Project, Restricted Project

May 12 2020

LukeZhuang created D79830: Add support of __builtin_expect_with_probability.
May 12 2020, 5:48 PM · Restricted Project, Restricted Project