Re: PATCH: PR target/39472: Add -mabi=[ms|sysv]

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Wed, Mar 18, 2009 at 3:52 AM, Kai Tietz <ktietz70@googlemail.com> wrote:
> 2009/3/18 Richard Guenther <richard.guenther@gmail.com>:
>> On Tue, Mar 17, 2009 at 8:52 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>>> 2009/3/17 H.J. Lu <hjl.tools@gmail.com>:
>>>> On Tue, Mar 17, 2009 at 11:52 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>> On Tue, Mar 17, 2009 at 11:20 AM, Kai Tietz <ktietz70@googlemail.com> wrote:
>>>>>> Hallo H.J,
>>>>>>
>>>>>> this patch looks fine to me, beside these three junks
>>>>>>
>>>>>>> @@ -2740,7 +2757,7 @@ override_options (bool main_args_p)
>>>>>>> ? ? ? ? use of rip-relative addressing. ?This eliminates fixups that
>>>>>>> ? ? ? ? would otherwise be needed if this object is to be placed in a
>>>>>>> ? ? ? ? DLL, and is essentially just as efficient as direct addressing. ?*/
>>>>>>> - ? ? ?if (TARGET_64BIT && DEFAULT_ABI == MS_ABI)
>>>>>>> + ? ? ?if (TARGET_64BIT && ix86_abi == MS_ABI)
>>>>>>> ? ? ? ?ix86_cmodel = CM_SMALL_PIC, flag_pic = 1;
>>>>>>> ? ? ? else if (TARGET_64BIT)
>>>>>>> ? ? ? ?ix86_cmodel = flag_pic ? CM_SMALL_PIC : CM_SMALL;
>>>>>>
>>>>>> Do you want really to modify by this switch to pic mode and small
>>>>>> model for linux64?
>>>>>
>>>>> OK, I will remove it.
>>>>>
>>>>>>
>>>>>>> @@ -10096,7 +10115,7 @@ output_pic_addr_const (FILE *file, rtx x
>>>>>>> ?#endif
>>>>>>> ? ? ? ? ?assemble_name (file, name);
>>>>>>> ? ? ? ?}
>>>>>>> - ? ? ?if (!TARGET_MACHO && !(TARGET_64BIT && DEFAULT_ABI == MS_ABI)
>>>>>>> + ? ? ?if (!TARGET_MACHO && !(TARGET_64BIT && ix86_abi == MS_ABI)
>>>>>>> ? ? ? ? ?&& code == 'P' && ! SYMBOL_REF_LOCAL_P (x))
>>>>>>> ? ? ? ?fputs ("@PLT", file);
>>>>>>> ? ? ? break;
>>>>>>
>>>>>> This looks wrong to me, because for coff targets you never can emit @PLT.
>>>>>
>>>>> I will remove it.
>>>>>
>>>>>>> @@ -26683,7 +26702,7 @@ x86_function_profiler (FILE *file, int l
>>>>>>> ? ? ? fprintf (file, "\tleaq\t%sP%d@(%%rip),%%r11\n", LPREFIX, labelno);
>>>>>>> ?#endif
>>>>>>>
>>>>>>> - ? ? ?if (DEFAULT_ABI == SYSV_ABI && flag_pic)
>>>>>>> + ? ? ?if (ix86_abi == SYSV_ABI && flag_pic)
>>>>>>> ? ? ? ?fprintf (file, "\tcall\t*%s@GOTPCREL(%%rip)\n", MCOUNT_NAME);
>>>>>>> ? ? ? else
>>>>>>> ? ? ? ?fprintf (file, "\tcall\t%s\n", MCOUNT_NAME);
>>>>>>
>>>>>> the same here, pep-coff do not have really a GOTPCREL.
>>>>>> By this I would check here DEFAULT_ABI, because it is the targets
>>>>>> default and can be used to decide if @PLT and @GOTPCREL are present.
>>>>>
>>>>> I will remove it.
>>>>>
>>>>> Thanks.
>>>>>
>>>>
>>>> Here is the updated patch. I also changed
>>>> gcc.target/x86_64/abi/callabi/vaarg-5b.c
>>>> to test cross ABI vararg. OK for 4.5 and 4.4 after it is reopened?
>>>>
>>>> Thanks.
>>>>
>>>>
>>>> --
>>>> H.J.
>>>>
>>>
>>> Yes, ok.
>>
>> Not for 4.4, as this is a new feature. ?Also I don't think you can approve
>> i386 backend changes.
>>
>> Thanks,
>> Richard.
>
> Well, I can't approve backend changes as far as they do not belong
> directly to windows targets. And IMHO this patch of H.J. affects this
> pretty much. So I think it is at least partial correct that an
> approval of a windows target maintainer is necessary.
>

First, I consider ms_abi attribute is a Windows feature. Secondly
ms_abi is a new feature in gcc 4.4. Third, ms_abi is incomplete.
The real goal for ms_abi attribute  is to implement -mabi=ms. I
don't think ms_abi attribute is very useful for UEFI by itself. I don't
 think people will write UEFI applications with

static
void CALLABI_CROSS do_cpy (char *s, ...)
{
  CROSS_VA_LIST argp;
  CROSS_VA_START (argp, s);
  vdo_cpy (s, argp);
  CROSS_VA_END (argp);
}

They will write normal C code like

void
do_cpy (char *s, ...)
{
  va_list argp;
  va_start (argp, s);
  vdo_cpy (s, argp);
  va_end (argp);
}

which works with -mabi=ms, not ms_abi attribute. That is why
I think it belongs to gcc 4.4 so that people can use system gcc
on Linux/x86-64 to compile UEFI applications.


-- 
H.J.