[RFC] Tighten memory type assumption in RTL combiner pass.

[Jeff Law]

On 01/14/15 04:27, Venkataramanan Kumar wrote:
> Hi all,
>
> When trying to debug GCC combiner pass with the test case in PR63949
> ref https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63949 I came across this code.
>
> This code in "make_compound_operation" assumes that all PLUS and MINUS
> RTX are "MEM" type for scalar int modes and tries to optimize based on
> that assumption.
>
> /* Select the code to be used in recursive calls.  Once we are inside an
>        address, we stay there.  If we have a comparison, set to COMPARE,
>        but once inside, go back to our default of SET.  */
>
>     next_code = (code == MEM ? MEM
>                  : ((code == PLUS || code == MINUS)
>                     && SCALAR_INT_MODE_P (mode)) ? MEM
>                  : ((code == COMPARE || COMPARISON_P (x))
>                     && XEXP (x, 1) == const0_rtx) ? COMPARE
>                  : in_code == COMPARE ? SET : in_code);
>
>   next_code is passed as in_code via recursive calls to
> "make_compound_operation". Based on that we are converting shift
> pattern to MULT pattern.
>
> case ASHIFT:
>        /* Convert shifts by constants into multiplications if inside
>           an address.  */
>        if (in_code == MEM && CONST_INT_P (XEXP (x, 1))
>            && INTVAL (XEXP (x, 1)) < HOST_BITS_PER_WIDE_INT
>            && INTVAL (XEXP (x, 1)) >= 0
>            && SCALAR_INT_MODE_P (mode))
>          {
>
> Now I tried to tighten it further by adding check to see in_code is
> also MEM type.
> Not sure if this right thing to do.  But this assumption about MEM
> seems to be very relaxed before.
>
> diff --git a/gcc/combine.c b/gcc/combine.c
> index 101cf35..1353f54 100644
> --- a/gcc/combine.c
> +++ b/gcc/combine.c
> @@ -7696,7 +7696,8 @@ make_compound_operation (rtx x, enum rtx_code in_code)
>
>     next_code = (code == MEM ? MEM
>                 : ((code == PLUS || code == MINUS)
> -                 && SCALAR_INT_MODE_P (mode)) ? MEM
> +                 && SCALAR_INT_MODE_P (mode)
> +                 && (in_code == MEM)) ? MEM
>                 : ((code == COMPARE || COMPARISON_P (x))
>                    && XEXP (x, 1) == const0_rtx) ? COMPARE
>                 : in_code == COMPARE ? SET : in_code);
>
>
> This passed bootstrap on x86_64 and  GCC tests are not regressing.
> On Aarch64 passed bootstrap tests, test case in PR passed, but few
> tests failed (failed to generate adds and subs), because there are
> patterns (extended adds and subs) based on multiplication only in
> Aarch64 backend.
>
> if this change is correct then I may need to add patterns in Aarch64
> based on shifts. Not sure about targets also.
>
> Requesting further comments/help about this.
>
> I am looking to get it fixed in stage 1.
So the first question I would ask here is what precisely are you trying 
to accomplish?  Is there some code where making this change is important 
or is it strictly a theoretical problem?  If the latter, can we make it 
concrete with a well crafted testcase?

jeff