[LLVMdev] Linux/ppc backend

[Chris Lattner]

On Sun, 25 Feb 2007, Nicolas Geoffray wrote:
> No problem. Plus the reviewing may have taken some time. So thx a lot
> for committing.  I talked to Jim who said
> he wanted to commit his changes before mine -- I hope everything's Ok.

There is one significant bug that fell out from this, which caused some 
macho function calls to be compiled with ELF ABI semantics.  I checked in 
this patch to fix it:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20070219/045027.html

I think you have to split PPCISD::CALL into PPCISD::CALL_ELF and 
PPCISD::CALL_Macho, like you do for BCTRL.  There were also a couple 
missing patterns, together these caused some nightly tester failures on 
darwin.

>> I applied the patches after some cleanups.  Please keep code within 80
>> columns, please don't use nested ?: expressions without parens, and please
>> be careful about indentation.

> Alright, sorry I forgot that. I'll be careful next time. Btw, if i have 
> some bug fixes to do, what should I do? Ask for a write access? Or just 
> do an RFC and waiting for someone to commit?

You are welcome to have commit-after-approval access.  I'll contact you 
offline.  Please familiarize yourself with the developer policy:
http://llvm.org/docs/DeveloperPolicy.html
specifically:
http://llvm.org/docs/DeveloperPolicy.html#commitaccess

>> Please verify that mainline CVS has
>> everything you think it should.
>>
> I just verified and launched some compilations. Everything seems OK.

Ok, I just broke ELF support to refix darwin, so please take a look again 
:)

>> The one hunk I didn't apply was this one:
>>
>> @@ -1392,12 +1418,13 @@
>>       case MVT::f32:
>>       case MVT::f64:
>> -      if (isVarArg && isPPC64) {
>> +      if (isVarArg || isPPC64) {
>>           // Float varargs need to be promoted to double.
>>           if (Arg.getValueType() == MVT::f32)
>>             Arg = DAG.getNode(ISD::FP_EXTEND, MVT::f64, Arg);
>>
>> This changes the darwin/ppc ABI.  It's not clear to me that this was
>> intended, so I just left it out.
>>

> Ah yes, I changed this and forgot to mention it (it deserves a different 
> commit - it's, as you noticed not, directly related to the linux/ppc 
> abi). I think it should be a "or" instead of an "and" : floats are 
> promoted to double in a vararg call wether you're on ppc64 or ppc32. 
> Maybe i'm missing something, anyone can confirm?

I spent quite a bit of time investigating this.  As it turns out, llvm-gcc 
automatically does the promotion before the code generator saw this, so it 
never could occur before.  I changed the check to:

       if (isVarArg) {
         // Float varargs need to be promoted to double.
         if (Arg.getValueType() == MVT::f32)
           Arg = DAG.getNode(ISD::FP_EXTEND, MVT::f64, Arg);
       }

which is correct (non-vararg floats passed to functions on ppc64 don't 
need extension).

Nice catch.

>> Thanks a lot for these patches.  How well does linux/ppc work now?
>>
>>
> Thanks to you for reviewing and committing.
> I would like to say it works great :) but I know there is one bug
> I have to found because I sometimes seem to get an infinite loop.
>
> If others have the opportunity to test it, please give us some feedback.

Ok!

-Chris

-- 
http://nondot.org/sabre/
http://llvm.org/