[llvm-commits] [llvm] r159146 - in /llvm/trunk: lib/Transforms/InstCombine/InstructionCombining.cpp lib/Transforms/Scalar/SimplifyCFGPass.cpp lib/VMCore/Verifier.cpp test/Transforms/InstCombine/objsize-64.ll test/Transforms/SimplifyCFG/invoke.ll

[Nuno Lopes]

Quoting Chris Lattner <clattner at apple.com>:

> On Jun 28, 2012, at 10:17 AM, Duncan Sands wrote:
>>>> This is also the part I don't like.  After thinking about this a  
>>>> bit, the only
>>>> way I can see to sanely handle this is to have instcombine zap the
>>>> alloc/delete pair.  Since it can't (and we don't want it to)  
>>>> modify the CFG, I
>>>> think that it is reasonable for instcombine to zap the alloc by  
>>>> changing the
>>>> invoke to call a noop intrinsic.
>>>>
>>>> That said, I don't agree that all intrinsics should be valid in an invoke,
>>
>> currently all intrinsics are marked nounwind (when I implemented  
>> that I removed
>> all the special "intrinsics can't unwind" logic at the IR level), so even if
>> you did an invoke on them it would be turned into a call by the  
>> optimizers.  It
>> would probably be easy at the codegen levels to turn invoke-of-nounwind-
>> function into a call, for the -O0 case.  That would make invoke of  
>> intrinsics
>> safe.  If one day we want an intrinsic that can unwind, it would be  
>> easy enough
>> to add by not marking it nounwind.  In short, I think it would be  
>> simple enough
>> to remove the restriction that you can't do invoke on an intrinsic.
>
> Makes sense,
>
>>>> 1. Introduce an new llvm.undef.result intrinsics, which can be used with
>>>> arbitrary arguments and return value.  It is fully defined to ignore its
>>>> argument and return undef.  Codegen should be enhanced to handle it by
>>>> lowering it to nothing (the results would be lowered to  
>>>> IMPLICIT_DEFs if not
>>>> dead).
>>
>> For some time I've wanted an llvm.identity intrinsic, which is  
>> basically just
>> an "undef" barrier.  The concept is that, given x, the result of
>> llvm.identity(x) is just x, however llvm.identity(undef) is  
>> something without
>> the funky undef semantics, for example 0.  This is useful for range checks.
>> Anyway, such an intrinsic would be generally useful, and could also be used
>> here rather than introducing llvm.undef.result, just use llvm.identity(0).
>
> This sounds like it could be independently useful, but one of the  
> ideas of llvm.undef.result is that it wouldn't have to have the same  
> argument and result types.  In the case of instcombine wanting to  
> zap an arbitrary invoke, it would be nice for it to be able to just  
> create a new intrinsic (with the right type) and replace the callee  
> of the invoke, rather than removing the invoke and replacing it with  
> another one.
>
> That said, now that I think about it, I don't know how interesting  
> or practical this is.  The getIntrinsic() call wants the list of  
> parametric types for the intrinsic.  Having to extract all those out  
> of the invoke to pass into getIntrinsic() is probably more work than  
> it is to just replace the invoke call.

That's a very important point. I was trying to implement this  
intrinsic, but it's quite difficult. Right now it's not possible to  
declare a parameter of an intrinsic as having an arbitrary type, nor  
tablegen supports variadics.
Is all this work useful, or can I just get away with a simpler version?

Nuno

P.S.: Regarding the llvm.identity thing, it's not forgotten; I just  
didn't spend enough brain cycles to think about those undef issues yet.