Bug 68662 - [6 regression] FAIL: gcc.dg/lto/20090210 c_lto_20090210_0.o-c_lto_20090210_1.o link, -O2 -flto -flto-partition=none -fuse-linker-plugin -fno-fat-lto-objects
Summary: [6 regression] FAIL: gcc.dg/lto/20090210 c_lto_20090210_0.o-c_lto_20090210_1....
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 6.0
: P1 normal
Target Milestone: 6.0
Assignee: Alan Modra
URL: https://gcc.gnu.org/ml/gcc-patches/20...
Depends on:
Blocks: 67548
  Show dependency treegraph
Reported: 2015-12-02 19:49 UTC by Andreas Schwab
Modified: 2016-05-05 00:07 UTC (History)
9 users (show)

See Also:
Target: powerpc-*-*
Known to work:
Known to fail:
Last reconfirmed: 2015-12-04 00:00:00

gcc6-pr68662.patch (1.04 KB, patch)
2016-01-26 12:36 UTC, Jakub Jelinek
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Schwab 2015-12-02 19:49:40 UTC
$ gcc/xgcc -Bgcc/ -O2 -flto -flto-partition=none -fuse-linker-plugin -fno-fat-lto-objects -c -m32 -o c_lto_20090210_0.o ../gcc/testsuite/gcc.dg/lto/20090210_0.c
$ gcc/xgcc -Bgcc/ -O2 -flto -flto-partition=none -fuse-linker-plugin -fno-fat-lto-objects -fPIC -c -m32 -o c_lto_20090210_1.o ../gcc/testsuite/gcc.dg/lto/20090210_1.c
$ gcc/xgcc -Bgcc/ c_lto_20090210_0.o c_lto_20090210_1.o -O2 -flto -flto-partition=none -fuse-linker-plugin -fno-fat-lto-objects -m32 -o gcc-dg-lto-20090210-11.exe -save-temps
c_lto_20090210_0.s: Assembler messages:
c_lto_20090210_0.s:30: Error: syntax error; end of line, expected `('
lto-wrapper: fatal error: gcc/xgcc returned 1 exit status
compilation terminated.
/usr/powerpc64-suse-linux/bin/ld: lto-wrapper failed
collect2: error: ld returned 1 exit status
$ tail -n +30 c_lto_20090210_0.s | head -n 1
        lwz 28,.LC0-(30)

4c4a180dae54e5a879c76015b3ed181283190872 is the first bad commit
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@230915 138bc75d-0d04-0410-961f-82ee72b054a4
Comment 1 Jan Hubicka 2015-12-02 20:18:49 UTC
That looks like (latent?) bug in the target asm output - we should not produce invalid asm based on visibility changes.  I will see if I can reproduce it on gcc compile farm.
Comment 2 Alan Modra 2015-12-04 06:12:50 UTC
Confirmed.  Also the -O0 variant of this test.  I had a look at assembly output and see lines like
 addis 30,30,-.L7@ha

This comes from rs6000_emit_load_toc_table use of toc_label_name which appears to be all-zero.  toc_label_name is supposed to be set in rs6000_option_override_internal, but apparently isn't because when rs6000_option_override_internal runs, TARGET_TOC is not true.  Making the initialisation unconditional isn't enough to cure the problem as that leads to linker errors about an undefined reference to ".LCTOC1".  That is because code to set .LCTOC1 isn't being emitted from rs6000_elf_output_toc_section_asm_op.
I haven't verified this, but I'd guess that is because rs6000_file_start doesn't see flag_pic == 2, and therefore doesn't call switch_to_section (toc_section).

Notice that c_lto_20090210_0.o does *not* have -fPIC, while c_lto_20090210_1.o does.
Comment 3 Jan Hubicka 2016-01-18 19:47:29 UTC
OK, this looks more like target issue  so I am changing component. flag_pic is a global option and it is merged by the lto wrapper and corrected based on the linker output. Because static executable is built, flag_pic should be 0. 

Targets must be ready for -fPIC being different at compile time and link time and ought to honor the link time setting...
Comment 4 Jakub Jelinek 2016-01-26 12:36:11 UTC
Created attachment 37475 [details]

So what about this untested patch?  ASM_GENERATE_INTERNAL_LABEL doesn't look to be too expensive to do unconditionally once, and for GC sanity it needs to go through ggc_strdup anyway (it is invalid to refer to non-GC allocated strings in GTY const char * fields that are not skipped by GC).
Comment 5 David Edelsohn 2016-01-26 13:53:06 UTC
Unconditionally generating toc_label_name is okay with me, but I thought that Alan commented it was not sufficient in Comment #2.
Comment 6 Jakub Jelinek 2016-01-26 19:54:09 UTC
Author: jakub
Date: Tue Jan 26 19:53:37 2016
New Revision: 232844

URL: https://gcc.gnu.org/viewcvs?rev=232844&root=gcc&view=rev
	PR target/68662
	* config/rs6000/rs6000.c (rs6000_option_override_internal): Initialize
	toc_label_name unconditionally.
	(rs6000_emit_load_toc_table): Call ggc_strdup on toc_label_name for
	SYMBOL_REF string.  Use toc_label_name instead of constructing
	(rs6000_elf_declare_function_name): Use toc_label_name instead of
	constructing LCTOC1.

Comment 7 Jakub Jelinek 2016-01-27 13:58:40 UTC
Seems like powerpc* has lots of other issues related to mixing pic and non-pic code.  E.g.
int x;

foo (void)
  return x;

__attribute__((optimize ("PIC"))) int
bar (void)
  return x;

seems to ICE due to endless recursion with -O2 -m32 (every force_reg causes another force_reg, at least in x86_64-linux -> powerpc64-linux cross).
Comment 8 Peter Bergner 2016-01-27 14:57:34 UTC
(In reply to Jakub Jelinek from comment #7)
> seems to ICE due to endless recursion with -O2 -m32 (every force_reg causes
> another force_reg, at least in x86_64-linux -> powerpc64-linux cross).

I see a similar failure on a native compile too.
Comment 9 Alan Modra 2016-01-27 22:32:22 UTC
For the testcase in comment #7, global_options are inconsistent (*) and wrong when compiling foo.  I see flag_pic == 2 there??

(*) In particular, TARGET_RELOCATABLE and flag_pic don't agree.  See config/rs6000/sysv4.h SUBTARGET_OVERRIDE_OPTIONS.  flag_pic == 2 ought to mean TARGET_RELOCATABLE is true, but TARGET_RELOCATABLE is false.  This combination is not recognized in rs6000_emit_move.
Comment 10 Alan Modra 2016-01-27 22:50:14 UTC
I guess rs6000 needs to implement targetm.override_options_after_change() if we're to keep flag_pic and TARGET_RELOCATABLE consistent.
Comment 11 Alan Modra 2016-01-28 14:15:19 UTC
Making the toc initialization in rs6000_file_start not depend on flag_pic, ie. use
      switch_to_section (toc_section);
      switch_to_section (text_section);

and applying Jakub's option handling fix for pr61925 to the rs6000 backend, and fixing a testcase seems to cure this problem.

However, I'll note that the comment #7 testcase fails to compile foo non-PIC, even on x86 -m32.  Looks like flag_pic and a whole bunch of other flags are not saved/restored around attribute((optimize())).
Comment 12 Peter Bergner 2016-01-28 15:30:32 UTC
Should we add an assert somewhere to ensure that flag_pic and TARGET_RELOCATABLE are consistent?
Comment 13 Alan Modra 2016-02-02 00:01:49 UTC
Author: amodra
Date: Tue Feb  2 00:01:16 2016
New Revision: 233061

URL: https://gcc.gnu.org/viewcvs?rev=233061&root=gcc&view=rev
[RS6000] ABI_V4 init of toc section

Since 4c4a180d lto has turned off flag_pic when linking a fixed
position executable.  So flag_pic is zero in rs6000_file_start.
However, when we get to actually emitting code, flag_pic may be on
again.  This results in undefined references to ".LCTOC1".

	PR target/68662
	* config/rs6000/rs6000.c (need_toc_init): New var, set it
	whenever toc_label_name used.
	(rs6000_file_start): Don't set up toc section here,
	(rs6000_output_function_epilogue): do so here instead,
	(rs6000_xcoff_file_start): and here.
	* config/rs6000/rs6000.md (load_toc_aix_si): Set need_toc_init.
	(load_toc_aix_di): Likewise.

Comment 14 Alan Modra 2016-02-02 02:03:20 UTC
Fixed with workaround.  https://gcc.gnu.org/ml/gcc-patches/2016-02/msg00002.html queued for gcc-7 and 6.1.
Comment 15 Alan Modra 2016-05-05 00:07:59 UTC
Author: amodra
Date: Thu May  5 00:07:27 2016
New Revision: 235914

URL: https://gcc.gnu.org/viewcvs?rev=235914&root=gcc&view=rev

For ABI_V4, -mrelocatable and -fPIC both generate position independent
code, with some extra "fixup" output for -mrelocatable.  The
similarity of these two options has led to the situation where the
sysv4.h SUBTARGET_OVERRIDE_OPTIONS sets flag_pic on seeing
-mrelocatable, and sets TARGET_RELOCATABLE on seeing -fPIC.  That
prevents LTO from properly optimizing position dependent executables,
because the mutual dependence of the flags and the fact that LTO
streaming records the state of rs6000_isa_flags, result in flag_pic
being set when it shouldn't be.

So, don't set TARGET_RELOCATABLE when -fPIC.  Places that currently
test TARGET_RELOCATABLE can instead test
or since TARGET_RELOCATABLE can only be enabled when ABI_V4,
DEFAULT_ABI == ABI_V4 && (TARGET_RELOCATABLE || flag_pic > 1).

Also, since flag_pic is set by -mrelocatable, a number of places that
currently test TARGET_RELOCATABLE can be simplified.  I also made
-mrelocatable set TARGET_NO_FP_IN_TOC, allowing TARGET_RELOCATABLE to
be removed from ASM_OUTPUT_SPECIAL_POOL_ENTRY_P.  Reducing occurrences
of TARGET_RELOCATABLE is a good thing.

	PR target/68662
	* config/rs6000/sysv4.h (SUBTARGET_OVERRIDE_OPTIONS): Don't
	set OPTION_MASK_RELOCATABLE when flag_pic == 2.  Set
	TARGET_NO_FP_IN_TOC for -mrelocatable.
	(MINIMAL_TOC_SECTION_ASM_OP): Remove redundant
	* config/rs6000/linux64.h (MINIMAL_TOC_SECTION_ASM_OP): Likewise.
	* config/rs6000/freebsd64.h (MINIMAL_TOC_SECTION_ASM_OP): Likewise.
	* config/rs6000/predicates.md (easy_fp_constant): Likewise.
	* config/rs6000/rs6000.c (rs6000_elf_output_toc_section_asm_op):
	(rs6000_assemble_integer): Update TARGET_RELOCATABLE test.
	(rs6000_stack_info): Likewise.
	(rs6000_elf_asm_out_constructor): Likewise.
	(rs6000_elf_asm_out_destructor): Likewise.
	(rs6000_elf_declare_function_name): Likewise.
	* config/rs6000/rs6000.md (load_toc_aix_di): Likewise.
	* config/rs6000/rs6000.h (MASK_RELOCATABLE, MASK_MINIMAL_TOC):
	Don't define.