[PATCH] D83362: Fix warning caused by __builtin_expect_with_probability was not handled in places such as constant folding

LukeZhuang added a comment. Thank you very much! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83362/new/ https://reviews.llvm.org/D83362 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D83362: Fix warning caused by __builtin_expect_with_probability was not handled in places such as constant folding

LukeZhuang added a comment. Hi, could you please help me commit this change? Thank you very much! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83362/new/ https://reviews.llvm.org/D83362 ___ cfe-commits mailing list

[PATCH] D83362: Fix warning caused by __builtin_expect_with_probability was not handled in places such as constant folding

LukeZhuang marked an inline comment as done. LukeZhuang added a comment. Thank you for the new comment. I've modified my test cases CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83362/new/ https://reviews.llvm.org/D83362 ___ cfe-commits

[PATCH] D83362: Fix warning caused by __builtin_expect_with_probability was not handled in places such as constant folding

LukeZhuang updated this revision to Diff 276557. LukeZhuang added a comment. **updated: 07/08/2020** (1) improve test case CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83362/new/ https://reviews.llvm.org/D83362 Files: clang/lib/AST/ExprConstant.cpp

[PATCH] D83362: Fix warning caused by __builtin_expect_with_probability was not handled in places such as constant folding

LukeZhuang marked an inline comment as done. LukeZhuang added a comment. @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. CHANGES SINCE LAST ACTION

[PATCH] D83362: Fix warning caused by __builtin_expect_with_probability was not handled in places such as constant folding

LukeZhuang updated this revision to Diff 276517. LukeZhuang added a comment. **updated: 07/08/2020** (1) improve test case CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83362/new/ https://reviews.llvm.org/D83362 Files: clang/lib/AST/ExprConstant.cpp

[PATCH] D83362: Fix warning caused by __builtin_expect_with_probability was not handled in places such as constant folding

LukeZhuang created this revision. LukeZhuang added reviewers: lebedev.ri, erichkeane, xbolva00, RKSimon, rsmith. LukeZhuang added a project: clang. Herald added subscribers: cfe-commits, martong. Previously some places that should have handled `__builtin_expect_with_probability` is missing, so

[PATCH] D82403: fix test failure for clang/test/CodeGen/builtin-expect-with-probability.cpp

LukeZhuang updated this revision to Diff 272805. LukeZhuang added a comment. Fixed. If it looks good to you, could you please help me commit it? Thank you very much! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82403/new/ https://reviews.llvm.org/D82403 Files:

[PATCH] D82403: fix test failure for clang/test/CodeGen/builtin-expect-with-probability.cpp

LukeZhuang created this revision. LukeZhuang added a reviewer: erichkeane. LukeZhuang added projects: LLVM, clang. Herald added a subscriber: cfe-commits. LukeZhuang edited the summary of this revision. Fix test case added by D79830 Rewrite the test case, which

[PATCH] D79830: Add support of __builtin_expect_with_probability

LukeZhuang added a comment. In D79830#2109324 , @erichkeane wrote: > @LukeZhuang : This patch causes the buildbots to fail, as O1 > means something slightly > different with the new pass manager : > >

[PATCH] D79830: Add support of __builtin_expect_with_probability

LukeZhuang added a comment. Thank you very much! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79830/new/ https://reviews.llvm.org/D79830 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D79830: Add support of __builtin_expect_with_probability

LukeZhuang added a comment. Herald added a reviewer: jdoerfert. Hi, I do not have access to commit, could anybody help me to commit it? Thank you! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79830/new/ https://reviews.llvm.org/D79830

[PATCH] D79830: Add support of __builtin_expect_with_probability

LukeZhuang added a comment. @rsmith Hi, could you please take a look at my revision since last comment? Thank you! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79830/new/ https://reviews.llvm.org/D79830 ___ cfe-commits mailing list

[PATCH] D79830: Add support of __builtin_expect_with_probability

LukeZhuang updated this revision to Diff 270966. LukeZhuang marked an inline comment as done. LukeZhuang added a comment. Added: fix comment CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79830/new/ https://reviews.llvm.org/D79830 Files: clang/include/clang/Basic/Builtins.def

[PATCH] D79830: Add support of __builtin_expect_with_probability

