[llvm-commits] [llvm] r134516 - in /llvm/trunk: include/llvm/ include/llvm/Support/ include/llvm/Transforms/ lib/CodeGen/ lib/CodeGen/SelectionDAG/ lib/Transforms/Scalar/ lib/Transforms/Utils/ lib/VMCore/ test/CodeGen/Generic/ test/Transforms/LowerExpectIntrinsic/

[Andrew Trick]

On Jul 12, 2011, at 10:07 PM, Evan Cheng wrote:

> If builtin expects may end up disabling some optimizations, that indicates llvm should lower than early in compilation before any optimization passes. 
> 
> Evan
> 
> On Jul 12, 2011, at 4:01 PM, Andrew Trick <atrick at apple.com> wrote:
> 
>> On Jul 12, 2011, at 2:24 PM, Jakub Staszak wrote:
>> 
>>> Hi Andy,
>>> 
>>> Optimizations can always be harmed by intrinsics (e.g. Instruction Combiner). Because of that we remove expect intrinsics at the same beginning before any optimizations.
>>> 
>>> -Kuba
>> 
>> Part of the answer to my question is that the intrinsic replaces the uses of the original value. Naturally that's a serious problem for anyone following the def-use chain. I'm not sure why you wanted to define the intrinsic that way. Presumably because we can't pin it to a block?
>> 
>> Ideally you want a positional intrinsic interpreted as such: the value operand takes on the expected constant on any path dominated by the intrinsic. There should be no need to violate the def-use chain.
>> 
>> -Andy

My previous message was a bit abstract. Below is an example of what I
had assumed expect intrinsics would typically look like:

%value = ...
...
br i1 %predicate, label %block... ;

block:
  call void llvm.expect(%value, N)
  %cmp = icmp ne %value, N
  br %cmp ...

Interpretation: %value is expected to be N whenever %predicate is
true. The llvm.expect intrinsic should have an implicit control
dependence on %predicate, but should not affect code motion or
optimization in any other way. If defined this way, I would be curious
what optimizations are affected. Clearly some additional framework
would be helpful, such as recognizing these sort of intrinsics as
"phantom" uses that aren't themselves DCE'd but don't affect DCE of
their operands.

I have two reasons for bringing this up

1) branch_weight meta-data has a host of its own problems. It was not
   intended to be used by anyone except BranchProbability, and only
   proposed as a way to make information provided by the front-end
   persistent. Adding new intrinsics generally stinks, but it is much
   better than dealing with branch_weight meta-data. Using both
   results in the worst of both worlds and additionally requires a
   gratuitous new lowering pass (I'm already annoyed by the new
   -debug-pass output).

2) The need for intrinsics-as-meta-data is felt far beyond branch
   profiling, so it's a problem worth solving right. By "meta-data" I
   don't necessarilly mean optional data, but auxiliary information
   about another value that is tied to control flow.

-Andy

>> 
>>> On Jul 12, 2011, at 2:01 PM, Andrew Trick wrote:
>>> 
>>>> 
>>>> On Jul 7, 2011, at 1:48 PM, Jakub Staszak wrote:
>>>>> Right now clang creates "expect" intrinsic for every __builtin_expect instruction. These intrinsics must be lowered early because in other case they can harm other optimizations. During the lowering we also create "branch_weight" metadata, which can be used by other optimizations using BranchProbability analysis.
>>>> 
>>>> Kuba,
>>>> 
>>>> Which optimizations are harmed by expect intrinsics?
>>>> 
>>>> If this is just a temporary solution, that's fine. In the long term, we need to formalize the properties of intrinsics that serve as positional meta-data. This is needed for debug info, exception handling, lifetimes, profiling, and probably more to come. We should have a uniform framework for these that guarantee they maintain their association with a control flow edge without affecting any optimizations or heuristics. BasicBlock::getFirstNonPHIOrDbgOrLifetime is a glaring example of the need for a general solution.
>>>> 
>>>> Three representations of almost exactly the same information (intrinsics, meta-data, and analysis) is also evidence of a design shortcoming.
>>>> 
>>>> -Andy
>>>> 
>>>>> On Jul 6, 2011, at 5:31 PM, Chris Lattner wrote:
>>>>> 
>>>>>> 
>>>>>> On Jul 6, 2011, at 11:22 AM, Jakub Staszak wrote:
>>>>>> 
>>>>>>> Author: kuba
>>>>>>> Date: Wed Jul  6 13:22:43 2011
>>>>>>> New Revision: 134516
>>>>>>> 
>>>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=134516&view=rev
>>>>>>> Log:
>>>>>>> Introduce "expect" intrinsic instructions.
>>>>>>> 
>>>>>> 
>>>>>> Hi Kuba,
>>>>>> 
>>>>>> Why is this introducing a new pass to do lowering?
>>>>>> 
>>>>>> -Chris

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20110712/136eae06/attachment.html>