> This patch fixes > FAIL: gcc.target/powerpc/ti_math1.c scan-assembler-times adde 1 > a failure caused by combine simplifying this i2src > > (plus:DI (plus:DI (reg:DI 165 [ val+8 ]) > (reg:DI 169 [+8 ])) > (reg:DI 76 ca)) > > to this > > (plus:DI (plus:DI (reg:DI 76 ca) > (reg:DI 165 [ val+8 ])) > (reg:DI 169 [+8 ])) > > which no longer matches rs6000.md adddi3_carry_in_internal. See > https://gcc.gnu.org/ml/gcc/2015-05/msg00206.html for related > discussion. Bootstrapped and regression tested powerpc64le-linux, > powerpc64-linux and x86_64-linux. OK to apply mainline? > > * rtlanal.c (commutative_operand_precedence): Correct comments. > * simplify-rtx.c (simplify_plus_minus_op_data_cmp): Delete forward > declaration. Return an int. Distinguish REG,REG return from > others. > (struct simplify_plus_minus_op_data): Make local to function. > (simplify_plus_minus): Rename canonicalized to not_canonical. > Don't set not_canonical if merely sorting registers. Avoid > packing ops if nothing changes. White space fixes.
OK in principle, but... > Some notes: Renaming canonicalized to not_canonical better reflects > its usage. At the time the var is set, the expression hasn't been > canonicalized. I'm quite skeptical, in particular given: + /* Just swapping registers doesn't count as canonicalization. */ + if (cmp != 1) + not_canonical = 1; and + /* If nothing changed, fail. */ + if (!not_canonical) + return NULL_RTX; Both are rather confusing now so the renaming isn't really a progress IMO. -- Eric Botcazou
