| Message ID | 20120928092438.1194A40E44@hchopra.mtv.corp.google.com |
|---|---|
| State | New |
| Headers | show |
Harshit, why didn't you propose this patch for trunk? Why should we make it a google-local patch? Diego. On Fri, Sep 28, 2012 at 5:24 AM, Harshit Chopra <harshit@google.com> wrote: > commit fc3a55ccec9bc770c79f8a221f5abd397befc8f6 > Author: Harshit Chopra <harshit@google.com> > Date: Thu Sep 20 17:49:59 2012 -0700 > > Instead of emitting absolute addresses to the function patch sections, emit relative addresses. Absolute addresses might require relocation, which is time consuming and fraught with other issues. > > M gcc/config/i386/i386.c > > Tested: > Ran make check-gcc and manually confirmed that the affected tests pass. > > ChangeLog: > > 2012-09-28 Harshit Chopra <harshit@google.com> > > * gcc/config/i386/i386.c (ix86_output_function_nops_prologue_epilogue): Emit relative address to function patch sections. > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > index f72b0b5..8c9334f 100644 > --- a/gcc/config/i386/i386.c > +++ b/gcc/config/i386/i386.c > @@ -11098,7 +11098,7 @@ ix86_output_function_nops_prologue_epilogue (FILE *file, > $LFPEL0: > <pre_instruction> > 0x90 (repeated num_actual_nops times) > - .quad $LFPESL0 > + .quad $LFPESL0 - . > followed by section 'section_name' which contains the address > of instruction at 'label'. > */ > @@ -11110,7 +11110,10 @@ ix86_output_function_nops_prologue_epilogue (FILE *file, > asm_fprintf (file, ASM_BYTE"0x90\n"); > > fprintf (file, ASM_QUAD); > + /* Output "section_label - ." for the relative address of the entry in > + the section 'section_name'. */ > assemble_name_raw (file, section_label); > + fprintf (file, " - ."); > fprintf (file, "\n"); > > /* Emit the backpointer section. For functions belonging to comdat group, > @@ -11144,7 +11147,7 @@ ix86_output_function_nops_prologue_epilogue (FILE *file, > .quad $LFPEL0 > */ > ASM_OUTPUT_INTERNAL_LABEL (file, section_label); > - fprintf(file, ASM_QUAD"\t"); > + fprintf(file, ASM_QUAD); > assemble_name_raw (file, label); > fprintf (file, "\n"); > > > -- > This patch is available for review at http://codereview.appspot.com/6572065
Harshit, why didn't you propose this patch for trunk? Why should we make it a google-local patch? Diego. On Fri, Sep 28, 2012 at 5:24 AM, Harshit Chopra <harshit@google.com> wrote: > commit fc3a55ccec9bc770c79f8a221f5abd397befc8f6 > Author: Harshit Chopra <harshit@google.com> > Date: Thu Sep 20 17:49:59 2012 -0700 > > Instead of emitting absolute addresses to the function patch sections, emit relative addresses. Absolute addresses might require relocation, which is time consuming and fraught with other issues. > > M gcc/config/i386/i386.c > > Tested: > Ran make check-gcc and manually confirmed that the affected tests pass. > > ChangeLog: > > 2012-09-28 Harshit Chopra <harshit@google.com> > > * gcc/config/i386/i386.c (ix86_output_function_nops_prologue_epilogue): Emit relative address to function patch sections. > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > index f72b0b5..8c9334f 100644 > --- a/gcc/config/i386/i386.c > +++ b/gcc/config/i386/i386.c > @@ -11098,7 +11098,7 @@ ix86_output_function_nops_prologue_epilogue (FILE *file, > $LFPEL0: > <pre_instruction> > 0x90 (repeated num_actual_nops times) > - .quad $LFPESL0 > + .quad $LFPESL0 - . > followed by section 'section_name' which contains the address > of instruction at 'label'. > */ > @@ -11110,7 +11110,10 @@ ix86_output_function_nops_prologue_epilogue (FILE *file, > asm_fprintf (file, ASM_BYTE"0x90\n"); > > fprintf (file, ASM_QUAD); > + /* Output "section_label - ." for the relative address of the entry in > + the section 'section_name'. */ > assemble_name_raw (file, section_label); > + fprintf (file, " - ."); > fprintf (file, "\n"); > > /* Emit the backpointer section. For functions belonging to comdat group, > @@ -11144,7 +11147,7 @@ ix86_output_function_nops_prologue_epilogue (FILE *file, > .quad $LFPEL0 > */ > ASM_OUTPUT_INTERNAL_LABEL (file, section_label); > - fprintf(file, ASM_QUAD"\t"); > + fprintf(file, ASM_QUAD); > assemble_name_raw (file, label); > fprintf (file, "\n"); > > > -- > This patch is available for review at http://codereview.appspot.com/6572065
On Fri, Oct 5, 2012 at 6:53 PM, Diego Novillo <dnovillo@google.com> wrote: > Harshit, why didn't you propose this patch for trunk? Why should we > make it a google-local patch? In the meantime, let's put it in the google branches. Please make sure that you ping the upstream patch. It will need more testing than just make check-gcc. You need to do a full bootstrap with all default languages. Also, make sure that the motivation and benefits of the patch are very clear. Thanks. Diego.
xray feature is not in trunk yet. David On Fri, Oct 5, 2012 at 3:53 PM, Diego Novillo <dnovillo@google.com> wrote: > Harshit, why didn't you propose this patch for trunk? Why should we > make it a google-local patch? > > > Diego. > > On Fri, Sep 28, 2012 at 5:24 AM, Harshit Chopra <harshit@google.com> wrote: >> commit fc3a55ccec9bc770c79f8a221f5abd397befc8f6 >> Author: Harshit Chopra <harshit@google.com> >> Date: Thu Sep 20 17:49:59 2012 -0700 >> >> Instead of emitting absolute addresses to the function patch sections, emit relative addresses. Absolute addresses might require relocation, which is time consuming and fraught with other issues. >> >> M gcc/config/i386/i386.c >> >> Tested: >> Ran make check-gcc and manually confirmed that the affected tests pass. >> >> ChangeLog: >> >> 2012-09-28 Harshit Chopra <harshit@google.com> >> >> * gcc/config/i386/i386.c (ix86_output_function_nops_prologue_epilogue): Emit relative address to function patch sections. >> >> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c >> index f72b0b5..8c9334f 100644 >> --- a/gcc/config/i386/i386.c >> +++ b/gcc/config/i386/i386.c >> @@ -11098,7 +11098,7 @@ ix86_output_function_nops_prologue_epilogue (FILE *file, >> $LFPEL0: >> <pre_instruction> >> 0x90 (repeated num_actual_nops times) >> - .quad $LFPESL0 >> + .quad $LFPESL0 - . >> followed by section 'section_name' which contains the address >> of instruction at 'label'. >> */ >> @@ -11110,7 +11110,10 @@ ix86_output_function_nops_prologue_epilogue (FILE *file, >> asm_fprintf (file, ASM_BYTE"0x90\n"); >> >> fprintf (file, ASM_QUAD); >> + /* Output "section_label - ." for the relative address of the entry in >> + the section 'section_name'. */ >> assemble_name_raw (file, section_label); >> + fprintf (file, " - ."); >> fprintf (file, "\n"); >> >> /* Emit the backpointer section. For functions belonging to comdat group, >> @@ -11144,7 +11147,7 @@ ix86_output_function_nops_prologue_epilogue (FILE *file, >> .quad $LFPEL0 >> */ >> ASM_OUTPUT_INTERNAL_LABEL (file, section_label); >> - fprintf(file, ASM_QUAD"\t"); >> + fprintf(file, ASM_QUAD); >> assemble_name_raw (file, label); >> fprintf (file, "\n"); >> >> >> -- >> This patch is available for review at http://codereview.appspot.com/6572065
Ok for google branches. Please consider resend the original xray patch to trunk (gcc-4_8) You need to make the runtime bits available publicly though. thanks, David On Fri, Sep 28, 2012 at 2:24 AM, Harshit Chopra <harshit@google.com> wrote: > commit fc3a55ccec9bc770c79f8a221f5abd397befc8f6 > Author: Harshit Chopra <harshit@google.com> > Date: Thu Sep 20 17:49:59 2012 -0700 > > Instead of emitting absolute addresses to the function patch sections, emit relative addresses. Absolute addresses might require relocation, which is time consuming and fraught with other issues. > > M gcc/config/i386/i386.c > > Tested: > Ran make check-gcc and manually confirmed that the affected tests pass. > > ChangeLog: > > 2012-09-28 Harshit Chopra <harshit@google.com> > > * gcc/config/i386/i386.c (ix86_output_function_nops_prologue_epilogue): Emit relative address to function patch sections. > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > index f72b0b5..8c9334f 100644 > --- a/gcc/config/i386/i386.c > +++ b/gcc/config/i386/i386.c > @@ -11098,7 +11098,7 @@ ix86_output_function_nops_prologue_epilogue (FILE *file, > $LFPEL0: > <pre_instruction> > 0x90 (repeated num_actual_nops times) > - .quad $LFPESL0 > + .quad $LFPESL0 - . > followed by section 'section_name' which contains the address > of instruction at 'label'. > */ > @@ -11110,7 +11110,10 @@ ix86_output_function_nops_prologue_epilogue (FILE *file, > asm_fprintf (file, ASM_BYTE"0x90\n"); > > fprintf (file, ASM_QUAD); > + /* Output "section_label - ." for the relative address of the entry in > + the section 'section_name'. */ > assemble_name_raw (file, section_label); > + fprintf (file, " - ."); > fprintf (file, "\n"); > > /* Emit the backpointer section. For functions belonging to comdat group, > @@ -11144,7 +11147,7 @@ ix86_output_function_nops_prologue_epilogue (FILE *file, > .quad $LFPEL0 > */ > ASM_OUTPUT_INTERNAL_LABEL (file, section_label); > - fprintf(file, ASM_QUAD"\t"); > + fprintf(file, ASM_QUAD); > assemble_name_raw (file, label); > fprintf (file, "\n"); > > > -- > This patch is available for review at http://codereview.appspot.com/6572065
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index f72b0b5..8c9334f 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -11098,7 +11098,7 @@ ix86_output_function_nops_prologue_epilogue (FILE *file, $LFPEL0: <pre_instruction> 0x90 (repeated num_actual_nops times) - .quad $LFPESL0 + .quad $LFPESL0 - . followed by section 'section_name' which contains the address of instruction at 'label'. */ @@ -11110,7 +11110,10 @@ ix86_output_function_nops_prologue_epilogue (FILE *file, asm_fprintf (file, ASM_BYTE"0x90\n"); fprintf (file, ASM_QUAD); + /* Output "section_label - ." for the relative address of the entry in + the section 'section_name'. */ assemble_name_raw (file, section_label); + fprintf (file, " - ."); fprintf (file, "\n"); /* Emit the backpointer section. For functions belonging to comdat group, @@ -11144,7 +11147,7 @@ ix86_output_function_nops_prologue_epilogue (FILE *file, .quad $LFPEL0 */ ASM_OUTPUT_INTERNAL_LABEL (file, section_label); - fprintf(file, ASM_QUAD"\t"); + fprintf(file, ASM_QUAD); assemble_name_raw (file, label); fprintf (file, "\n");
commit fc3a55ccec9bc770c79f8a221f5abd397befc8f6 Author: Harshit Chopra <harshit@google.com> Date: Thu Sep 20 17:49:59 2012 -0700 Instead of emitting absolute addresses to the function patch sections, emit relative addresses. Absolute addresses might require relocation, which is time consuming and fraught with other issues. M gcc/config/i386/i386.c Tested: Ran make check-gcc and manually confirmed that the affected tests pass. ChangeLog: 2012-09-28 Harshit Chopra <harshit@google.com> * gcc/config/i386/i386.c (ix86_output_function_nops_prologue_epilogue): Emit relative address to function patch sections. -- This patch is available for review at http://codereview.appspot.com/6572065