[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

[Chris Lattner]

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.

-Chris