[PATCH] D12341: add llvm.unpredictable intrinsic and lower it to metadata

[Hal Finkel via llvm-commits]

----- Original Message -----
> From: "Sanjay Patel" <spatel at rotateright.com>
> To: spatel at rotateright.com, chandlerc at gmail.com, kparzysz at codeaurora.org, hfinkel at anl.gov, "bob wilson"
> <bob.wilson at apple.com>, kubastaszak at gmail.com
> Cc: "kevin b smith" <kevin.b.smith at intel.com>, llvm-commits at lists.llvm.org
> Sent: Thursday, August 27, 2015 6:38:39 PM
> Subject: Re: [PATCH] D12341: add llvm.unpredictable intrinsic and lower it to metadata
> 
> spatel added a comment.
> 
> In http://reviews.llvm.org/D12341#234428, @hfinkel wrote:
> 
> > The problem with the intrinsic is that ... it confuses information
> > about a value with information about a branch based on that value.
> 
> 
> Do you see this being different than __builtin_expect() in Clang? If
> it's information about the branch itself, do we have:
> 
>   __builtin_unpredictable_if( MyCondition ) { ... }
> 
> If it's about the condition, then it looks like the existing
> __builtin_expect:
> 
>   if ( __builtin_unpredictable( MyCondition ) { ... }
> 
> ...but we'll have to propagate the metadata from the condition value
> to branches that use it in Clang?

I think that the intrinsic can look like __builtin_expect, in that it wraps a value (that's easiest in C), but the propagation should be done to the branches in Clang. We don't gain anything from doing it later because we run LowerExpectIntrinsic before inlining regardless (this might technically be untrue, because we do an initial SROA run, but I've never seen this used outside of its documented form inside an if/while/for statement, and so the data dependence is clear from the AST).

 -Hal

> Or does unpredictable metadata
> stick to a value in Clang and then the backend is responsible for
> tracking back to that metadata when looking at branches?
> 
> 
> http://reviews.llvm.org/D12341
> 
> 
> 
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory