This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch i386]: Add support of "hotfix" -feature for x64


On Wed, Jul 7, 2010 at 3:53 AM, Kai Tietz <ktietz70@googlemail.com> wrote:
> 2010/7/7 Richard Guenther <richard.guenther@gmail.com>:
>> On Wed, Jul 7, 2010 at 10:27 AM, Kai Tietz <Kai.Tietz@onevision.com> wrote:
>>> Hello,
>>>
>>> Jakub Jelinek <jakub@redhat.com> 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.
>>>>
>>>> BTW,
>>>>
>>>> ? bool is_ms_hook = ((decl && ix86_function_ms_hook_prologue (decl)) ?
>>> true
>>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? :
>>> false);
>>>>
>>>> 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).
>>
>> Thanks,
>> Richard.
>
> 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.


-- 
H.J.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]