[llvm-commits] [patch][arm] Implement support for the Q, R and H modifiers

Eric Christopher echristo at apple.com
Tue Aug 9 12:52:43 PDT 2011


On Aug 8, 2011, at 12:01 PM, Rafael Ávila de Espíndola wrote:

>> Sorry about the delay in responding, I've just returned from
>> vacation.
> 
> np and welcome back!
> 
>> I have some concerns about this patch as is - basically I'm worried
>> about subtle miscompiles based on how gcc does register allocation
>> for multiple reg wide values versus how llvm does it. In the gcc case
>> it will (as far as I know) assign consecutive registers for these
>> sorts of values, but there's no guarantee that llvm will do this.
> 
> I think that is correct. When printing 'H' for example, gcc uses "REGNO (x) + 1". I could not find any other reference to those constraints, so it looks like gcc uses sequential registers unconditionally.
> 

Yep.

>> This means that in the case of stm/ldm instructions with the 'Q' and
>> 'R' modifiers that we'd get registers that weren't meant, leading to
>> subtle problems that users would need to debug. When we get support
>> for assigning values into consecutive registers this won't be a
>> problem.
>> 
>> What are your thoughts?
> 
> This is the first time I have seen these constraints, so I don't know how common they are and of those uses which ones assume sequential registers.
> 

Btw, these are modifiers not constraints just to be careful about it.

> I have attached the testcase where we have found this problem. In this particular case, we don't depend on the registers being sequential (but llvm does it anyway because it is an argument).
> 

I see that. This is a good example of how you're supposed to be doing this.

> How hard would it be to add the constraint that the registers have to be sequential? If doing it I would probably try it by creating a new pseudo reg class that alias the regular R registers and use those as operands to the inline asm. Is that what you had in mind?
> 

Pretty much exactly.

> So, I guess this is a judgment call. The current patch allows us to handle some inline asm but causes us to miscompile others that we currently reject.

I'm going to say we should add them for now. People that are using it incorrectly may get a hard to debug problem, but the use case you guys have totally makes sense and isn't necessarily easy to rewrite to make more sense. Though, seriously, what's wrong with a .s file for that function?

-eric



More information about the llvm-commits mailing list