[llvm] r181753 - This is the first of three patches which creates stubs used for

[reed kotler]

Hi Jim,

Thanks as always for the good suggestions.

I will definitely look into ways to incorporate your ideas from the ARM 
port in order to make this pass more generally palatable to all.

When this first version is working I want to think about the cleanest 
way to do this.

Reed

On 05/15/2013 11:02 AM, Jim Grosbach wrote:
> Hi Reed,
>
> Glad to see the discussion getting back going again. Thanks. Let’s see 
> if we can work through
> On May 15, 2013, at 10:02 AM, reed kotler <rkotler at mips.com 
> <mailto:rkotler at mips.com>> wrote:
>
>> On 05/15/2013 07:14 AM, Rafael Espíndola wrote:
>>>> I probably air too much of what I'm doing on the list as far as 
>>>> design and
>>>> my attempt is never to hide such things. I really could not have 
>>>> imagined
>>>> such a big discussion about generating some tiny stubs . It only 
>>>> came up
>>>> during the issue of the APP/NO_APP wrappers.
>>> I still think you are doing this wrong and it will be harder to
>>> implement it correctly once you have this in. IMHO the only thing
>>> needing special treatment for the stubs in the backend would be
>>> calling convention. Other than that they would be regular mips16 or
>>> mips32 *IL* functions. Mixing mips16 and mip32 should not be that
>>> different from mixing thumb and arm.
>> This is a separate module pass. There is nothing being done here 
>> except solving this problem
>> so it's easy to modify, delete or replace in the future..
>>
> OK.
>
>> I have filed a bug already against myself at Mips to look at this 
>> issue of using special calling conventions. I did a special calling 
>> convention already for the return helpers but these are more 
>> involved. There are special issues here not normally covered by a 
>> simple calling convention. If you look at the code in 
>> assureFPCallStub you will see that the way the helper even calls the 
>> real function depends on whether the function returns float, double, 
>> single or double complex and then the final return is different too. 
>> The next patch after this, which you have not seen yet, relies on 
>> some special gas tricks that. This is just the way it was designed 
>> for gcc but it's very efficient the way they work
>> so that was the reason.
>
> GCC is a useful example, to be sure, but the compiler architectures 
> are so different that what’s a good design fit for one does not imply 
> it’s a good fit for the other. For example, inline asm for GCC is just 
> a textual blob of output. For LLVM it’s a much different thing due to 
> the integrated assembler and MC layers. What GCC does isn’t a very 
> compelling argument.
>
>> I'm with you on making things better and simpler all the time. It's 
>> not always practical to hit the
>> final goal on the first try.
>
> Yep. Iterative development is great. We just want to make sure that 
> the incremental steps actually are moving in the direction of the 
> eventual goal and won’t get in the way when the time comes. Making 
> sure we don’t incur more technical debt than we need to, basically.
>
>>
>>> But you are correct, I don't have big objections to you guys
>>> implementing this in the way you think is best, *as long as it stays
>>> local to mips*. In particular, two things that came up in this
>>> discussion that really don't want to see in trunk are:
>>>
>>> * the ability for the compiler to tell if an inline asm is compiler
>>> generated or human written. (the APP/NO_APP discussion). I am ok with
>>> avoiding printing them, as long as we avoid for all inline asm (by not
>>> printing it at all for mips target for example).
>> This APP/NO_APP would be covered in a separate patch and is only for 
>> cosmetic purposes.
>>
>> I was not asking for the compiler to be able to tell this.
>>
>> I want be able to have a function attribute which tells asmprinter to 
>> not emit those.
>> This could be usable by the compiler itself or if we *chose* to make 
>> it general, by a user.
>
>
> I’m strongly opposed to any target independent patch which makes a 
> distinction about “compiler generated” or “user generated” inline 
> assembly blocks. If you can do this purely in code in the 
> lib/Target/Mips tree, go for it. If it touches anywhere else, that’s a 
> problem. This is a non-starter.
>
>
>> When GCC creates these stubs, they look like normal compiler code (no 
>> APP/NO_APP) markers
>> and without this ability LLVM is going to add these markers after 
>> each line.
>>
>> There is no functional difference in the code but they look ugly to 
>> me and it makes it harder to
>> read them.
>>
>> This is not really that important but I think we should not have them 
>> here because they serve no
>> purpose other than to make the stub harder to read.
>
> That’s the compiler telling you that you’re using a mechanism not 
> intended for this sort of purpose.
>
> There are a few things going on here, so I’ll try to break them apart 
> a bit.
>
> 1) Generally speaking, how to get interworking stubs into the code.
>
> Interworking is a tricky thing to be sure. ARM has the same sort of 
> problems with Thumb vs. ARM code. On Darwin, we solve this by having 
> the linker insert the stub functions when necessary and fixing up the 
> interworking calls to do the right thing. If that’s possible for you, 
> I’d encourage investigating the option, as it’s worked really well for 
> us. There’s complications when you have historical ABI, of course. If 
> you really do have to do this in the compiler, we end up with:
>
> 2) How to get a specific asm sequence emitted by the compiler into a 
> function.
>
> This is what pseudo-instructions, target intrinsics, custom lowerings, 
> etc. are for. Depending on which layers of the back end you want the 
> operation to be opaque or transparent to, you can choose which 
> mechanism to use. As I’ve said before, have a look at ARM. It does a 
> *lot* of this sort of thing. The closest analog to inline asm is an 
> MC-level pseudo, which is a black-box to the compiler all the way 
> through the backend until lowering the MC right before .s/.o emission. 
> Switching to use something like that instead would likely be very 
> straightforward and will play much more cleanly with the rest of the 
> compiler. It’ll also get rid of the APP/NO_APP markers for you.
>
>
> Regards,
>
> -Jim
>
>
>>> * the extension of the section attribute to be an arbitrary string
>>> passed to the assembler or the addition of section related attributes
>>> (i.e.,llvm.org/pr15788 <http://llvm.org/pr15788>should be closed as 
>>> wontfix).
>> I did not need anything special here as I thought I would originally.
>> I was able to use the builtin function attributes that LLVM supports 
>> to get the desired result.
>>
>>
>> Thanks for taking the time to respond in such detail.
>>
>> Reed
>>
>>> Cheers,
>>> Rafael
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130515/8bc06154/attachment.html>