[PATCH] D140337: Propagate branchweight through phi values when ExpectedValue is unlikely in LowerExpectIntrinsic

Zhi Zhuang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 19 12:04:32 PST 2022


LukeZhuang created this revision.
LukeZhuang added a reviewer: davidxl.
LukeZhuang added a project: LLVM.
Herald added a subscriber: hiraditya.
Herald added a project: All.
LukeZhuang requested review of this revision.
Herald added subscribers: llvm-commits, jdoerfert.

Currently `handlePhiDef` does not take the probability operand of `expect_with_probability` into consideration.
When the probability is low, meaning the "expected value" is unlikely, we need to mark the branches from which the phi value is the same as "expected value" as unlikely.

The existing code treats `__builtin_expect((a0==1)||(a1==1)||(a2==1), 1)` and `__builtin_expect_with_probability((a0==1)||(a1==1)||(a2==1), 1, 0)` the same, when in fact they are very different:  They both "expect" value 1, but the first says it is very likely and the second says it has probability 0, i.e. very unlikely.

An example of IR code we need to handle is:

  b0:
    %c0 = icmp eq i8 %a0, 1
    br i1 %c0, label %b3, label %b1
    
  b1:
    %c1 = icmp eq i8 %a1, 1
    br i1 %c1, label %b3, label %b2
    
  b2:
    %c2 = icmp eq i8 %a2, 1
    br label %b3
  
  b3:
    %cond0 = phi i1 [ true, %b0 ], [ true, %b1 ], [ %c2, %b2 ]
    %cond1 = extend %cond0 to i64
    %expval = call i64 @llvm.expect.with.probability.i64(i64 %cond1, i64 1, double 0.0)

We should also annotate the `BranchInst` of `b0` and `b1` because we know that "true" is unlikely.  The only path that does not result in an unlikely outcome is through b2.

This helps `__builtin_expect(x, 0)` and `__builtin_expect_with_probability(x, 1, 0)` have the same behavior.  For reference, GCC does already behave consistently like this.

In addition, we also added an `ImmArg` attribute to the `expect.with.probability` intrinsic to disallow non-constant probability arguments.

This issue was introduced by https://reviews.llvm.org/D79830, which did not update `handlePhiDef` correctly for the `expect.with.probability` case.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140337

Files:
  llvm/include/llvm/IR/Intrinsics.td
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/test/Transforms/LowerExpectIntrinsic/phi_unexpect.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D140337.484031.patch
Type: text/x-patch
Size: 9654 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20221219/940ff5b7/attachment.bin>


More information about the llvm-commits mailing list