On 01/09/2018 01:26 AM, Nathan Rossi wrote: > 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? I'd approve a push/pop. THe current patch as-is seems broken to me.
jeff