[PATCH] Mangling for intrinsic names w/function type parameters

Philip Reames listmail at philipreames.com
Thu Oct 9 14:13:21 PDT 2014


On 10/09/2014 02:02 PM, Andrew Trick wrote:
> This is my current thinking on this patch:
>
> We do want to allow defining a single intrinsic in tablegen that can support any return type. Having a single intrinsic opcode will cleanup the code.
>
> The only issue with the approach taken in this patch is that it mangles the entire signature of the target function into the patchpoint symbol. This seems unnecessary to me. I would prefer to see simple declarations for the intrinsics and use a bitcast when needed to pass the target function as i8*. I'm not aware of any use for the complex manglings, but we can debate that in the larger statepoint infrastructure review.
We started off with the bitcast approach.  It has two serious downsides:
1) It makes the IR much less readable.  This is hard to quantify, but 
our initial experience using a bitcast approach (which used casts for 
the target input, the gc operands, and the return type) was not 
positive.  Even folks who were very familiar with the IR patterns had a 
hard time deciphering what was going on.
2) It complicates pattern matching slightly.  This adds some 
complication to SelectionDAGBuilder.  (i.e. what happens if two bitcasts 
of the same function get commoned by CSE?)

I don't consider this to be an absolute must have for the statepoint 
infrastructure, but it's definitely a nice to have.  Frankly, if this 
gets rejected (and we don't settle on a better solution), I'm going to 
continue to use this patch in our own tree.

An alternate approach here would be to extend LLVM IR with an "any" 
type, but I can't imagine folks would want that...  Although, we do 
effectively have something close with the varadic arg notation.  Hm, we 
could simply declare the statepoint to be entirely varadic and then do 
custom type checking in the verifier?  That seems a bit messy though.  
It doesn't help with the return type either, but you sound like you 
might have an alternate approach for that.
>
> I believe Juergen has a patch that takes a simpler approach and I've asked him to post it.
>
> http://reviews.llvm.org/D4608
>
>




More information about the llvm-commits mailing list