On Wed, Nov 23, 2016 at 04:24:37PM +0100, Michael Matz wrote: > > r242414, for PR77881, introduces some bugs (PR78390, PR78438, PR78477). > > It all has the same root cause: that patch makes combine convert every > > lowpart subreg of a logical shift right to a zero_extract. This cannot > > work at all if it is not a constant shift, > > Even with non-constant shifts it remains an extract and make_extraction > does support variable start positions (which is the shift amount), as does > the zero_extract pattern (depending on target of course).
Sure, but the extraction length is non-constant as well then! And not just not-constant, with a conditional inside it, even. > > if (GET_CODE (inner) == LSHIFTRT > > + && CONST_INT_P (XEXP (inner, 1)) > > && GET_MODE_SIZE (mode) < GET_MODE_SIZE (GET_MODE (inner)) > > && subreg_lowpart_p (x)) > > { > > new_rtx = make_compound_operation (XEXP (inner, 0), next_code); > > + int width = GET_MODE_PRECISION (GET_MODE (inner)) > > + - INTVAL (XEXP (inner, 1)); > > GET_MODE (new_rtx), because that's the object you're giving to > make_extraction, not inner (and not XEXP(inner, 0)). This is about the *original* rtx, the lshiftrt: how many non-zero bits does that leave. The bits zeroed out by the lshiftrt have to be zeroed out by the zero_extract as well, to keep the same semantics in the resulting rtl. > > + if (width > mode_width) > > + width = mode_width; > > new_rtx = make_extraction (mode, new_rtx, 0, XEXP (inner, 1), > > - mode_width, 1, 0, in_code == COMPARE); > > + width, 1, 0, in_code == COMPARE); > > break; > > } Segher