[LLVMdev] [cfe-commits] [PATCH/RFC, PowerPC] Extend 32-bit function arguments / return values

[Eli Friedman]

On Wed, Oct 31, 2012 at 7:44 AM, Ulrich Weigand
<Ulrich.Weigand at de.ibm.com> wrote:
> Duncan Sands <duncan.sands at gmail.com> wrote on 24.10.2012 08:20:07:
>> Hal Finkel <hfinkel at anl.gov> wrote:
>
>> >> Chris wrote some notes about this:
>> >>     http://www.nondot.org/sabre/LLVMNotes/ExtendedIntegerResults.txt
>> >> The plan seems sensible to me, but was blocked by not having a good
>> >> way of attaching the information to parameters and return values.
>> >> Hopefully Bill's attribute work means it is now possible.
>> >
>> > Indeed. Are you proposing that Ulrich hold off on the current fix
>> in favor of the new attribute scheme, or that he should fix this
>> using the current mechanism for now (perhaps with a FIXME to upgrade
>> to the new scheme once it is in place)? I would prefer for him to
>> commit this now, and then upgrade later.
>>
>> I don't have anything useful to say about Ulrich's fix since I didn't
> look at
>> it.  I just wanted to make sure that everyone knew where we are
>> probably going:
>> in the direction laid out by Chris in those notes.
>
>
> I'm wondering how to proceed on this issue for now.  Current status is
> that I've checked in the testsuite patch, since there was agreement
> that fixing the tests by adding target triples was the way to go.
> However, the actual fix to enable the sign-extensions required by
> the PowerPC64 ABI is not in, so we still generate code that violates
> the ABI.
>
> I understand that there is a long-term goal of having those extensions
> done in the front end instead of in LLVM.  However, as far as I can
> see the necessary infrastructure is not yet fully present (or in any
> event, I don't see how to do it in the front end right now) ...
>
> Therefore, since my patch doesn't make moving to the new scheme
> any more difficult (it just does the same thing for "int" that is
> already being done for "short" and "char"), and it does fix the
> ABI bug we have right now, I'd propose to check it in now (and
> then move to new scheme with everyone else once it is ready).
>
> Would this be OK?  Any other suggestions?
>
> For reference, the patch in question is appended again.

Please factor out the check for whether an integer type needs to be
extended into a separate method.

Please make sure there's a testcase on the LLVM side to make sure we
actually handle the attributes correctly.

Otherwise, looks fine.

-Eli