Re: [PATCH, rs6000] Fix PR72827 (ada bootstrap failure)

[Segher Boessenkool <segher at kernel dot crashing dot org>]

Hi Bill,

On Tue, Aug 30, 2016 at 08:23:46PM -0500, Bill Schmidt wrote:
> The ada bootstrap failure reported in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=72827
> occurs because of a latent bug in the powerpc back end.  The immediate cause is dead store
> elimination removing two stores relative to the frame pointer that are not dead; however,
> DSE is tricked into doing this because we have temporarily adjusted the frame pointer prior
> to performing the loads.  DSE relies on the frame pointer remaining constant to be able to
> infer stack stores that are dead.

DSE should really detect this is happening and not do the wrong thing.
Maybe add an assert somewhere?  Much easier to debug, that way.

> Is this ok for trunk, and eventually for backport to the 5 and 6 branches?

Will it backport without changes?

> 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)

This should only check for HARD_FRAME_POINTER_REGNUM if there *is* a
frame pointer?

>  		    {
> -		      rtx tmp = offsetreg;
> -		      offsetreg = basereg;
> -		      basereg = tmp;

std::swap (basereg, offsetreg);

> +		      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)

If (say) base=r1 offset=r0 this will now adjust r1?  That cannot be good.


Segher