Hi David,
I have several suggestions here.
1. I still insist on generalizing and normalizing the code without ABI info
in wgen.
It is better to do conversion only in backend using ABI information.
Currently, the
Need_Hidden_Parameter in wgen uses ABI information, which will decrease
portiblity
for other architectures.
2. If redesign is difficult, please use /* bug xxxx*/ or other similar ways
to comment all locations
where will be changed before merging into the trunk. Currently, AMD
usually doesn't
add any #ifdef or /* bug xxxx*/ like what Pathscale did before, which
makes people difficult
to track all changes associated to a certain problem. Since these
changes are in mulitple
components, it is better to add it.
3. Please test some fortran code
Thanks
Yin
----- Original Message -----
From: "David Coakley" <dcoak...@gmail.com>
To: "Yin Ma" <y...@absoft.com>
Cc: "open64-devel" <open64-devel@lists.sourceforge.net>
Sent: Tuesday, May 17, 2011 5:35 PM
Subject: Re: [Open64-devel] patch for better AMD64 ABI compatibility
> 1. Yes, we should be able to generalize and handle non-struct return
> cases too. However, the patch is well-tested as-is so I would prefer
> to make that improvement as a separate change and do more testing.
>
> 2. add_fake_parm creates a new VCALL with return type of void, so it
> will not add another fake parameter when we lower_call for the newly
> generated VCALL.
>
> On Tue, May 17, 2011 at 7:05 AM, Yin Ma <y...@absoft.com> wrote:
>> Hi David,
>>
>> I looked at those code. I have some concerns on this implementations.
>>
>> 1. in wgen_decl.cxx/wgen_expr.cxx, the code only remove
>> Mreturn.bar9_temp_0
>> if
>> Need_Hidden_Parameter return true, which need first fake argument.
>> Should we also remove temp_0 for the case that the return value doesn't
>> require the first fake argument because temp copy is also not neccesary?
>> Normalize to
>> MCALL bar9
>> MSTID return_decl_1885
>> for all the cases? And leave for backend to handle the conversion for
>> better compatibility because the frontend should not use too much
>> about lower level information such as ABI.
>>
>> 2. I am also wondering in wn_lower.cxx, after lower_return_mstid is
>> called
>> and
>> add_fake_parm is called. And later when the backend runs to lower_call,
>> .vcall will not be generated?
>>
>> Because I cannot integrate those changes into the backend and actually
>> run
>> the
>> code, my questions may be ambigurous.
>>
>>
>> Yin
>>
>>
>>
>>
>> ----- Original Message ----- From: "David Coakley" <dcoak...@gmail.com>
>> To: "Sun Chan" <sun.c...@gmail.com>
>> Cc: "open64-devel" <open64-devel@lists.sourceforge.net>
>> Sent: Monday, May 16, 2011 10:40 PM
>> Subject: Re: [Open64-devel] patch for better AMD64 ABI compatibility
>>
>>
>>> It is generic. The document that describes the ABI (from
>>> http://www.x86-64.org) uses the term AMD64 instead of x8664, but the
>>> same ABI is used for Intel too.
>>>
>>> On Mon, May 16, 2011 at 6:22 PM, Sun Chan <sun.c...@gmail.com> wrote:
>>>>
>>>> is this AMD64 ABI or generic X8664 ABI? I guess I don't know much
>>>> about Intel vs AMD ABI differences, if there is.
>>>> Sun
>>>>
>>>> On Tue, May 17, 2011 at 8:01 AM, David Coakley <dcoak...@gmail.com>
>>>> wrote:
>>>>>
>>>>> (Attachment added this time.)
>>>>>
>>>>> On Mon, May 16, 2011 at 5:01 PM, David Coakley <dcoak...@gmail.com>
>>>>> wrote:
>>>>>>
>>>>>> Could a gatekeeper review the attached patch? It fixes some AMD64 ABI
>>>>>> compatibility problems and also eliminates some unnecessary struct
>>>>>> copies. The second part should apply to all targets.
>>>>>>
>>>>>> Here is the proposed log message:
>>>>>>
>>>>>>
>>>>>> Improve AMD64 ABI compliance and avoid unnecessary struct copies.
>>>>>>
>>>>>> For the following code, the compiler generated unnecessary struct
>>>>>> copies
>>>>>> for a return value that has a size too big to be passed in registers.
>>>>>>
>>>>>> typedef struct { char big[1024]; } C;
>>>>>> C gc;
>>>>>> extern C bar9 (void);
>>>>>> extern C check9 (void) { return bar9 (); }
>>>>>>
>>>>>> In this case there were two copies and one temporary variable
>>>>>> generated.
>>>>>> After this change the compiler does not generate any extra copies.
>>>>>>
>>>>>> According to the AMD64 ABI, the caller provides space for the return
>>>>>> value in a hidden first argument and this address is returned in
>>>>>> %rax.
>>>>>> Previously the compiler was not following this rule.
>>>>>>
>>>>>> The change handles the similar cases of initialization by function
>>>>>> call ("C c = bar9();") and assignment to a pointer ("*cp = bar9()").
>>>>>>
>>>>>> Also, a complex long double field is now returned in the register
>>>>>> pair
>>>>>> (%st0, %st1) as specified by the AMD64 ABI.
>>>>>>
>>>>>>
>>>>>>
>>>>>> I am still looking for a review for the patch I posted last week (May
>>>>>> 10) as well. Thanks,
>>>>>>
>>>>>> -David Coakley / AMD Open Source Compiler Engineering
>>>>>>
>>>>>
>>>>>
>>>>> ------------------------------------------------------------------------------
>>>>> Achieve unprecedented app performance and reliability
>>>>> What every C/C++ and Fortran developer should know.
>>>>> Learn how Intel has extended the reach of its next-generation tools
>>>>> to help boost performance applications - inlcuding clusters.
>>>>> http://p.sf.net/sfu/intel-dev2devmay
>>>>> _______________________________________________
>>>>> Open64-devel mailing list
>>>>> Open64-devel@lists.sourceforge.net
>>>>> https://lists.sourceforge.net/lists/listinfo/open64-devel
>>>>>
>>>>>
>>>>
>>>
>>>
>>> ------------------------------------------------------------------------------
>>> Achieve unprecedented app performance and reliability
>>> What every C/C++ and Fortran developer should know.
>>> Learn how Intel has extended the reach of its next-generation tools
>>> to help boost performance applications - inlcuding clusters.
>>> http://p.sf.net/sfu/intel-dev2devmay
>>> _______________________________________________
>>> Open64-devel mailing list
>>> Open64-devel@lists.sourceforge.net
>>> https://lists.sourceforge.net/lists/listinfo/open64-devel
>>>
>>>
>>
>>
>
> ------------------------------------------------------------------------------
> What Every C/C++ and Fortran developer Should Know!
> Read this article and learn how Intel has extended the reach of its
> next-generation tools to help Windows* and Linux* C/C++ and Fortran
> developers boost performance applications - including clusters.
> http://p.sf.net/sfu/intel-dev2devmay
> _______________________________________________
> Open64-devel mailing list
> Open64-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/open64-devel
>
>
------------------------------------------------------------------------------
What Every C/C++ and Fortran developer Should Know!
Read this article and learn how Intel has extended the reach of its
next-generation tools to help Windows* and Linux* C/C++ and Fortran
developers boost performance applications - including clusters.
http://p.sf.net/sfu/intel-dev2devmay
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open64-devel
