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. Some notes: Renaming canonicalized to not_canonical better reflects its usage. At the time the var is set, the expression hasn't been canonicalized. diff -w shown to exclude the whitespace fixes. Index: gcc/rtlanal.c =================================================================== --- gcc/rtlanal.c (revision 223463) +++ gcc/rtlanal.c (working copy) @@ -3140,10 +3140,9 @@ regno_use_in (unsigned int regno, rtx x) } /* Return a value indicating whether OP, an operand of a commutative - operation, is preferred as the first or second operand. The higher - the value, the stronger the preference for being the first operand. - We use negative values to indicate a preference for the first operand - and positive values for the second operand. */ + operation, is preferred as the first or second operand. The more + positive the value, the stronger the preference for being the first + operand. */ int commutative_operand_precedence (rtx op) @@ -3150,7 +3149,7 @@ commutative_operand_precedence (rtx op) { enum rtx_code code = GET_CODE (op); - /* Constants always come the second operand. Prefer "nice" constants. */ + /* Constants always become the second operand. Prefer "nice" constants. */ if (code == CONST_INT) return -8; if (code == CONST_WIDE_INT) Index: gcc/simplify-rtx.c =================================================================== --- gcc/simplify-rtx.c (revision 223463) +++ gcc/simplify-rtx.c (working copy) @@ -71,7 +71,6 @@ along with GCC; see the file COPYING3. If not see static rtx neg_const_int (machine_mode, const_rtx); static bool plus_minus_operand_p (const_rtx); -static bool simplify_plus_minus_op_data_cmp (rtx, rtx); static rtx simplify_plus_minus (enum rtx_code, machine_mode, rtx, rtx); static rtx simplify_immed_subreg (machine_mode, rtx, machine_mode, unsigned int); @@ -4064,20 +4063,10 @@ simplify_const_binary_operation (enum rtx_code cod -/* Simplify a PLUS or MINUS, at least one of whose operands may be another - PLUS or MINUS. +/* Return a positive integer if X should sort after Y. The value + returned is 1 if and only if X and Y are both regs. */ - Rather than test for specific case, we do this by a brute-force method - and do all possible simplifications until no more changes occur. Then - we rebuild the operation. */ - -struct simplify_plus_minus_op_data -{ - rtx op; - short neg; -}; - -static bool +static int simplify_plus_minus_op_data_cmp (rtx x, rtx y) { int result; @@ -4085,23 +4074,36 @@ simplify_plus_minus_op_data_cmp (rtx x, rtx y) result = (commutative_operand_precedence (y) - commutative_operand_precedence (x)); if (result) - return result > 0; + return result + result; /* Group together equal REGs to do more simplification. */ if (REG_P (x) && REG_P (y)) return REGNO (x) > REGNO (y); - else - return false; + + return 0; } +/* Simplify and canonicalize a PLUS or MINUS, at least one of whose + operands may be another PLUS or MINUS. + + Rather than test for specific case, we do this by a brute-force method + and do all possible simplifications until no more changes occur. Then + we rebuild the operation. + + May return NULL_RTX when no changes were made. */ + static rtx simplify_plus_minus (enum rtx_code code, machine_mode mode, rtx op0, rtx op1) { - struct simplify_plus_minus_op_data ops[16]; + struct simplify_plus_minus_op_data + { + rtx op; + short neg; + } ops[16]; rtx result, tem; int n_ops = 2; - int changed, n_constants, canonicalized = 0; + int changed, n_constants, not_canonical = 0; int i, j; memset (ops, 0, sizeof ops); @@ -4139,7 +4141,18 @@ simplify_plus_minus (enum rtx_code code, machine_m ops[i].op = XEXP (this_op, 0); changed = 1; - canonicalized |= this_neg || i != n_ops - 2; + /* If this operand was negated then we will potentially + canonicalize the expression. Similarly if we don't + place the operands adjacent we're re-ordering the + expression and thus might be performing a + canonicalization. Ignore register re-ordering. + ??? It might be better to shuffle the ops array here, + but then (plus (plus (A, B), plus (C, D))) wouldn't + be seen as non-canonical. */ + if (this_neg + || (i != n_ops - 2 + && !(REG_P (ops[i].op) && REG_P (ops[n_ops - 1].op)))) + not_canonical = 1; break; case NEG: @@ -4146,7 +4159,7 @@ simplify_plus_minus (enum rtx_code code, machine_m ops[i].op = XEXP (this_op, 0); ops[i].neg = ! this_neg; changed = 1; - canonicalized = 1; + not_canonical = 1; break; case CONST: @@ -4160,7 +4173,7 @@ simplify_plus_minus (enum rtx_code code, machine_m ops[n_ops].neg = this_neg; n_ops++; changed = 1; - canonicalized = 1; + not_canonical = 1; } break; @@ -4173,7 +4186,7 @@ simplify_plus_minus (enum rtx_code code, machine_m ops[i].op = XEXP (this_op, 0); ops[i].neg = !this_neg; changed = 1; - canonicalized = 1; + not_canonical = 1; } break; @@ -4184,7 +4197,7 @@ simplify_plus_minus (enum rtx_code code, machine_m ops[i].op = neg_const_int (mode, this_op); ops[i].neg = 0; changed = 1; - canonicalized = 1; + not_canonical = 1; } break; @@ -4196,7 +4209,7 @@ simplify_plus_minus (enum rtx_code code, machine_m while (changed); if (n_constants > 1) - canonicalized = 1; + not_canonical = 1; gcc_assert (n_ops >= 2); @@ -4228,21 +4241,27 @@ simplify_plus_minus (enum rtx_code code, machine_m } /* Now simplify each pair of operands until nothing changes. */ - do + while (1) { /* Insertion sort is good enough for a small array. */ for (i = 1; i < n_ops; i++) { struct simplify_plus_minus_op_data save; + int cmp; + j = i - 1; - if (!simplify_plus_minus_op_data_cmp (ops[j].op, ops[i].op)) + cmp = simplify_plus_minus_op_data_cmp (ops[j].op, ops[i].op); + if (cmp <= 0) continue; + /* Just swapping registers doesn't count as canonicalization. */ + if (cmp != 1) + not_canonical = 1; - canonicalized = 1; save = ops[i]; do ops[j + 1] = ops[j]; - while (j-- && simplify_plus_minus_op_data_cmp (ops[j].op, save.op)); + while (j-- + && simplify_plus_minus_op_data_cmp (ops[j].op, save.op) > 0); ops[j + 1] = save; } @@ -4306,14 +4325,13 @@ simplify_plus_minus (enum rtx_code code, machine_m ops[i].neg = lneg; ops[j].op = NULL_RTX; changed = 1; - canonicalized = 1; + not_canonical = 1; } } } - /* If nothing changed, fail. */ - if (!canonicalized) - return NULL_RTX; + if (!changed) + break; /* Pack all the operands to the lower-numbered entries. */ for (i = 0, j = 0; j < n_ops; j++) @@ -4324,8 +4342,11 @@ simplify_plus_minus (enum rtx_code code, machine_m } n_ops = i; } - while (changed); + /* If nothing changed, fail. */ + if (!not_canonical) + return NULL_RTX; + /* Create (minus -C X) instead of (neg (const (plus X C))). */ if (n_ops == 2 && CONST_INT_P (ops[1].op) -- Alan Modra Australia Development Lab, IBM