LukeZhuang added a comment. In D79830#2109324 <https://reviews.llvm.org/D79830#2109324>, @erichkeane wrote:
> @LukeZhuang : This patch causes the buildbots to fail, as O1 > <https://reviews.llvm.org/owners/package/1/> 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. Thank you for pointing out! I have fixed by mimic the test case builtin-expect.c does. It makes sense to test generated llvm intrinsic instead of branch weight here. The lowering to branch weight is already tested in llvm side. 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 https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