LukeZhuang updated this revision to Diff 270963. LukeZhuang added a comment. **updated: 06/15/2020** (1) fix minor variable name and doc word CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79830/new/ https://reviews.llvm.org/D79830 Files: clang/include/clang/Basic/Builtins.def

[PATCH] D79830: Add support of __builtin_expect_with_probability

LukeZhuang updated this revision to Diff 270937. LukeZhuang marked an inline comment as done. LukeZhuang added a comment. **updated: 06/15/2020** (1) minor change of assertion and cast CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79830/new/ https://reviews.llvm.org/D79830 Files:

[PATCH] D79830: Add support of __builtin_expect_with_probability

LukeZhuang added a comment. In D79830#2093338 , @erichkeane wrote: > Please fix the 'nit' when committing. Fixed. Thank you! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79830/new/ https://reviews.llvm.org/D79830

[PATCH] D79830: Add support of __builtin_expect_with_probability

LukeZhuang added a comment. ping :) For clang side is there any suggestions? Thank you! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79830/new/ https://reviews.llvm.org/D79830 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D79830: Add support of __builtin_expect_with_probability

LukeZhuang updated this revision to Diff 269642. LukeZhuang added a comment. **updated: 06/09/2020** (1) improve code in LowerExpectIntrinsic (2) update and simplify test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79830/new/ https://reviews.llvm.org/D79830 Files:

[PATCH] D79830: Add support of __builtin_expect_with_probability

LukeZhuang updated this revision to Diff 269408. LukeZhuang added a comment. **updated: 06/08/2020** (1) update llvm side test for intrinsic llvm.expect.with.probability, which mimics test for llvm.expect CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79830/new/

[PATCH] D79830: Add support of __builtin_expect_with_probability

LukeZhuang updated this revision to Diff 269028. LukeZhuang added a reviewer: dexonsmith. LukeZhuang added a comment. **updated: 06/06/2020** (1) updated llvm.expect.with.probability intrinsic document in LangRef.rst CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79830/new/

[PATCH] D79830: Add support of __builtin_expect_with_probability

LukeZhuang updated this revision to Diff 269002. LukeZhuang added a comment. Herald added subscribers: Jim, dylanmckay. **updated: 06/06/2020** (1) convert argument to IEEEdouble for different target (2) update tests, add edge cases and llvm test (3) minor change CHANGES SINCE LAST ACTION

[PATCH] D79830: Add support of __builtin_expect_with_probability

LukeZhuang marked 12 inline comments as done. LukeZhuang added a comment. In D79830#2075038 , @davidxl wrote: > Can you add some tests at the LLVM side? added ll test case Comment at: clang/lib/Sema/SemaChecking.cpp:1818-1819 +

[PATCH] D79830: Add support of __builtin_expect_with_probability

