Fix pr78390: bootstrap on ia64 and s390x (with zEC12)

Fix pr78390: bootstrap on ia64 and s390x (with zEC12)

[Michael Matz]

Hi,

as the bug trail explains make_extraction is claiming but failing to 
handle extractions that would go outside the underlying object.  So, let's 
not construct such, as the patch does.

Dominik tested s390x bootstrap being recovered with this, Andreas ia64 
bootstrap, and I regstrapped this on x86-64-linux without regressions (all 
langs+ada).  Okay for trunk?


Ciao,
Michael.

	PR bootstrap/78390
	* combine.c (make_compound_operation_int): Don't extract
	from outside underlying object.

diff --git a/gcc/combine.c b/gcc/combine.c
index 0210685..1d8bddf 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -8108,9 +8108,16 @@ make_compound_operation (rtx x, enum rtx_code in_code)
 	    && GET_MODE_SIZE (mode) < GET_MODE_SIZE (GET_MODE (inner))
 	    && subreg_lowpart_p (x))
 	  {
+	    int len = mode_width;
 	    new_rtx = make_compound_operation (XEXP (inner, 0), next_code);
+	    /* Don't extract bits outside the underlying mode.  */
+	    if (CONST_INT_P (XEXP (inner, 1))
+		&& (INTVAL (XEXP (inner, 1)) + len
+		    > GET_MODE_PRECISION (GET_MODE (new_rtx))))
+	      len = GET_MODE_PRECISION (GET_MODE (new_rtx))
+		    - INTVAL (XEXP (inner, 1));
 	    new_rtx = make_extraction (mode, new_rtx, 0, XEXP (inner, 1),
-				       mode_width, 1, 0, in_code == COMPARE);
+				       len, 1, 0, in_code == COMPARE);
 	    break;
 	  }
 

Re: Fix pr78390: bootstrap on ia64 and s390x (with zEC12)

[Jakub Jelinek]

On Wed, Nov 23, 2016 at 02:26:49PM +0100, Michael Matz wrote:
> Hi,
> 
> as the bug trail explains make_extraction is claiming but failing to 
> handle extractions that would go outside the underlying object.  So, let's 
> not construct such, as the patch does.
> 
> Dominik tested s390x bootstrap being recovered with this, Andreas ia64 
> bootstrap, and I regstrapped this on x86-64-linux without regressions (all 
> langs+ada).  Okay for trunk?

Shouldn't new_rtx be set to NULL_RTX if that condition is false?  Otherwise
it will be whatever make_compound_operation returned.  What about the break?
Shouldn't that be done only if the condition is true too?

Anyway, I'm afraid I don't know this code enough, so deferring to Segher.

> 	PR bootstrap/78390
> 	* combine.c (make_compound_operation_int): Don't extract
> 	from outside underlying object.
> 
> diff --git a/gcc/combine.c b/gcc/combine.c
> index 0210685..1d8bddf 100644
> --- a/gcc/combine.c
> +++ b/gcc/combine.c
> @@ -8108,9 +8108,16 @@ make_compound_operation (rtx x, enum rtx_code in_code)
>  	    && GET_MODE_SIZE (mode) < GET_MODE_SIZE (GET_MODE (inner))
>  	    && subreg_lowpart_p (x))
>  	  {
> +	    int len = mode_width;
>  	    new_rtx = make_compound_operation (XEXP (inner, 0), next_code);
> +	    /* Don't extract bits outside the underlying mode.  */
> +	    if (CONST_INT_P (XEXP (inner, 1))
> +		&& (INTVAL (XEXP (inner, 1)) + len
> +		    > GET_MODE_PRECISION (GET_MODE (new_rtx))))
> +	      len = GET_MODE_PRECISION (GET_MODE (new_rtx))
> +		    - INTVAL (XEXP (inner, 1));
>  	    new_rtx = make_extraction (mode, new_rtx, 0, XEXP (inner, 1),
> -				       mode_width, 1, 0, in_code == COMPARE);
> +				       len, 1, 0, in_code == COMPARE);
>  	    break;
>  	  }
>  

	Jakub

