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

Andrew Trick atrick at apple.com
Thu Oct 9 14:40:26 PDT 2014


> On Oct 9, 2014, at 2:13 PM, Philip Reames <listmail at philipreames.com> wrote:
> 
> 
> 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 think that’s fair. As long as the mangling degenerates to something very close to the current intrinsic names for passing functions as i8* then it shouldn’t have impact on current clients. We will always need to bitcast because we’re not symbolically resolving the target addresses.

I’d like Lang and Juergen to chime in if they have an opinion.

-Andy 

> 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