LukeZhuang added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:1818-1819 +llvm::APFloat Probability = Eval.Val.getFloat(); +if (!(Probability >= llvm::APFloat(0.0) && + Probability <= llvm::APFloat(1.0))) { + Diag(ProbArg->getLocStart(),

[PATCH] D79830: Add support of __builtin_expect_with_probability

LukeZhuang marked 4 inline comments as done. LukeZhuang added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:1818-1819 +llvm::APFloat Probability = Eval.Val.getFloat(); +if (!(Probability >= llvm::APFloat(0.0) || + Probability <=

[PATCH] D79830: Add support of __builtin_expect_with_probability

LukeZhuang updated this revision to Diff 268226. LukeZhuang added a comment. **updated 06/03/2020**: (1) fix bug of range checking in SemaChecking CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79830/new/ https://reviews.llvm.org/D79830 Files: clang/include/clang/Basic/Builtins.def

[PATCH] D79830: Add support of __builtin_expect_with_probability

LukeZhuang added a comment. ping :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79830/new/ https://reviews.llvm.org/D79830 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D79830: Add support of __builtin_expect_with_probability

LukeZhuang updated this revision to Diff 266901. LukeZhuang added a comment. **updated: 05/28/2020** (1) remove redundant "else" according to coding standard (2) change a few document words Thank you Erich! I think removing this "else" here makes sense. CHANGES SINCE LAST ACTION

[PATCH] D79830: Add support of __builtin_expect_with_probability

LukeZhuang updated this revision to Diff 266285. LukeZhuang added a comment. **updated: 05/26/2020** (1) add Sema test (2) improve assert (3) format change CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79830/new/ https://reviews.llvm.org/D79830 Files:

[PATCH] D79830: Add support of __builtin_expect_with_probability

LukeZhuang marked 3 inline comments as done. LukeZhuang added a comment. Fixed in latest update as well as adding test. Thank you! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79830/new/ https://reviews.llvm.org/D79830 ___ cfe-commits

[PATCH] D79830: Add support of __builtin_expect_with_probability

LukeZhuang marked 8 inline comments as done. LukeZhuang added a comment. In D79830#2049582 , @erichkeane wrote: > 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.

[PATCH] D79830: Add support of __builtin_expect_with_probability

LukeZhuang updated this revision to Diff 265598. LukeZhuang added a comment. **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 CHANGES SINCE LAST ACTION

[PATCH] D79830: Add support of __builtin_expect_with_probability

LukeZhuang marked 6 inline comments as done. LukeZhuang added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:1805 +const Expr *ProbArg = TheCall->getArg(2); +if (ProbArg->isValueDependent()) + return ExprError(); erichkeane wrote: >

[PATCH] D79830: Add support of __builtin_expect_with_probability

LukeZhuang added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2204 +if (CGM.getCodeGenOpts().OptimizationLevel == 0) + return RValue::get(ArgValue); + erichkeane wrote: > Do you still need to evaluate this for side-effects even when

[PATCH] D79830: Add support of __builtin_expect_with_probability

LukeZhuang updated this revision to Diff 264357. LukeZhuang added a comment. 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

[PATCH] D79830: Add support of __builtin_expect_with_probability

LukeZhuang marked 15 inline comments as done. LukeZhuang added inline comments. Comment at: clang/include/clang/Basic/Builtins.def:567 BUILTIN(__builtin_expect, "LiLiLi" , "nc") +BUILTIN(__builtin_expect_with_probability, "LiLiLid" , "nc") BUILTIN(__builtin_prefetch,

[PATCH] D79830: Add support of __builtin_expect_with_probability

LukeZhuang added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:1805 +const Expr *ProbArg = TheCall->getArg(2); +if (ProbArg->EvaluateAsFloat(Confidence, Context)) { + double P = Confidence.convertToDouble(); LukeZhuang wrote: > rsmith

[PATCH] D79830: Add support of __builtin_expect_with_probability

LukeZhuang added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2195 +Value *ExpectedValue = EmitScalarExpr(E->getArg(1)); +Value *Confidence = EmitScalarExpr(E->getArg(2)); +// Don't generate llvm.expect.with.probability on -O0 as the backend

[PATCH] D79830: Add support of __builtin_expect_with_probability

LukeZhuang added a comment. In D79830#2034796 , @lebedev.ri wrote: > In D79830#2034779 , @LukeZhuang > wrote: > > > In D79830#2033367 , @lebedev.ri > > wrote: > > > > >

[PATCH] D79830: Add support of __builtin_expect_with_probability

LukeZhuang marked 4 inline comments as done. LukeZhuang added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2049 +ConstantFP *Confidence_f = dyn_cast(Confidence); +if (!Confidence_f) { + CGM.Error(E->getArg(2)->getLocStart(),

[PATCH] D79830: Add support of __builtin_expect_with_probability

LukeZhuang updated this revision to Diff 263877. LukeZhuang added a comment. 1. fix code format 2. move probability type and value check from codegen to semacheck 3. update patch with full context CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79830/new/ https://reviews.llvm.org/D79830

[PATCH] D79830: Add support of __builtin_expect_with_probability

LukeZhuang added a comment. In D79830#2033367 , @lebedev.ri wrote: > 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

[PATCH] D79830: Add support of __builtin_expect_with_probability

LukeZhuang created this revision. LukeZhuang added reviewers: RKSimon, phosek, gribozavr, davidxl, chandlerc, pete, jyknight, dblaikie, kuba. LukeZhuang added projects: LLVM, clang. Herald added subscribers: llvm-commits, cfe-commits, jdoerfert, hiraditya. Add a new builtin-function