[PATCH] MicroBlaze resolve section change issues for ident output
Change 'microblaze_asm_output_ident' so that instead of just switching
section to .sdata2/.rodata it uses the assembler .pushsection and
.popsection directives to preserve the section context when emitting the
ident string content.
This resolves issues associated with the use of the .sdata2/.rodata
operation in cases where emitted assembly after the ident output is
incorrectly in the .sdata2/.rodata section instead of .text or any other
expected section. This results in assembly failures including operations
with symbols across different segments.
gcc/ChangeLog
2018-01-11 Nathan Rossi <nat...@nathanrossi.com>
PR target/83013
* config/microblaze/microblaze.c (microblaze_asm_output_ident):
Use .pushsection/.popsection
---
gcc/config/microblaze/microblaze.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/gcc/config/microblaze/microblaze.c
b/gcc/config/microblaze/microblaze.c
index 51511ca2fc..9a4a287be2 100644
--- a/gcc/config/microblaze/microblaze.c
+++ b/gcc/config/microblaze/microblaze.c
@@ -3394,7 +3394,9 @@ microblaze_asm_output_ident (const char *string)
else
section_asm_op = READONLY_DATA_SECTION_ASM_OP;
- buf = ACONCAT ((section_asm_op, "\n\t.ascii \"", string, "\\0\"\n", NULL));
+ buf = ACONCAT (("\t.pushsection", section_asm_op,
+ "\n\t.ascii \"", string, "\\0\"\n",
+ "\t.popsection\n", NULL));
symtab->finalize_toplevel_asm (build_string (strlen (buf), buf));
}
--
2.15.1
Re: [PATCH] MicroBlaze use default ident output generation
On 18 November 2017 at 22:13, Nathan Rossi <nat...@nathanrossi.com> wrote: > On 18 November 2017 at 04:25, Jeff Law <l...@redhat.com> wrote: >> On 11/15/2017 11:58 PM, Nathan Rossi wrote: >>> Remove the MicroBlaze specific TARGET_ASM_OUTPUT_IDENT definition, and >>> use the default. >>> >>> This resolves issues associated with the use of the .sdata2 operation in >>> cases where emitted assembly after the ident output is incorrectly in >>> the .sdata2 section instead of .text or any other expected section. >>> Which results in assembly failures including operations with symbols >>> across different segments. >>> >>> gcc/ChangeLog >>> >>> 2017-11-16 Nathan Rossi <nat...@nathanrossi.com> >>> >>> PR target/83013 >>> * config/microblaze/microblaze-protos.h >>> (microblaze_asm_output_ident): Delete >>> * config/microblaze/microblaze.c (microblaze_asm_output_ident): Delete >>> * config/microblaze/microblaze.h (TARGET_ASM_OUTPUT_IDENT): Default >> But isn't the purpose of the override to force certain small-enough >> objects into the .sdata2 section and by removing the override aren't you >> losing that capability? >> >> It does seem like the override is broken in that it changes the current >> section behind the back of the generic code. >> >> Wouldn't a better fix be to ensure that the override arranges to switch >> back to whatever the current section is? Perhaps using .pushsection and >> .popsection would help here? >> > > That would be a better fix, however I sent this change first as it > seemed it might be preferred to remove the target specific behavior > instead of fixing it. Since it is the only target that actually uses > the TARGET_ASM_OUTPUT_IDENT to change the output asm content (others > either define the default or have a function that calls the default). > > But I can sort out a patch that fixes the behavior instead if that is > preferred? Ping. Should I sort out a patch which uses the push/pop of the section or is this patch preferred? Regards, Nathan
Re: [PATCH] MicroBlaze use default ident output generation
On 18 November 2017 at 04:30, Michael Eager <ea...@eagerm.com> wrote: > On 11/15/2017 10:58 PM, Nathan Rossi wrote: >> >> Remove the MicroBlaze specific TARGET_ASM_OUTPUT_IDENT definition, and >> use the default. >> >> This resolves issues associated with the use of the .sdata2 operation in >> cases where emitted assembly after the ident output is incorrectly in >> the .sdata2 section instead of .text or any other expected section. >> Which results in assembly failures including operations with symbols >> across different segments. > > > Can you give me an example where this causes a problem? Sure, I filed a GCC bugzilla with a sample code file that exhibits the cross-segment symbol issue. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83013 There was also an older bug I saw before filing the above which might be the same issue though I have not verified that. Just looking at the .s files from the linked binutils issue appears to show the same issue however with .rodata instead of .sdata2. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63261 Regards, Nathan
Re: [PATCH] MicroBlaze use default ident output generation
On 18 November 2017 at 04:25, Jeff Law <l...@redhat.com> wrote: > On 11/15/2017 11:58 PM, Nathan Rossi wrote: >> Remove the MicroBlaze specific TARGET_ASM_OUTPUT_IDENT definition, and >> use the default. >> >> This resolves issues associated with the use of the .sdata2 operation in >> cases where emitted assembly after the ident output is incorrectly in >> the .sdata2 section instead of .text or any other expected section. >> Which results in assembly failures including operations with symbols >> across different segments. >> >> gcc/ChangeLog >> >> 2017-11-16 Nathan Rossi <nat...@nathanrossi.com> >> >> PR target/83013 >> * config/microblaze/microblaze-protos.h >> (microblaze_asm_output_ident): Delete >> * config/microblaze/microblaze.c (microblaze_asm_output_ident): Delete >> * config/microblaze/microblaze.h (TARGET_ASM_OUTPUT_IDENT): Default > But isn't the purpose of the override to force certain small-enough > objects into the .sdata2 section and by removing the override aren't you > losing that capability? > > It does seem like the override is broken in that it changes the current > section behind the back of the generic code. > > Wouldn't a better fix be to ensure that the override arranges to switch > back to whatever the current section is? Perhaps using .pushsection and > .popsection would help here? > That would be a better fix, however I sent this change first as it seemed it might be preferred to remove the target specific behavior instead of fixing it. Since it is the only target that actually uses the TARGET_ASM_OUTPUT_IDENT to change the output asm content (others either define the default or have a function that calls the default). But I can sort out a patch that fixes the behavior instead if that is preferred? Regards, Nathan
[PATCH] MicroBlaze use default ident output generation
Remove the MicroBlaze specific TARGET_ASM_OUTPUT_IDENT definition, and
use the default.
This resolves issues associated with the use of the .sdata2 operation in
cases where emitted assembly after the ident output is incorrectly in
the .sdata2 section instead of .text or any other expected section.
Which results in assembly failures including operations with symbols
across different segments.
gcc/ChangeLog
2017-11-16 Nathan Rossi <nat...@nathanrossi.com>
PR target/83013
* config/microblaze/microblaze-protos.h
(microblaze_asm_output_ident): Delete
* config/microblaze/microblaze.c (microblaze_asm_output_ident): Delete
* config/microblaze/microblaze.h (TARGET_ASM_OUTPUT_IDENT): Default
---
gcc/config/microblaze/microblaze-protos.h | 1 -
gcc/config/microblaze/microblaze.c| 24
gcc/config/microblaze/microblaze.h| 2 +-
3 files changed, 1 insertion(+), 26 deletions(-)
diff --git a/gcc/config/microblaze/microblaze-protos.h
b/gcc/config/microblaze/microblaze-protos.h
index 747ef35971..3d3e8c5a64 100644
--- a/gcc/config/microblaze/microblaze-protos.h
+++ b/gcc/config/microblaze/microblaze-protos.h
@@ -51,7 +51,6 @@ extern int microblaze_regno_ok_for_base_p (int, int);
extern HOST_WIDE_INT microblaze_initial_elimination_offset (int, int);
extern void microblaze_declare_object (FILE *, const char *, const char *,
const char *, int);
-extern void microblaze_asm_output_ident (const char *);
extern int microblaze_legitimate_pic_operand (rtx);
extern bool microblaze_tls_referenced_p (rtx);
extern int symbol_mentioned_p (rtx);
diff --git a/gcc/config/microblaze/microblaze.c
b/gcc/config/microblaze/microblaze.c
index 7487523877..379f4c4d7f 100644
--- a/gcc/config/microblaze/microblaze.c
+++ b/gcc/config/microblaze/microblaze.c
@@ -3377,30 +3377,6 @@ microblaze_eh_return (rtx op0)
emit_insn (gen_movsi (gen_rtx_MEM (Pmode, stack_pointer_rtx), op0));
}
-/* Queue an .ident string in the queue of top-level asm statements.
- If the string size is below the threshold, put it into .sdata2.
- If the front-end is done, we must be being called from toplev.c.
- In that case, do nothing. */
-void
-microblaze_asm_output_ident (const char *string)
-{
- const char *section_asm_op;
- int size;
- char *buf;
-
- if (symtab->state != PARSING)
-return;
-
- size = strlen (string) + 1;
- if (size <= microblaze_section_threshold)
-section_asm_op = SDATA2_SECTION_ASM_OP;
- else
-section_asm_op = READONLY_DATA_SECTION_ASM_OP;
-
- buf = ACONCAT ((section_asm_op, "\n\t.ascii \"", string, "\\0\"\n", NULL));
- symtab->finalize_toplevel_asm (build_string (strlen (buf), buf));
-}
-
static void
microblaze_elf_asm_init_sections (void)
{
diff --git a/gcc/config/microblaze/microblaze.h
b/gcc/config/microblaze/microblaze.h
index 59cc1cc2e3..06155d3163 100644
--- a/gcc/config/microblaze/microblaze.h
+++ b/gcc/config/microblaze/microblaze.h
@@ -705,7 +705,7 @@ do {
\
#define STRING_ASM_OP "\t.asciz\t"
#undef TARGET_ASM_OUTPUT_IDENT
-#define TARGET_ASM_OUTPUT_IDENT microblaze_asm_output_ident
+#define TARGET_ASM_OUTPUT_IDENT default_asm_output_ident_directive
/* Default to -G 8 */
#ifndef MICROBLAZE_DEFAULT_GVALUE
--
2.15.0
