| Message ID | 1579308383-22325-1-git-send-email-apinski@marvell.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fix PR 93242: patchable-function-entry broken on MIPS | expand |
On Sat, Jan 18, 2020 at 1:47 AM <apinski@marvell.com> wrote: > > From: Andrew Pinski <apinski@marvell.com> > > On MIPS, .set noreorder/reorder needs to emitted around > the nop. The template for the nop instruction uses %(/%) to > do that. But default_print_patchable_function_entry uses > fprintf rather than output_asm_insn to output the instruction. > > This fixes the problem by using output_asm_insn to emit the nop > instruction. > > OK? Bootstrapped and tested on x86_64-linux-gnu and built a full > mips toolchain also. OK. Richard. > Thanks, > Andrew Pinski > > ChangeLog: > > * targhooks.c (default_print_patchable_function_entry): Use > output_asm_insn to emit the nop instruction. > > Change-Id: I9d7cff2fc227a41461b9068e3af1fd3a5a9c059b > --- > gcc/targhooks.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gcc/targhooks.c b/gcc/targhooks.c > index 4819bb8..415c21b 100644 > --- a/gcc/targhooks.c > +++ b/gcc/targhooks.c > @@ -1822,7 +1822,7 @@ default_print_patchable_function_entry (FILE *file, > > unsigned i; > for (i = 0; i < patch_area_size; ++i) > - fprintf (file, "\t%s\n", nop_templ); > + output_asm_insn (nop_templ, NULL); > } > > bool > -- > 1.8.3.1 >
On Mon, 2020-01-20 at 09:42 +0100, Richard Biener wrote: > On Sat, Jan 18, 2020 at 1:47 AM <apinski@marvell.com> wrote: > > From: Andrew Pinski <apinski@marvell.com> > > > > On MIPS, .set noreorder/reorder needs to emitted around > > the nop. The template for the nop instruction uses %(/%) to > > do that. But default_print_patchable_function_entry uses > > fprintf rather than output_asm_insn to output the instruction. > > > > This fixes the problem by using output_asm_insn to emit the nop > > instruction. > > > > OK? Bootstrapped and tested on x86_64-linux-gnu and built a full > > mips toolchain also. > > OK. FWIW, I think this may have broke the arc-elf port. I'm getting failures for the patchable function entry tests. It looks like the port wants to peek a the current_output_insn in its handling of an output punctuation characters and current_output_insn is NULL. jeff >
On Wed, Jan 22, 2020 at 12:48 PM Jeff Law <law@redhat.com> wrote: > > On Mon, 2020-01-20 at 09:42 +0100, Richard Biener wrote: > > On Sat, Jan 18, 2020 at 1:47 AM <apinski@marvell.com> wrote: > > > From: Andrew Pinski <apinski@marvell.com> > > > > > > On MIPS, .set noreorder/reorder needs to emitted around > > > the nop. The template for the nop instruction uses %(/%) to > > > do that. But default_print_patchable_function_entry uses > > > fprintf rather than output_asm_insn to output the instruction. > > > > > > This fixes the problem by using output_asm_insn to emit the nop > > > instruction. > > > > > > OK? Bootstrapped and tested on x86_64-linux-gnu and built a full > > > mips toolchain also. > > > > OK. > FWIW, I think this may have broke the arc-elf port. I'm getting > failures for the patchable function entry tests. It looks like the > port wants to peek a the current_output_insn in its handling of an > output punctuation characters and current_output_insn is NULL. I suspect arc-elf was failing beforehand; just not crashing the compiler :). Before this patch we would be printing out "nop%?" for arc-elf. The tests are "compile" so they would have "passed" but only because the tests was not trying to assemble them. If someone had tried to use this option of arc-elf, they would have ran into a similar problem as mips, printing out %? (in arc case). Thanks, Andrew Pinski > > jeff > > >
On Wed, Jan 22, 2020 at 1:16 PM Andrew Pinski <pinskia@gmail.com> wrote: > > On Wed, Jan 22, 2020 at 12:48 PM Jeff Law <law@redhat.com> wrote: > > > > On Mon, 2020-01-20 at 09:42 +0100, Richard Biener wrote: > > > On Sat, Jan 18, 2020 at 1:47 AM <apinski@marvell.com> wrote: > > > > From: Andrew Pinski <apinski@marvell.com> > > > > > > > > On MIPS, .set noreorder/reorder needs to emitted around > > > > the nop. The template for the nop instruction uses %(/%) to > > > > do that. But default_print_patchable_function_entry uses > > > > fprintf rather than output_asm_insn to output the instruction. > > > > > > > > This fixes the problem by using output_asm_insn to emit the nop > > > > instruction. > > > > > > > > OK? Bootstrapped and tested on x86_64-linux-gnu and built a full > > > > mips toolchain also. > > > > > > OK. > > FWIW, I think this may have broke the arc-elf port. I'm getting > > failures for the patchable function entry tests. It looks like the > > port wants to peek a the current_output_insn in its handling of an > > output punctuation characters and current_output_insn is NULL. > > I suspect arc-elf was failing beforehand; just not crashing the compiler :). > Before this patch we would be printing out "nop%?" for arc-elf. The > tests are "compile" so they would have "passed" but only because the > tests was not trying to assemble them. If someone had tried to use > this option of arc-elf, they would have ran into a similar problem as > mips, printing out %? (in arc case). Just a quick survey of the targets which had an issue before this patch: * arc - uses %? * bpf - uses %% * mips - uses %(/%) which emits .set noreorder/reorder * nios2 - %. * s390 - %% nios2 - checks current_output_insn for null-ness when it is seeing if it needs to print . or not. arc - does not check current_output_insn for null-ness Thanks, Andrew Pinski > > Thanks, > Andrew Pinski > > > > > > jeff > > > > >
On Wed, 2020-01-22 at 13:16 -0800, Andrew Pinski wrote: > On Wed, Jan 22, 2020 at 12:48 PM Jeff Law <law@redhat.com> wrote: > > On Mon, 2020-01-20 at 09:42 +0100, Richard Biener wrote: > > > On Sat, Jan 18, 2020 at 1:47 AM <apinski@marvell.com> wrote: > > > > From: Andrew Pinski <apinski@marvell.com> > > > > > > > > On MIPS, .set noreorder/reorder needs to emitted around > > > > the nop. The template for the nop instruction uses %(/%) to > > > > do that. But default_print_patchable_function_entry uses > > > > fprintf rather than output_asm_insn to output the instruction. > > > > > > > > This fixes the problem by using output_asm_insn to emit the nop > > > > instruction. > > > > > > > > OK? Bootstrapped and tested on x86_64-linux-gnu and built a full > > > > mips toolchain also. > > > > > > OK. > > FWIW, I think this may have broke the arc-elf port. I'm getting > > failures for the patchable function entry tests. It looks like the > > port wants to peek a the current_output_insn in its handling of an > > output punctuation characters and current_output_insn is NULL. > > I suspect arc-elf was failing beforehand; just not crashing the compiler :). > Before this patch we would be printing out "nop%?" for arc-elf. The > tests are "compile" so they would have "passed" but only because the > tests was not trying to assemble them. If someone had tried to use > this option of arc-elf, they would have ran into a similar problem as > mips, printing out %? (in arc case). The tester complains when something changes from pass to fail, it does not complain for new tests that fail. Jeff
diff --git a/gcc/targhooks.c b/gcc/targhooks.c index 4819bb8..415c21b 100644 --- a/gcc/targhooks.c +++ b/gcc/targhooks.c @@ -1822,7 +1822,7 @@ default_print_patchable_function_entry (FILE *file, unsigned i; for (i = 0; i < patch_area_size; ++i) - fprintf (file, "\t%s\n", nop_templ); + output_asm_insn (nop_templ, NULL); } bool
From: Andrew Pinski <apinski@marvell.com> On MIPS, .set noreorder/reorder needs to emitted around the nop. The template for the nop instruction uses %(/%) to do that. But default_print_patchable_function_entry uses fprintf rather than output_asm_insn to output the instruction. This fixes the problem by using output_asm_insn to emit the nop instruction. OK? Bootstrapped and tested on x86_64-linux-gnu and built a full mips toolchain also. Thanks, Andrew Pinski ChangeLog: * targhooks.c (default_print_patchable_function_entry): Use output_asm_insn to emit the nop instruction. Change-Id: I9d7cff2fc227a41461b9068e3af1fd3a5a9c059b --- gcc/targhooks.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)