[PATCH] D88363: [CodeGen] Improve likelihood attribute branch weights

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

In D88363#2319242 <https://reviews.llvm.org/D88363#2319242>, @bdf wrote:

> In D88363#2319157 <https://reviews.llvm.org/D88363#2319157>, @jmorse wrote:
>
>> In D88363#2317241 <https://reviews.llvm.org/D88363#2317241>, @Mordante wrote:
>>
>>> Can you explain the kind of issues you're having?
>>
>> At the shallowest level, our -O1 produces different IR and fails the test, which is more or less our problem; however my understanding is that tests in the LLVM project / subprojects should aim to test as little amount of code as possible. Relying on all of -O1 makes it a brittle test -- changes to any optimisation pass enabled in -O1 could cause this test to fail spuriously.
>>
>> Instead, I believe the test should be in two parts:
>>
>> - One checking clang produces the correct /unoptimised/ IR output
>> - One or more checking that the consuming IR passes do-the-right-thing
>
> As I see, the intent of the test is not so much to verify a certain expected output, but more to verify that two styles of likelihood hints in C code produce the same code structure and branch weights. Theses styles are likely/unlikely-annotations, and use of __builtin_expect in the if condition. But the processing of these two is quite different:
>
> - for likely/unlikely annotations, branch weights are added immediately in the initial CodeGen
> - __builtin_expect is first translated straightforward to an expect intrinsic, then processed by a later lower-expect pass
>
> To make the test less brittle, would it be possible to explicitly select only the optimization passes that are needed?

Indeed verifying the output of the likelihood attributes against `__builtin_expect` is exactly what's required. But I think I can make the test less brittle by using the following command. This only runs the lower expect pass, which lowers the `__builtin_expect`.
`RUN: %clang_cc1 -O1 -disable-llvm-passes -emit-llvm %s -o - -triple=x86_64-linux-gnu | opt --lower-expect -S | FileCheck %s`

I'll work on a patch to solve the issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88363