This is the mail archive of the
mailing list for the GCC project.
Re: [patch i386]: Add support of "hotfix" -feature for x64
On Wed, Jul 7, 2010 at 3:53 AM, Kai Tietz <firstname.lastname@example.org> wrote:
> 2010/7/7 Richard Guenther <email@example.com>:
>> On Wed, Jul 7, 2010 at 10:27 AM, Kai Tietz <Kai.Tietz@onevision.com> wrote:
>>> Jakub Jelinek <firstname.lastname@example.org> wrote on 07.07.2010 09:25:20:
>>>> On Wed, Jul 07, 2010 at 09:15:45AM +0200, Kai Tietz wrote:
>>>> > this is the more improved patch, which handles for Wine linux the
>>>> > hotfix part via ASM_DECLARE_FUNCTION_NAME, too.
>>> I renamed by this patch the SUBTARGET_ASM_DECLARE_FUNCTION_NAME to
>>> ASM_DECLARE_OS_FUNCTION_NAME as suggested.
>>>> This is very ugly. ?An OS isn't a subtarget of any kind.
>>>> If this is really the way to go, use ASM_DECLARE_OS_FUNCTION_NAME
>>>> or something similar.
>>>> ? bool is_ms_hook = ((decl && ix86_function_ms_hook_prologue (decl)) ?
>>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? :
>>>> is horribly ugly. ?You should just use
>>> Cleaned up.
>> I don't see the "cleaned up" part.
> Then look into patch about ix86_asm_declare_function_name in i386.c.
> This was what Jakub was refering to here AFAIU.
>> Can you please revert the original patch that caused the massive
>> breakage on *-linux and post a fixed patch instead (the patch below
>> doesn't make sense to me on its own).
> Isn't the patch for fixing PR/44850 quite obvious? Caused by OS
> headers, which are defining target macros (target is architecture plus
> OS AFAIU), the override of ASM_DECLARE_FUNCTION_NAME needs to be
> splitted here into two parts. As i386 needs to override this hook to
> allow architecture depend logic, the macro in OS headers get renamed
> to ASM_DECLARE_OS_FUNCTION_NAME. As other architecture don't override
> the ASM_DECLARE_FUNCTION_NAME macro, it is necessary to provide the
> default macro implementation of ASM_DECLARE_FUNCTION_NAME, too.
> So, I'll wait for comment of rth before I revert the already approved patch.
I will revert it for now. The latest patch looks wrong to me and
it doesn't work on Darwin.