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 x86/darwin] fix PR51784 'PIC register not correctly preserved...'


On Fri, Jul 19, 2013 at 3:47 PM, Iain Sandoe <iain@codesourcery.com> wrote:
> Hi Uros,
>
> thanks for your reviews,
>
> On 18 Jul 2013, at 12:39, Uros Bizjak wrote:
>
>> On Thu, Jul 18, 2013 at 12:12 PM, Iain Sandoe <iain@codesourcery.com> wrote:
>>>
>>> So, I think we have to use the define_insn_and_split, or am I still missing something?
>>
>> Just a wild guess, do you also need "&& reload_completed" in the split
>> condition?
>
> good catch, thanks - this got cut erroneously from the last variant of the patch.
>
> Fixed & re-tested on x86_64-darwin12 / x86_64-linux (both at m32 and m64) showing the expected progressions on darwin (and correct behaviour on linux for the shlib example).

> gcc/
>
>         PR target/51784
>         * config/i386/i386.c (output_set_got) [TARGET_MACHO]: Adjust to emit a second label for
>         nonlocal goto receivers. Don't output pic base labels unless we're producing PIC; mark
>         that action unreachable().
>         (ix86_save_reg): If the function contains a nonlocal label, save the PIC base reg.
>         * config/darwin-protos.h (machopic_should_output_picbase_label): New.
>         * gcc/config/darwin.c (emitted_pic_label_num): New GTY.
>         (update_pic_label_number_if_needed): New.
>         (machopic_output_function_base_name): Adjust for nonlocal receiver case.
>         (machopic_should_output_picbase_label): New.
>         * config/i386/i386.md (enum unspecv): UNSPECV_NLGR: New.
>         (nonlocal_goto_receiver): New insn and split.
> OK for trunk?

Assuming that Jakub is OK with the patch, it is OK for trunk.

> Ok for open branches? (this is a wrong-code bug)

OK from the maintainer POV, but also needs release manager approval.

I'd suggest a small improvement:

+++ b/gcc/config/i386/i386.c
@@ -8827,10 +8827,8 @@ output_set_got (rtx dest, rtx label ATTRIBUTE_UNUSED)
       output_asm_insn ("mov%z0\t{%2, %0|%0, %2}", xops);

 #if TARGET_MACHO
-      /* Output the Mach-O "canonical" label name ("Lxx$pb") here too.  This
-         is what will be referenced by the Mach-O PIC subsystem.  */
-      if (!label)
- ASM_OUTPUT_LABEL (asm_out_file, MACHOPIC_FUNCTION_BASE_NAME);
+      /* We don't need a pic base, we're not producing pic.  */
+      gcc_unreachable ();
 #endif

Put this part just after the opening curly brace. There is no need to
calculate xops and output asm insn for TARGET_MACHO.

Thanks,
Uros.


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