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
>
