Hi Eric, Thanks for the review.
> +/* Return true if X is a sign_extract or zero_extract from the least > + significant bit. */ > + > +static bool > +lsb_bitfield_op_p (rtx x) > +{ > + if (GET_RTX_CLASS (GET_CODE (x)) == RTX_BITFIELD_OPS) > + { > + enum machine_mode mode = GET_MODE(x); > + unsigned HOST_WIDE_INT len = INTVAL (XEXP (x, 1)); > + HOST_WIDE_INT pos = INTVAL (XEXP (x, 2)); > + > + return (pos == (BITS_BIG_ENDIAN ? GET_MODE_PRECISION (mode) - len : > 0)); > > It seems strange to use the destination mode to decide whether this is the LSB > of the source. Indeed, I think it has to be the mode of loc, but I just wonder if it is not always the same, as in the doc it is written that mode m is the same as the mode that would be used for loc if it were a register. > +/* Return true if X is a shifting operation. */ > + > +static bool > +shift_code_p (rtx x) > +{ > + return (GET_CODE (x) == ASHIFT > + || GET_CODE (x) == ASHIFTRT > + || GET_CODE (x) == LSHIFTRT > + || GET_CODE (x) == ROTATE > + || GET_CODE (x) == ROTATERT); > +} > > ROTATE and ROTATERT aren't really shifting operations though, so are they > really needed here? According to gcc internals, ROTATE and ROTATERT are similar to the shifting operations, but to be more accurate maybe we can rename shif_code_p in shift_and _rotate_code_p rotation are used in arm address calculation, and thus need to be handle in must_be_index_p and set_address_index Thanks, Yvan