On 4/28/20 5:39 PM, Richard Sandiford wrote:
> If we use simplify_gen_binary then I don't think we need to validate
> each change individually. It should be enough to do something like:
>
> x0 = cse_process_notes (XEXP (x, 0), object, changed);
> x1 = cse_process_notes (XEXP (x, 1), object, changed);
> if (x0 == XEXP (x, 0) && x1 == XEXP (x, 1))
> return x;
> return simplify_gen_binary (code, GET_MODE (x), x0, x1);
Ok, I wasn't sure whether simplify_gen_binary verified its changes or not.
> Alternatively, in the hope of reducing redundant rtl, I guess we could
> add the switch statement after the format walk and do:
>
> switch (GET_RTX_CLASS (code))
> {
> case RTX_BIN_ARITH:
> case RTX_COMM_ARITH:
> if (rtx new_rtx = simplify_binary (code, GET_MODE (x),
> XEXP (x, 0), XEXP (x, 1)))
> return new_rtx;
> break;
> default:
> break;
> }
Doesn't moving this after the format walk, create the situation of
producing constant expressions without (const:P ...) and then fixing
them up after the fact like my original patch did? That's why I placed
the previous version before the format walk, but if you're ok with that
now, then that's fine with me. That said, the PLUS will be wrapped in
a (const:P ...) before it's substituted into the MEM, so maybe it was
just the MEM you were worried about?
However, do you mean the change to be the following, since I don't think
simplify_gen_binary ever returns NULL?
validate_change (object, &XEXP (x, i),
cse_process_notes (XEXP (x, i), object, changed), 0);
+ switch (GET_RTX_CLASS (code))
+ {
+ case RTX_BIN_ARITH:
+ case RTX_COMM_ARITH:
+ return simplify_gen_binary (code, GET_MODE (x), XEXP (x, 0), XEXP (x,
1));
+ default:
+ break;
+ }
+
return x;
}
Peter