Re: Fix pr78390: bootstrap on ia64 and s390x (with zEC12)

[Michael Matz]

Hi,

On Wed, 23 Nov 2016, Jakub Jelinek wrote:

> On Wed, Nov 23, 2016 at 02:26:49PM +0100, Michael Matz wrote:
> > Hi,
> > 
> > as the bug trail explains make_extraction is claiming but failing to 
> > handle extractions that would go outside the underlying object.  So, let's 
> > not construct such, as the patch does.
> > 
> > Dominik tested s390x bootstrap being recovered with this, Andreas ia64 
> > bootstrap, and I regstrapped this on x86-64-linux without regressions (all 
> > langs+ada).  Okay for trunk?
> 
> Shouldn't new_rtx be set to NULL_RTX if that condition is false?  Otherwise
> it will be whatever make_compound_operation returned.  What about the break?
> Shouldn't that be done only if the condition is true too?

Hmm?  The conditionial statement only guards the adjustment of len if that 
happens to be too large to be completely contained inside new_rtx.  If 
it's not outside len doesn't need adjustment.  Yes, new_rtx will always be 
whatever make_compound_operation returns (no matter the condition) and be 
the input to make_extraction as the object from which to extract (which 
itself will then either return NULL_RTX if the asked extraction is 
unfeasible or the extracted object).  The break is there to not enter the 
following code on a (subreg (lshiftrt )).  It could be conditionalized on 
new_rtx being != NULL (so that it only breaks if the extraction was 
successfull and otherwise falls through to the generic subreg code).  But 
that would be a different change not having to do with the bootstrap 
problem.  (It would make sense from my POV).


Ciao,
Michael.


> 
> Anyway, I'm afraid I don't know this code enough, so deferring to Segher.
> 
> > 	PR bootstrap/78390
> > 	* combine.c (make_compound_operation_int): Don't extract
> > 	from outside underlying object.
> > 
> > diff --git a/gcc/combine.c b/gcc/combine.c
> > index 0210685..1d8bddf 100644
> > --- a/gcc/combine.c
> > +++ b/gcc/combine.c
> > @@ -8108,9 +8108,16 @@ make_compound_operation (rtx x, enum rtx_code in_code)
> >  	    && GET_MODE_SIZE (mode) < GET_MODE_SIZE (GET_MODE (inner))
> >  	    && subreg_lowpart_p (x))
> >  	  {
> > +	    int len = mode_width;
> >  	    new_rtx = make_compound_operation (XEXP (inner, 0), next_code);
> > +	    /* Don't extract bits outside the underlying mode.  */
> > +	    if (CONST_INT_P (XEXP (inner, 1))
> > +		&& (INTVAL (XEXP (inner, 1)) + len
> > +		    > GET_MODE_PRECISION (GET_MODE (new_rtx))))
> > +	      len = GET_MODE_PRECISION (GET_MODE (new_rtx))
> > +		    - INTVAL (XEXP (inner, 1));
> >  	    new_rtx = make_extraction (mode, new_rtx, 0, XEXP (inner, 1),
> > -				       mode_width, 1, 0, in_code == COMPARE);
> > +				       len, 1, 0, in_code == COMPARE);
> >  	    break;
> >  	  }
> >  
> 
> 	Jakub
> 

Re: Fix pr78390: bootstrap on ia64 and s390x (with zEC12)

[Segher Boessenkool]

On Wed, Nov 23, 2016 at 02:45:23PM +0100, Michael Matz wrote:
> > Shouldn't new_rtx be set to NULL_RTX if that condition is false?  Otherwise
> > it will be whatever make_compound_operation returned.  What about the break?
> > Shouldn't that be done only if the condition is true too?
> 
> Hmm?  The conditionial statement only guards the adjustment of len if that 
> happens to be too large to be completely contained inside new_rtx.

new_rtx shouldn't be set at all here for non-constant shifts, because
those have the same problem as this new conditional tries to solve.

I'm committing a similar patch, see other thread.


Segher