[PATCH] D79830: Add support of __builtin_expect_with_probability

Erich Keane via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 23 09:04:19 PDT 2020


erichkeane added a comment.

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79830/new/

https://reviews.llvm.org/D79830





More information about the llvm-commits mailing list