Confirmed.  Reproducible using gnat 4.8 as bootstrap compiler on Ubuntu 14.04.

See also https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77405 which appears to be very similar (slight difference in the error message).

I disagree, Ada was bootstrapping fine on x86-64, x86, SPARC, etc until very recently.  Please investigate at least a little before recategorizing.

This may have been introduced by:

2016-08-03  Peter Bergner  <bergner@vnet.ibm.com>

	* config/rs6000/rs6000.c (rs6000_option_override_internal): Make LRA
	the default for the rs6000 port.

Peter, did you bootstrap Ada before making the switch?

(In reply to Eric Botcazou from comment #4)
> This may have been introduced by:
> 
> 2016-08-03  Peter Bergner  <bergner@vnet.ibm.com>
> 
> 	* config/rs6000/rs6000.c (rs6000_option_override_internal): Make LRA
> 	the default for the rs6000 port.
> 
> Peter, did you bootstrap Ada before making the switch?

I did not, as my normal build scripts don't include ada by default since I use them on some systems that don't have a system gnat installed.  I'll fire off builds with and without the switch to LRA and see if the switch is at fault or not.

Peter

Backtrace info (svn r239837):

Program received signal SIGSEGV, Segmentation fault.
system.secondary_stack.ss_release (m=...) at ../rts/s-secsta.adb:479
479	         To_Stack_Ptr (M.Sstk).Top := M.Sptr;
(gdb) bt
#0  system.secondary_stack.ss_release (m=...) at ../rts/s-secsta.adb:479
#1  0x00000000100dae28 in make.initialize (project_node_tree=0x0, env=...)
    at /home/wschmidt/gcc/gcc-mainline-base/gcc/ada/make.adb:6327
#2  0x00000000100e0c98 in make.gnatmake ()
    at /home/wschmidt/gcc/gcc-mainline-base/gcc/ada/make.adb:5563
#3  0x000000001009be98 in gnatmake ()
    at /home/wschmidt/gcc/gcc-mainline-base/gcc/ada/gnatmake.adb:38
(gdb) i r pc
pc             0x10223460	0x10223460 <system.secondary_stack.ss_release>
(gdb) p M
$1 = (sstk => (system.address) 0x0, sptr => 70368744172512)
(gdb) up
#1  0x00000000100dae28 in make.initialize (project_node_tree=0x0, env=...)
    at /home/wschmidt/gcc/gcc-mainline-base/gcc/ada/make.adb:6327
6327	   procedure Initialize
(gdb) up
#2  0x00000000100e0c98 in make.gnatmake ()
    at /home/wschmidt/gcc/gcc-mainline-base/gcc/ada/make.adb:5563
5563	      Make.Initialize (Project_Node_Tree, Root_Environment);
(gdb) up
#3  0x000000001009be98 in gnatmake ()
    at /home/wschmidt/gcc/gcc-mainline-base/gcc/ada/gnatmake.adb:38
38	   Make.Gnatmake;
(gdb)

I'm afraid I don't know anything about Ada and how its runtime works; it looks like system.secondary_stack.ss_release is called automatically somehow as part of entering make.Initialize, but I have no idea if that's supposed to happen.  It seems questionable at first glance.

It does look possible that this is an LRA issue.  Here's the code right before failure:

    100dae08:   f8 95 22 39     addi    r9,r2,-27144
    100dae0c:   01 00 40 39     li      r10,1
    100dae10:   00 00 49 99     stb     r10,0(r9)
    100dae14:   b8 04 20 39     li      r9,1208
    100dae18:   14 4a 7f 7c     add     r3,r31,r9
    100dae1c:   08 00 83 e8     ld      r4,8(r3)
    100dae20:   00 00 63 e8     ld      r3,0(r3)
    100dae24:   3d 86 14 48     bl      10223460 <system__secondary_stack__ss_re
lease>
    100dae28:   00 00 00 60     nop

At the call, r3 has a value of 0.  It looks quite possible that the stack load at 0x100dae20 is from a spill slot.  I don't see offset 1208 used for a corresponding store anywhere in the function.  There is, however, some odd code that uses that constant to fiddle with the frame pointer and then set it back to where it was:

    100da5d8:   b8 04 20 39     li      r9,1208

    100da5e4:   14 4a ff 7f     add     r31,r31,r9
    100da5e8:   50 f8 e9 7f     subf    r31,r9,r31

Whatever's going on there looks like a bug.

> I'm afraid I don't know anything about Ada and how its runtime works; it
> looks like system.secondary_stack.ss_release is called automatically somehow
> as part of entering make.Initialize, but I have no idea if that's supposed
> to happen.  It seems questionable at first glance.

Unfortunately I don't seem to be able to connect to gcc112 in the CompileFarm:

eric@arcturus:~> ssh -l ebotcazou gcc112.osuosl.org
ssh: Could not resolve hostname gcc112.osuosl.org: Name or service not known

Try to recompile the gnattools with reload instead of LRA, for example:
  rm -rf gnattools gcc/ada/tools gcc/stamp-tools
  make all-gnattools CFLAGS="-O2 -mno-lra"

Does this eliminate the problem?

(In reply to Eric Botcazou from comment #8)
> Unfortunately I don't seem to be able to connect to gcc112 in the
> CompileFarm:
> 
> eric@arcturus:~> ssh -l ebotcazou gcc112.osuosl.org
> ssh: Could not resolve hostname gcc112.osuosl.org: Name or service not known

gcc112.fsffrance.org (aka gcc2-power8.osuosl.org)

> It does look possible that this is an LRA issue.  Here's the code right
> before failure:
> 
>     100dae08:   f8 95 22 39     addi    r9,r2,-27144
>     100dae0c:   01 00 40 39     li      r10,1
>     100dae10:   00 00 49 99     stb     r10,0(r9)
>     100dae14:   b8 04 20 39     li      r9,1208
>     100dae18:   14 4a 7f 7c     add     r3,r31,r9
>     100dae1c:   08 00 83 e8     ld      r4,8(r3)
>     100dae20:   00 00 63 e8     ld      r3,0(r3)
>     100dae24:   3d 86 14 48     bl      10223460
> <system__secondary_stack__ss_re
> lease>
>     100dae28:   00 00 00 60     nop
> 
> At the call, r3 has a value of 0.  It looks quite possible that the stack
> load at 0x100dae20 is from a spill slot.  I don't see offset 1208 used for a
> corresponding store anywhere in the function.  There is, however, some odd
> code that uses that constant to fiddle with the frame pointer and then set
> it back to where it was:
> 
>     100da5d8:   b8 04 20 39     li      r9,1208
> 
>     100da5e4:   14 4a ff 7f     add     r31,r31,r9
>     100da5e8:   50 f8 e9 7f     subf    r31,r9,r31

Here are the declarations of the relevant subroutines:

   type Mark_Id is private;
   --  Type used to mark the stack for mark/release processing

   function SS_Mark return Mark_Id;
   --  Return the Mark corresponding to the current state of the stack

   procedure SS_Release (M : Mark_Id);
   --  Restore the state of the stack corresponding to the mark M. If an
   --  additional chunk have been allocated, it will never be freed during a
   --  ??? missing comment here

   type Mark_Id is record
      Sstk : System.Address;
      Sptr : SS_Ptr;
   end record;
   --  A mark value contains the address of the secondary stack structure,
   --  as returned by System.Soft_Links.Get_Sec_Stack_Addr, and a stack
   --  pointer value corresponding to the point of the mark call.

So the double-word load before the call to SS_Release should be from a Mark_Id object obtained from a preceding call to SS_Mark.  It indeed looks like the double-word store to this Mark_Id object has been optimized away, possibly because of the strange back-and-forth adjustment to the FP.  Does passing CFLAGS="-O2 -fno-dse" for the gnattools make a difference?

(In reply to Eric Botcazou from comment #8)
> > I'm afraid I don't know anything about Ada and how its runtime works; it
> > looks like system.secondary_stack.ss_release is called automatically somehow
> > as part of entering make.Initialize, but I have no idea if that's supposed
> > to happen.  It seems questionable at first glance.
> 
> Unfortunately I don't seem to be able to connect to gcc112 in the
> CompileFarm:
> 
> eric@arcturus:~> ssh -l ebotcazou gcc112.osuosl.org
> ssh: Could not resolve hostname gcc112.osuosl.org: Name or service not known
> 
> Try to recompile the gnattools with reload instead of LRA, for example:
>   rm -rf gnattools gcc/ada/tools gcc/stamp-tools
>   make all-gnattools CFLAGS="-O2 -mno-lra"
> 
> Does this eliminate the problem?

No, unfortunately it doesn't; I see the same error.  I'll try the DSE experiment now.

(In reply to Eric Botcazou from comment #10)
> > So the double-word load before the call to SS_Release should be from a
> Mark_Id object obtained from a preceding call to SS_Mark.  It indeed looks
> like the double-word store to this Mark_Id object has been optimized away,
> possibly because of the strange back-and-forth adjustment to the FP.  Does
> passing CFLAGS="-O2 -fno-dse" for the gnattools make a difference?

Yes, it does!  The tools appear to be building normally with this option.  Without it the build of gnattools fails almost immediately.

I'll work on getting some dumps to see what is happening in DSE.

> Yes, it does!  The tools appear to be building normally with this option. 
> Without it the build of gnattools fails almost immediately.
> 
> I'll work on getting some dumps to see what is happening in DSE.

Thanks in advance, that would be great.  The weird dance with FP can certainly be a mightily confusing factor for the RTL DSE pass.

On my side, I have bootstrapped on good old 32-bit PowerPC/Linux and Ada is in nominal shape here (we're slowing migrating to 64-bit... big-endian for now).
Thanks to Marc, I'll try to reproduce the issue on gcc2-power8, but the system compiler doesn't come with Ada so this may take some time.

Confirmed that the frame pointer dance is where the issue is.  Prior to dse2:

(insn 3854 144 4133 2 (set (reg:DI 9 9 [1674])
        (const_int 1208 [0x4b8])) /home/wschmidt/gcc/gcc-mainline-base/gcc/ada/\
make.adb:6327 565 {*movdi_internal64}
     (expr_list:REG_EQUIV (const_int 1208 [0x4b8])
        (nil)))
(insn 4133 3854 4135 2 (set (reg/f:DI 31 31)
        (plus:DI (reg/f:DI 31 31)
            (reg:DI 9 9 [1674]))) /home/wschmidt/gcc/gcc-mainline-base/gcc/ada/\
make.adb:6327 75 {*adddi3}
     (nil))
(insn 4135 4133 4136 2 (set (mem/j/c:DI (reg/f:DI 31 31) [1137 FRAME.2381.M1944\
b+0 S8 A64])
        (reg:DI 3 3)) /home/wschmidt/gcc/gcc-mainline-base/gcc/ada/make.adb:632\
7 565 {*movdi_internal64}
     (nil))
(insn 4136 4135 4134 2 (set (mem/j/c:DI (plus:DI (reg/f:DI 31 31)
                (const_int 8 [0x8])) [1137 FRAME.2381.M1944b+8 S8 A64])
        (reg:DI 4 4 [+8 ])) /home/wschmidt/gcc/gcc-mainline-base/gcc/ada/make.a\
db:6327 565 {*movdi_internal64}
     (nil))
(insn 4134 4136 3868 2 (set (reg/f:DI 31 31)
        (minus:DI (reg/f:DI 31 31)
            (reg:DI 9 9 [1674]))) /home/wschmidt/gcc/gcc-mainline-base/gcc/ada/\
make.adb:6327 102 {*subfdi3}
     (nil))

You can see the "mark" being established with the stores from r3 and r4.

Following dse2:

(insn 3854 144 4133 2 (set (reg:DI 9 9 [1674])
        (const_int 1208 [0x4b8])) /home/wschmidt/gcc/gcc-mainline-base/gcc/ada/\
make.adb:6327 565 {*movdi_internal64}
     (expr_list:REG_EQUIV (const_int 1208 [0x4b8])
        (nil)))
(insn 4133 3854 4134 2 (set (reg/f:DI 31 31)
        (plus:DI (reg/f:DI 31 31)
            (reg:DI 9 9 [1674]))) /home/wschmidt/gcc/gcc-mainline-base/gcc/ada/\
make.adb:6327 75 {*adddi3}
     (nil))
(insn 4134 4133 3868 2 (set (reg/f:DI 31 31)
        (minus:DI (reg/f:DI 31 31)
            (reg:DI 9 9 [1674]))) /home/wschmidt/gcc/gcc-mainline-base/gcc/ada/\
make.adb:6327 102 {*subfdi3}
     (expr_list:REG_DEAD (reg:DI 9 9 [1674])
        (nil)))

DSE thought the two stores were dead and nuked them.

Is this kind of frame pointer adjustment a common occurrence with Ada code gen, or do you think this is related to POWER code gen somehow?  I've never seen this sort of thing before.

Created attachment 39520 [details]
Dumps before and after dse2

Here are the full dumps before and after dse2 in tarball form.

Looks like the back end must be inserting the frame pointer adjustments, as they aren't there at expand time.  From the 218.vregs dump:

(call_insn 141 140 3128 2 (parallel [
            (set (reg:TI 3 3)
                (call (mem:SI (symbol_ref:DI ("system__secondary_stack__ss_mark\
") [flags 0x41]  <function_decl 0x1000008a9200 system__secondary_stack__ss_mark\
>) [0 system__secondary_stack__ss_mark S4 A8])
                    (const_int 0 [0])))
            (clobber (reg:DI 65 lr))
        ]) /home/wschmidt/gcc/gcc-mainline-base/gcc/ada/make.adb:6327 661 {*cal\
l_value_nonlocal_aixdi}
     (expr_list:REG_CALL_DECL (symbol_ref:DI ("system__secondary_stack__ss_mark\
") [flags 0x41]  <function_decl 0x1000008a9200 system__secondary_stack__ss_mark\
>)
        (nil))
    (expr_list (use (reg:DI 2 2))
        (nil)))
(insn 3128 141 142 2 (set (reg:DI 1674)
        (const_int 1208 [0x4b8])) /home/wschmidt/gcc/gcc-mainline-base/gcc/ada/\
make.adb:6327 -1
     (nil))
(insn 142 3128 143 2 (set (mem/j/c:TI (plus:DI (reg/f:DI 113 sfp)
                (reg:DI 1674)) [1137 FRAME.2381.M1944b+0 S16 A64])
        (reg:TI 3 3)) /home/wschmidt/gcc/gcc-mainline-base/gcc/ada/make.adb:632\
7 975 {*vsx_movti_64bit}
     (nil))

I'll continue to dig.

> Is this kind of frame pointer adjustment a common occurrence with Ada code
> gen, or do you think this is related to POWER code gen somehow?  I've never
> seen this sort of thing before.

No, that's really weird, but the indices of the 4 instructions show that they have been generated as a group after the initial RTL expansion.  Can you find you which RTL pass generates them and attach the associated couple of dump files?

The frame pointer adjustments are introduced in 263r.split2.  I haven't yet run down the offending split, but the pattern being split is a *vsx_movti_64bit.  I know we've had changes in the back end fairly recently involving TImode moves, so this may be why the problem didn't show up before.

I think DSE relies on the stack being fairly well behaved; it probably shouldn't be making assumptions when the frame pointer is messed with like this during the function.  Which means doing this sort of thing is bad for optimization anyway, and should be avoided.  I'll look further.

I'm suspicious of rs6000_split_multireg_move in rs6000.c, which appears to be the code that gets called to split a TImode move involving a GPR pair.

In particular, this code:

              else
                {
                  rtx basereg = XEXP (XEXP (dst, 0), 0);
                  rtx offsetreg = XEXP (XEXP (dst, 0), 1);
                  gcc_assert (GET_CODE (XEXP (dst, 0)) == PLUS
                              && REG_P (basereg)
                              && REG_P (offsetreg)
                              && REGNO (basereg) != REGNO (offsetreg));
                  if (REGNO (basereg) == 0)
                    {
                      rtx tmp = offsetreg;
                      offsetreg = basereg;
                      basereg = tmp;
                    }
                  emit_insn (gen_add3_insn (basereg, basereg, offsetreg));
                  restore_basereg = gen_sub3_insn (basereg, basereg, offsetreg)\
;
                  dst = replace_equiv_address (dst, basereg);
                }

(Later on, restore_basereg is used to put the basereg back where it was before.  This looks exactly like the case we're seeing.)

I don't think we should allow going through this logic if basereg is the stack pointer or the frame pointer.  Mike, what do you think?

Yeah, it sounds like you don't want to adjust any of the stack related registers.

However, in looking at this $#!%, we probably need to rewrite it so it doesn't do clever tricks like this.  Which probably means a trip through the whole legitimize addressing support and secondary reload.

One thing that might help is removing the test for TARGET_QUAD_WORD around line 9448 of rs6000.c, and look for other places in rs6000.c that TImode and TARGET_QUAD_WORD are mentioned near each other.

On LE systems, TARGET_QUAD_WORD is false because the LQ/STQ load the words in the wrong order.

One consequence of this is it only allows TImode value addresses to be a single register, on the other hand, when you are moving TImode in GPRs, you can use D-FORM addressing to address the secondary parts.

In theory, PRE_MODIFY, PRE_INC, and PRE_DEC should never be set for TImode.

I think for the purposes of this bug we should be able to work around it by forcing the offset register to be modified instead of the base register.  Going to try testing this:

Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c  (revision 239871)
+++ gcc/config/rs6000/rs6000.c  (working copy)
@@ -24506,7 +24506,9 @@ rs6000_split_multireg_move (rtx dst, rtx src)
                              && REG_P (basereg)
                              && REG_P (offsetreg)
                              && REGNO (basereg) != REGNO (offsetreg));
-                 if (REGNO (basereg) == 0)
+                 if (REGNO (basereg) == 0
+                     || REGNO (basereg) == STACK_POINTER_REGNUM
+                     || REGNO (basereg) == HARD_FRAME_POINTER_REGNUM)
                    {
                      rtx tmp = offsetreg;
                      offsetreg = basereg;

Note, if the index register is R0 and the base register is SP, you might not be able to use the other register (well you can use it, but you likely will get a segmentation fault or just access the wrong memory).

I would at least put in a gcc_assert to make sure it isn't R0.

Bleah, that doesn't work because offsetreg needs to contain something that's a valid address in order to use replace_equiv_address.  So something more involved is needed.

This seems to work as a short-term solution (c,c++,ada bootstrap succeeds).  Need to do a full regstrap with all the languages.

Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c  (revision 239871)
+++ gcc/config/rs6000/rs6000.c  (working copy)
@@ -24506,15 +24506,31 @@ rs6000_split_multireg_move (rtx dst, rtx src)
                              && REG_P (basereg)
                              && REG_P (offsetreg)
                              && REGNO (basereg) != REGNO (offsetreg));
-                 if (REGNO (basereg) == 0)
+                 /* We mustn't modify the stack pointer or frame pointer
+                    as this will confuse dead store elimination.  */
+                 if ((REGNO (basereg) == STACK_POINTER_REGNUM
+                      || REGNO (basereg) == HARD_FRAME_POINTER_REGNUM)
+                     && REGNO (offsetreg) != 0)
                    {
-                     rtx tmp = offsetreg;
-                     offsetreg = basereg;
-                     basereg = tmp;
+                     emit_insn (gen_add3_insn (offsetreg, basereg,
+                                               offsetreg));
+                     restore_basereg = gen_sub3_insn (offsetreg, offsetreg,
+                                                      basereg);
+                     dst = replace_equiv_address (dst, offsetreg);
                    }
-                 emit_insn (gen_add3_insn (basereg, basereg, offsetreg));
-                 restore_basereg = gen_sub3_insn (basereg, basereg, offsetreg);
-                 dst = replace_equiv_address (dst, basereg);
+                 else
+                   {
+                     if (REGNO (basereg) == 0)
+                       {
+                         rtx tmp = offsetreg;
+                         offsetreg = basereg;
+                         basereg = tmp;
+                       }
+                     emit_insn (gen_add3_insn (basereg, basereg, offsetreg));
+                     restore_basereg = gen_sub3_insn (basereg, basereg,
+                                                      offsetreg);
+                     dst = replace_equiv_address (dst, basereg);
+                   }
                }
            }
          else if (GET_CODE (XEXP (dst, 0)) != LO_SUM)

Author: wschmidt
Date: Thu Sep  1 14:43:55 2016
New Revision: 239938

URL: https://gcc.gnu.org/viewcvs?rev=239938&root=gcc&view=rev
Log:
2016-09-01  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
	    Michael Meissner <meissner@linux.vnet.ibm.com>

	PR target/72827
	* config/rs6000/rs6000.c (rs6000_legitimize_address): Avoid
	reg+reg addressing for TImode.
	(rs6000_legitimate_address_p): Only allow register indirect
	addressing for TImode, even without TARGET_QUAD_MEMORY.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/rs6000/rs6000.c

The original patch direction was impossible to make work, as with a base register of r31 (frame pointer) and an offset register of r0 (not allowed as a base register), there is no legitimate way to update the address into one of the existing registers without spilling.

I've committed a temporary workaround that prevents reg+reg addressing for TImode, so that the situation can't arise.  When we have a little more time, our team will work on a solution with secondary reload, but this clears up the bootstrap issue for now.

.

Just recording that, as expected, this patch had neutral performance on SPECcpu2006.

Author: wschmidt
Date: Tue Dec  6 22:11:54 2016
New Revision: 243319

URL: https://gcc.gnu.org/viewcvs?rev=243319&root=gcc&view=rev
Log:
2016-12-06  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	Backport from mainline
	2016-09-01  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
	            Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/72827
	* config/rs6000/rs6000.c (rs6000_legitimize_address): Avoid
	reg+reg addressing for TImode.
	(rs6000_legitimate_address_p): Only allow register indirect
	addressing for TImode, even without TARGET_QUAD_MEMORY.


Modified:
    branches/gcc-6-branch/gcc/ChangeLog
    branches/gcc-6-branch/gcc/config/rs6000/rs6000.c