* Extend widening_mul pass to handle fixed-point types @ 2010-07-18 12:03 Richard Sandiford 2010-07-18 20:47 ` Bernd Schmidt 0 siblings, 1 reply; 13+ messages in thread From: Richard Sandiford @ 2010-07-18 12:03 UTC (permalink / raw) To: gcc-patches This patch makes the widening_mul pass handle fixed-point types, thus fixing gcc.target/dpaq_sa_l_w.c. WIDEN_MULT_PLUS_EXPR and WIDEN_MULT_MINUS_EXPR already handle the types correctly, it's just that we never generate them. The current multiply-accumulate code (quite reasonably) converts the multiplication to a WIDEN_MULT first, then uses the operands of that WIDEN_MULT in the WIDEN_MULT_PLUS_EXPR and WIDEN_MULT_MINUS_EXPR. The main gotcha with fixed-point types is that WIDEN_MULT_EXPR _doesn't_ support fixed-point types. Indeed MIPS -- the only fixed-point target AFAIK -- doesn't have a pure fixed-point widening multiplication instruction, just a multiply-accumulate one[*]. So there isn't any use for a fixed-point WIDEN_MULT_EXPR right now. Even if I tried to implement it, it would just be dead code, and there's no easy way of telling whether I got right. This patch therefore separates out the processes of getting the unwidened operands and of generating the WIDEN_MULT, so that we can still use the operands in cases where converting to a WIDEN_MULT isn't possible or worthwhile. I realise this is a bit of a strange case, but it does mean that the generation of WIDEN_MULT_{PLUS,MINUS}_EXPR depends only on whether the associated multiply-accumulate optab exists, not on whether other optabs (like the pure multiplication ones) exist too. Note that expand_debug_expr doesn't handle fixed-point types at the moment. That goes for these codes and for much simpler ones. I think adding that is a separate change. Tested on mipsisa64-elfoabi, where it fixes the testsuite failure. Also bootstrapped & regression-tested on x86_64-linux-gnu. OK to instal? Richard [*] It may well be that MIPS should use multiply-accumulate patterns to implement plain multiplication, but I don't have appropriate hardware to test that performance-wise. gcc/ * tree-ssa-math-opts.c (convert_mult_to_widen): Add an RHS_OUT parameter. Handle fixed-point types as well as integer ones, but continue to only generate WIDEN_MULT_EXPR for the latter. Use RHS_OUT to return the unwidened operands to the caller. (convert_plusminus_to_widen): Handle fixed-point types as well as integer ones. Update the call to convert_mult_to_widen. (execute_optimize_widening_mul): Update the call to convert_mult_to_widen. Index: gcc/tree-ssa-math-opts.c =================================================================== --- gcc/tree-ssa-math-opts.c 2010-07-18 08:36:48.000000000 +0100 +++ gcc/tree-ssa-math-opts.c 2010-07-18 12:35:42.000000000 +0100 @@ -1260,22 +1260,30 @@ struct gimple_opt_pass pass_optimize_bsw } }; -/* Process a single gimple statement STMT, which has a MULT_EXPR as - its rhs, and try to convert it into a WIDEN_MULT_EXPR. The return - value is true iff we converted the statement. */ +/* Process a single gimple statement STMT, which has a MULT_EXPR as its + rhs, and see if it is a widening multiplication. If so: + + - store the two unextended operands in RHS_OUT[0] and RHS_OUT[1], + if RHS_OUT is nonnull; and + - try to convert the statement into a WIDEN_MULT_EXPR. + + Return true if the statement could be converted or if operands + were stored in RHS_OUT. */ static bool -convert_mult_to_widen (gimple stmt) +convert_mult_to_widen (gimple stmt, tree *rhs_out) { gimple rhs1_stmt = NULL, rhs2_stmt = NULL; tree type1 = NULL, type2 = NULL; - tree rhs1, rhs2, rhs1_convop = NULL, rhs2_convop = NULL; + tree rhs1, rhs2, unwidened_rhs1 = NULL, unwidened_rhs2 = NULL; enum tree_code rhs1_code, rhs2_code; tree type; + bool use_widen_mult_p; type = TREE_TYPE (gimple_assign_lhs (stmt)); - if (TREE_CODE (type) != INTEGER_TYPE) + if (TREE_CODE (type) != INTEGER_TYPE + && TREE_CODE (type) != FIXED_POINT_TYPE) return false; rhs1 = gimple_assign_rhs1 (stmt); @@ -1287,14 +1295,17 @@ convert_mult_to_widen (gimple stmt) if (!is_gimple_assign (rhs1_stmt)) return false; rhs1_code = gimple_assign_rhs_code (rhs1_stmt); - if (!CONVERT_EXPR_CODE_P (rhs1_code)) + if (TREE_CODE (type) == INTEGER_TYPE + ? !CONVERT_EXPR_CODE_P (rhs1_code) + : rhs1_code != FIXED_CONVERT_EXPR) return false; - rhs1_convop = gimple_assign_rhs1 (rhs1_stmt); - type1 = TREE_TYPE (rhs1_convop); + unwidened_rhs1 = gimple_assign_rhs1 (rhs1_stmt); + type1 = TREE_TYPE (unwidened_rhs1); if (TYPE_PRECISION (type1) * 2 != TYPE_PRECISION (type)) return false; } - else if (TREE_CODE (rhs1) != INTEGER_CST) + else if (TREE_CODE (rhs1) != INTEGER_CST + && TREE_CODE (rhs1) != FIXED_CST) return false; if (TREE_CODE (rhs2) == SSA_NAME) @@ -1303,52 +1314,73 @@ convert_mult_to_widen (gimple stmt) if (!is_gimple_assign (rhs2_stmt)) return false; rhs2_code = gimple_assign_rhs_code (rhs2_stmt); - if (!CONVERT_EXPR_CODE_P (rhs2_code)) + if (TREE_CODE (type) == INTEGER_TYPE + ? !CONVERT_EXPR_CODE_P (rhs2_code) + : rhs2_code != FIXED_CONVERT_EXPR) return false; - rhs2_convop = gimple_assign_rhs1 (rhs2_stmt); - type2 = TREE_TYPE (rhs2_convop); + unwidened_rhs2 = gimple_assign_rhs1 (rhs2_stmt); + type2 = TREE_TYPE (unwidened_rhs2); if (TYPE_PRECISION (type2) * 2 != TYPE_PRECISION (type)) return false; } - else if (TREE_CODE (rhs2) != INTEGER_CST) + else if (TREE_CODE (rhs2) != INTEGER_CST + && TREE_CODE (rhs2) != FIXED_CST) return false; if (rhs1_stmt == NULL && rhs2_stmt == NULL) return false; + /* At present, WIDEN_MULT_EXPR only supports integer types, + not fixed-point ones. Processing fixed-point types is only + useful if the caller wants the unextended operands. */ + if (TREE_CODE (type) == FIXED_POINT_TYPE) + use_widen_mult_p = false; /* Verify that the machine can perform a widening multiply in this mode/signedness combination, otherwise this transformation is likely to pessimize code. */ - if ((rhs1_stmt == NULL || TYPE_UNSIGNED (type1)) - && (rhs2_stmt == NULL || TYPE_UNSIGNED (type2)) - && (optab_handler (umul_widen_optab, TYPE_MODE (type)) - == CODE_FOR_nothing)) - return false; + else if ((rhs1_stmt == NULL || TYPE_UNSIGNED (type1)) + && (rhs2_stmt == NULL || TYPE_UNSIGNED (type2)) + && (optab_handler (umul_widen_optab, TYPE_MODE (type)) + == CODE_FOR_nothing)) + use_widen_mult_p = false; else if ((rhs1_stmt == NULL || !TYPE_UNSIGNED (type1)) && (rhs2_stmt == NULL || !TYPE_UNSIGNED (type2)) && (optab_handler (smul_widen_optab, TYPE_MODE (type)) == CODE_FOR_nothing)) - return false; + use_widen_mult_p = false; else if (rhs1_stmt != NULL && rhs2_stmt != NULL && (TYPE_UNSIGNED (type1) != TYPE_UNSIGNED (type2)) && (optab_handler (usmul_widen_optab, TYPE_MODE (type)) == CODE_FOR_nothing)) + use_widen_mult_p = false; + else + use_widen_mult_p = true; + + if (!use_widen_mult_p && rhs_out == NULL) return false; if ((rhs1_stmt == NULL && !int_fits_type_p (rhs1, type2)) || (rhs2_stmt == NULL && !int_fits_type_p (rhs2, type1))) return false; - if (rhs1_stmt == NULL) - gimple_assign_set_rhs1 (stmt, fold_convert (type2, rhs1)); - else - gimple_assign_set_rhs1 (stmt, rhs1_convop); - if (rhs2_stmt == NULL) - gimple_assign_set_rhs2 (stmt, fold_convert (type1, rhs2)); - else - gimple_assign_set_rhs2 (stmt, rhs2_convop); - gimple_assign_set_rhs_code (stmt, WIDEN_MULT_EXPR); - update_stmt (stmt); + if (unwidened_rhs1 == NULL) + unwidened_rhs1 = fold_convert (type2, rhs1); + if (unwidened_rhs2 == NULL) + unwidened_rhs2 = fold_convert (type1, rhs2); + + if (rhs_out) + { + rhs_out[0] = unwidened_rhs1; + rhs_out[1] = unwidened_rhs2; + } + + if (use_widen_mult_p) + { + gimple_assign_set_rhs1 (stmt, unwidened_rhs1); + gimple_assign_set_rhs2 (stmt, unwidened_rhs2); + gimple_assign_set_rhs_code (stmt, WIDEN_MULT_EXPR); + update_stmt (stmt); + } return true; } @@ -1364,14 +1396,15 @@ convert_plusminus_to_widen (gimple_stmt_ { gimple rhs1_stmt = NULL, rhs2_stmt = NULL; tree type; - tree lhs, rhs1, rhs2, mult_rhs1, mult_rhs2, add_rhs; + tree lhs, rhs1, rhs2, mult_rhs[2], add_rhs; enum tree_code rhs1_code = ERROR_MARK, rhs2_code = ERROR_MARK; optab this_optab; enum tree_code wmult_code; lhs = gimple_assign_lhs (stmt); type = TREE_TYPE (lhs); - if (TREE_CODE (type) != INTEGER_TYPE) + if (TREE_CODE (type) != INTEGER_TYPE + && TREE_CODE (type) != FIXED_POINT_TYPE) return false; if (code == MINUS_EXPR) @@ -1407,29 +1440,28 @@ convert_plusminus_to_widen (gimple_stmt_ else return false; - if (rhs1_code == MULT_EXPR) + if (code == PLUS_EXPR && rhs1_code == MULT_EXPR) { - if (!convert_mult_to_widen (rhs1_stmt)) + if (!convert_mult_to_widen (rhs1_stmt, mult_rhs)) return false; - rhs1_code = gimple_assign_rhs_code (rhs1_stmt); + add_rhs = rhs2; } - if (rhs2_code == MULT_EXPR) + else if (rhs2_code == MULT_EXPR) { - if (!convert_mult_to_widen (rhs2_stmt)) + if (!convert_mult_to_widen (rhs2_stmt, mult_rhs)) return false; - rhs2_code = gimple_assign_rhs_code (rhs2_stmt); + add_rhs = rhs1; } - - if (code == PLUS_EXPR && rhs1_code == WIDEN_MULT_EXPR) + else if (code == PLUS_EXPR && rhs1_code == WIDEN_MULT_EXPR) { - mult_rhs1 = gimple_assign_rhs1 (rhs1_stmt); - mult_rhs2 = gimple_assign_rhs2 (rhs1_stmt); + mult_rhs[0] = gimple_assign_rhs1 (rhs1_stmt); + mult_rhs[1] = gimple_assign_rhs2 (rhs1_stmt); add_rhs = rhs2; } else if (rhs2_code == WIDEN_MULT_EXPR) { - mult_rhs1 = gimple_assign_rhs1 (rhs2_stmt); - mult_rhs2 = gimple_assign_rhs2 (rhs2_stmt); + mult_rhs[0] = gimple_assign_rhs1 (rhs2_stmt); + mult_rhs[1] = gimple_assign_rhs2 (rhs2_stmt); add_rhs = rhs1; } else @@ -1437,7 +1469,7 @@ convert_plusminus_to_widen (gimple_stmt_ /* ??? May need some type verification here? */ - gimple_assign_set_rhs_with_ops_1 (gsi, wmult_code, mult_rhs1, mult_rhs2, + gimple_assign_set_rhs_with_ops_1 (gsi, wmult_code, mult_rhs[0], mult_rhs[1], add_rhs); update_stmt (gsi_stmt (*gsi)); return true; @@ -1467,7 +1499,7 @@ execute_optimize_widening_mul (void) code = gimple_assign_rhs_code (stmt); if (code == MULT_EXPR) - changed |= convert_mult_to_widen (stmt); + changed |= convert_mult_to_widen (stmt, NULL); else if (code == PLUS_EXPR || code == MINUS_EXPR) changed |= convert_plusminus_to_widen (&gsi, stmt, code); } ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Extend widening_mul pass to handle fixed-point types 2010-07-18 12:03 Extend widening_mul pass to handle fixed-point types Richard Sandiford @ 2010-07-18 20:47 ` Bernd Schmidt 2010-07-19 18:35 ` Richard Sandiford 0 siblings, 1 reply; 13+ messages in thread From: Bernd Schmidt @ 2010-07-18 20:47 UTC (permalink / raw) To: gcc-patches, rdsandiford On 07/18/2010 02:03 PM, Richard Sandiford wrote: > + /* At present, WIDEN_MULT_EXPR only supports integer types, > + not fixed-point ones. Processing fixed-point types is only > + useful if the caller wants the unextended operands. */ > + if (TREE_CODE (type) == FIXED_POINT_TYPE) > + use_widen_mult_p = false; I don't like this bit. I'd break up this function into one that just extracts the unwidened operands, and another one that generates the widening multiply. The former can then be used for also generating widening-macc. Whether to generate anything for fixed-point should just depend on the availability of the optabs. Ok with that change. Bernd ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Extend widening_mul pass to handle fixed-point types 2010-07-18 20:47 ` Bernd Schmidt @ 2010-07-19 18:35 ` Richard Sandiford 2010-07-19 21:53 ` Bernd Schmidt 0 siblings, 1 reply; 13+ messages in thread From: Richard Sandiford @ 2010-07-19 18:35 UTC (permalink / raw) To: Bernd Schmidt; +Cc: gcc-patches Bernd Schmidt <bernds@codesourcery.com> writes: > On 07/18/2010 02:03 PM, Richard Sandiford wrote: > >> + /* At present, WIDEN_MULT_EXPR only supports integer types, >> + not fixed-point ones. Processing fixed-point types is only >> + useful if the caller wants the unextended operands. */ >> + if (TREE_CODE (type) == FIXED_POINT_TYPE) >> + use_widen_mult_p = false; > > I don't like this bit. I'd break up this function into one that just > extracts the unwidened operands, and another one that generates the > widening multiply. The former can then be used for also generating > widening-macc. Whether to generate anything for fixed-point should just > depend on the availability of the optabs. I'm not sure I follow the last sentence. Whether we generate widening macc for fixed-point types _does_ just depend on the optabs. The code you quote above is there to handle the fact that WIDEN_MULT_EXPR (as opposed to WIDEN_MULT_PLUS_EXPR and WIDEN_MULT_MINUS_EXPR) don't handle fixed-point types at present. I'd rather not add that support because nothing needs it, and it'd therefore be hard to test. (See covering message for details.) I did wonder about splitting the stuff out into a separate function. The advantage of not doing is that it avoids duplicate work in cases where we visit the macc first, and where the WIDEN_MULT_EXPR _can_ be used. I thought that would be quite a common case. Here's a version that splits it out and drops the two-at-once thing. Only lightly tested, but does it look better? Richard gcc/ * tree-ssa-math-opts.c (is_widening_mult_rhs_p): New function. (is_widening_mult_p): Likewise. (convert_to_widen): Use them. (convert_plusminus_to_widen): Likewise. Handle fixed-point types as well as integer ones. Index: gcc/tree-ssa-math-opts.c =================================================================== *** gcc/tree-ssa-math-opts.c 2010-07-18 13:16:54.000000000 +0100 --- gcc/tree-ssa-math-opts.c 2010-07-19 19:28:27.000000000 +0100 *************** struct gimple_opt_pass pass_optimize_bsw *** 1260,1352 **** } }; ! /* Process a single gimple statement STMT, which has a MULT_EXPR as ! its rhs, and try to convert it into a WIDEN_MULT_EXPR. The return ! value is true iff we converted the statement. */ ! static bool ! convert_mult_to_widen (gimple stmt) ! { ! gimple rhs1_stmt = NULL, rhs2_stmt = NULL; ! tree type1 = NULL, type2 = NULL; ! tree rhs1, rhs2, rhs1_convop = NULL, rhs2_convop = NULL; ! enum tree_code rhs1_code, rhs2_code; ! tree type; ! type = TREE_TYPE (gimple_assign_lhs (stmt)); ! ! if (TREE_CODE (type) != INTEGER_TYPE) ! return false; ! rhs1 = gimple_assign_rhs1 (stmt); ! rhs2 = gimple_assign_rhs2 (stmt); ! if (TREE_CODE (rhs1) == SSA_NAME) { ! rhs1_stmt = SSA_NAME_DEF_STMT (rhs1); ! if (!is_gimple_assign (rhs1_stmt)) return false; ! rhs1_code = gimple_assign_rhs_code (rhs1_stmt); ! if (!CONVERT_EXPR_CODE_P (rhs1_code)) return false; ! rhs1_convop = gimple_assign_rhs1 (rhs1_stmt); ! type1 = TREE_TYPE (rhs1_convop); ! if (TYPE_PRECISION (type1) * 2 != TYPE_PRECISION (type)) return false; } - else if (TREE_CODE (rhs1) != INTEGER_CST) - return false; ! if (TREE_CODE (rhs2) == SSA_NAME) { ! rhs2_stmt = SSA_NAME_DEF_STMT (rhs2); ! if (!is_gimple_assign (rhs2_stmt)) ! return false; ! rhs2_code = gimple_assign_rhs_code (rhs2_stmt); ! if (!CONVERT_EXPR_CODE_P (rhs2_code)) ! return false; ! rhs2_convop = gimple_assign_rhs1 (rhs2_stmt); ! type2 = TREE_TYPE (rhs2_convop); ! if (TYPE_PRECISION (type2) * 2 != TYPE_PRECISION (type)) ! return false; } ! else if (TREE_CODE (rhs2) != INTEGER_CST) return false; ! if (rhs1_stmt == NULL && rhs2_stmt == NULL) return false; ! /* Verify that the machine can perform a widening multiply in this ! mode/signedness combination, otherwise this transformation is ! likely to pessimize code. */ ! if ((rhs1_stmt == NULL || TYPE_UNSIGNED (type1)) ! && (rhs2_stmt == NULL || TYPE_UNSIGNED (type2)) ! && (optab_handler (umul_widen_optab, TYPE_MODE (type)) ! == CODE_FOR_nothing)) return false; ! else if ((rhs1_stmt == NULL || !TYPE_UNSIGNED (type1)) ! && (rhs2_stmt == NULL || !TYPE_UNSIGNED (type2)) ! && (optab_handler (smul_widen_optab, TYPE_MODE (type)) ! == CODE_FOR_nothing)) return false; ! else if (rhs1_stmt != NULL && rhs2_stmt != NULL ! && (TYPE_UNSIGNED (type1) != TYPE_UNSIGNED (type2)) ! && (optab_handler (usmul_widen_optab, TYPE_MODE (type)) ! == CODE_FOR_nothing)) return false; ! if ((rhs1_stmt == NULL && !int_fits_type_p (rhs1, type2)) ! || (rhs2_stmt == NULL && !int_fits_type_p (rhs2, type1))) return false; ! if (rhs1_stmt == NULL) ! gimple_assign_set_rhs1 (stmt, fold_convert (type2, rhs1)); ! else ! gimple_assign_set_rhs1 (stmt, rhs1_convop); ! if (rhs2_stmt == NULL) ! gimple_assign_set_rhs2 (stmt, fold_convert (type1, rhs2)); else ! gimple_assign_set_rhs2 (stmt, rhs2_convop); gimple_assign_set_rhs_code (stmt, WIDEN_MULT_EXPR); update_stmt (stmt); return true; --- 1260,1379 ---- } }; ! /* Return true if RHS is a suitable operand for a widening multiplication. ! There are two cases: ! - RHS makes some value twice as wide. Store that value in *NEW_RHS_OUT ! if so, and store its type in *TYPE_OUT. ! - RHS is an integer constant. Store that value in *NEW_RHS_OUT if so, ! but leave *TYPE_OUT untouched. */ ! static bool ! is_widening_mult_rhs_p (tree rhs, tree *type_out, tree *new_rhs_out) ! { ! gimple stmt; ! tree type, type1, rhs1; ! enum tree_code rhs_code; ! if (TREE_CODE (rhs) == SSA_NAME) { ! type = TREE_TYPE (rhs); ! stmt = SSA_NAME_DEF_STMT (rhs); ! if (!is_gimple_assign (stmt)) return false; ! ! rhs_code = gimple_assign_rhs_code (stmt); ! if (TREE_CODE (type) == INTEGER_TYPE ! ? !CONVERT_EXPR_CODE_P (rhs_code) ! : rhs_code != FIXED_CONVERT_EXPR) return false; ! ! rhs1 = gimple_assign_rhs1 (stmt); ! type1 = TREE_TYPE (rhs1); ! if (TREE_CODE (type1) != TREE_CODE (type) ! || TYPE_PRECISION (type1) * 2 != TYPE_PRECISION (type)) return false; + + *new_rhs_out = rhs1; + *type_out = type1; + return true; } ! if (TREE_CODE (rhs) == INTEGER_CST) { ! *new_rhs_out = rhs; ! *type_out = NULL; ! return true; } ! ! return false; ! } ! ! /* Return true if STMT performs a widening multiplication. If so, ! store the unwidened types of the operands in *TYPE1_OUT and *TYPE2_OUT ! respectively. Also fill *RHS1_OUT and *RHS2_OUT such that converting ! those operands to types *TYPE1_OUT and *TYPE2_OUT would give the ! operands of the multiplication. */ ! ! static bool ! is_widening_mult_p (gimple stmt, ! tree *type1_out, tree *rhs1_out, ! tree *type2_out, tree *rhs2_out) ! { ! tree type; ! ! type = TREE_TYPE (gimple_assign_lhs (stmt)); ! if (TREE_CODE (type) != INTEGER_TYPE ! && TREE_CODE (type) != FIXED_POINT_TYPE) return false; ! if (!is_widening_mult_rhs_p (gimple_assign_rhs1 (stmt), type1_out, rhs1_out)) return false; ! if (!is_widening_mult_rhs_p (gimple_assign_rhs2 (stmt), type2_out, rhs2_out)) return false; ! ! if (*type1_out == NULL ! && (*type2_out == NULL || !int_fits_type_p (*rhs1_out, *type2_out))) return false; ! ! if (*type2_out == NULL && !int_fits_type_p (*rhs2_out, *type1_out)) ! return false; ! ! return true; ! } ! ! /* Process a single gimple statement STMT, which has a MULT_EXPR as ! its rhs, and try to convert it into a WIDEN_MULT_EXPR. The return ! value is true iff we converted the statement. */ ! ! static bool ! convert_mult_to_widen (gimple stmt) ! { ! tree lhs, rhs1, rhs2, type, type1, type2; ! enum insn_code handler; ! ! lhs = gimple_assign_lhs (stmt); ! type = TREE_TYPE (lhs); ! if (TREE_CODE (type) != INTEGER_TYPE) return false; ! if (!is_widening_mult_p (stmt, &type1, &rhs1, &type2, &rhs2)) return false; ! if (TYPE_UNSIGNED (type1) && TYPE_UNSIGNED (type2)) ! handler = optab_handler (umul_widen_optab, TYPE_MODE (type)); ! else if (!TYPE_UNSIGNED (type1) && !TYPE_UNSIGNED (type2)) ! handler = optab_handler (smul_widen_optab, TYPE_MODE (type)); else ! handler = optab_handler (usmul_widen_optab, TYPE_MODE (type)); ! ! if (handler == CODE_FOR_nothing) ! return false; ! ! gimple_assign_set_rhs1 (stmt, fold_convert (type1, rhs1)); ! gimple_assign_set_rhs2 (stmt, fold_convert (type2, rhs2)); gimple_assign_set_rhs_code (stmt, WIDEN_MULT_EXPR); update_stmt (stmt); return true; *************** convert_plusminus_to_widen (gimple_stmt_ *** 1363,1369 **** enum tree_code code) { gimple rhs1_stmt = NULL, rhs2_stmt = NULL; ! tree type; tree lhs, rhs1, rhs2, mult_rhs1, mult_rhs2, add_rhs; enum tree_code rhs1_code = ERROR_MARK, rhs2_code = ERROR_MARK; optab this_optab; --- 1390,1396 ---- enum tree_code code) { gimple rhs1_stmt = NULL, rhs2_stmt = NULL; ! tree type, type1, type2; tree lhs, rhs1, rhs2, mult_rhs1, mult_rhs2, add_rhs; enum tree_code rhs1_code = ERROR_MARK, rhs2_code = ERROR_MARK; optab this_optab; *************** convert_plusminus_to_widen (gimple_stmt_ *** 1371,1377 **** lhs = gimple_assign_lhs (stmt); type = TREE_TYPE (lhs); ! if (TREE_CODE (type) != INTEGER_TYPE) return false; if (code == MINUS_EXPR) --- 1398,1405 ---- lhs = gimple_assign_lhs (stmt); type = TREE_TYPE (lhs); ! if (TREE_CODE (type) != INTEGER_TYPE ! && TREE_CODE (type) != FIXED_POINT_TYPE) return false; if (code == MINUS_EXPR) *************** convert_plusminus_to_widen (gimple_stmt_ *** 1407,1426 **** else return false; ! if (rhs1_code == MULT_EXPR) { ! if (!convert_mult_to_widen (rhs1_stmt)) return false; ! rhs1_code = gimple_assign_rhs_code (rhs1_stmt); } ! if (rhs2_code == MULT_EXPR) { ! if (!convert_mult_to_widen (rhs2_stmt)) return false; ! rhs2_code = gimple_assign_rhs_code (rhs2_stmt); } ! ! if (code == PLUS_EXPR && rhs1_code == WIDEN_MULT_EXPR) { mult_rhs1 = gimple_assign_rhs1 (rhs1_stmt); mult_rhs2 = gimple_assign_rhs2 (rhs1_stmt); --- 1435,1459 ---- else return false; ! if (code == PLUS_EXPR && rhs1_code == MULT_EXPR) { ! if (!is_widening_mult_p (rhs1_stmt, &type1, &mult_rhs1, ! &type2, &mult_rhs2)) return false; ! mult_rhs1 = fold_convert (type1, mult_rhs1); ! mult_rhs2 = fold_convert (type2, mult_rhs2); ! add_rhs = rhs2; } ! else if (rhs2_code == MULT_EXPR) { ! if (!is_widening_mult_p (rhs1_stmt, &type1, &mult_rhs1, ! &type2, &mult_rhs2)) return false; ! mult_rhs1 = fold_convert (type1, mult_rhs1); ! mult_rhs2 = fold_convert (type2, mult_rhs2); ! add_rhs = rhs1; } ! else if (code == PLUS_EXPR && rhs1_code == WIDEN_MULT_EXPR) { mult_rhs1 = gimple_assign_rhs1 (rhs1_stmt); mult_rhs2 = gimple_assign_rhs2 (rhs1_stmt); ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Extend widening_mul pass to handle fixed-point types 2010-07-19 18:35 ` Richard Sandiford @ 2010-07-19 21:53 ` Bernd Schmidt 2010-07-27 12:28 ` Ramana Radhakrishnan 0 siblings, 1 reply; 13+ messages in thread From: Bernd Schmidt @ 2010-07-19 21:53 UTC (permalink / raw) To: gcc-patches, rdsandiford On 07/19/2010 08:34 PM, Richard Sandiford wrote: > I did wonder about splitting the stuff out into a separate function. > The advantage of not doing is that it avoids duplicate work in cases > where we visit the macc first, and where the WIDEN_MULT_EXPR _can_ > be used. I thought that would be quite a common case. Not sure about that - normally I'd expect the multiply and add to appear roughly in the order they're executed. > Here's a version that splits it out and drops the two-at-once thing. > Only lightly tested, but does it look better? That's pretty much what I had in mind. Ok if it passes testing. Bernd ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Extend widening_mul pass to handle fixed-point types 2010-07-19 21:53 ` Bernd Schmidt @ 2010-07-27 12:28 ` Ramana Radhakrishnan 2010-07-27 15:54 ` Bernd Schmidt 0 siblings, 1 reply; 13+ messages in thread From: Ramana Radhakrishnan @ 2010-07-27 12:28 UTC (permalink / raw) To: Bernd Schmidt; +Cc: gcc-patches, rdsandiford On Mon, 2010-07-19 at 23:53 +0200, Bernd Schmidt wrote: > That's pretty much what I had in mind. Ok if it passes testing. This broke bootstraps on arm-linux-gnueabi and caused http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45067 . Taking a brief look at it this morning, I think the problem here is that the call to optab_for_tree_code is made with the wrong type of the expression. The presence of a widening optab for a WIDEN_MULT_PLUS_EXPR should be based on the type of ops->op2 rather than opnd0 because you have something like intval0 = shortval1 * shortval2 + intval1 whereas the expander is testing for a widening mult and plus where the results are into a HImode value and thus effectively for a widening operation into an HImode value. The assert that fails is because gcc_assert (icode != CODE_FOR_nothing); Something like this fixes the problem for the test-cases under question or would you prefer something else. A full bootstrap and test is underway. Ok to commit if no regressions ? cheers Ramana 2010-07-27 Ramana Radhakrishnan <ramana.radhakrishnan@arm.com> PR bootstrap/45067 * optabs.c (expand_widen_pattern_expr): Initialize widen_pattern_optab correctly for WIDEN_MULT_PLUS_EXPR and WIDEN_MULT_MINUS_EXPR. Index: gcc/optabs.c =================================================================== --- gcc/optabs.c (revision 162526) +++ gcc/optabs.c (working copy) @@ -511,14 +511,20 @@ oprnd0 = ops->op0; tmode0 = TYPE_MODE (TREE_TYPE (oprnd0)); - widen_pattern_optab = - optab_for_tree_code (ops->code, TREE_TYPE (oprnd0), optab_default); if (ops->code == WIDEN_MULT_PLUS_EXPR || ops->code == WIDEN_MULT_MINUS_EXPR) - icode = (int) optab_handler (widen_pattern_optab, + { + widen_pattern_optab = + optab_for_tree_code (ops->code, TREE_TYPE (ops->op2), optab_default); + icode = (int) optab_handler (widen_pattern_optab, TYPE_MODE (TREE_TYPE (ops->op2))); + } else - icode = (int) optab_handler (widen_pattern_optab, tmode0); + { + widen_pattern_optab = + optab_for_tree_code (ops->code, TREE_TYPE (oprnd0), optab_default); + icode = (int) optab_handler (widen_pattern_optab, tmode0); + } gcc_assert (icode != CODE_FOR_nothing); xmode0 = insn_data[icode].operand[1].mode; > > > Bernd ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Extend widening_mul pass to handle fixed-point types 2010-07-27 12:28 ` Ramana Radhakrishnan @ 2010-07-27 15:54 ` Bernd Schmidt 2010-07-27 17:10 ` Ramana Radhakrishnan 2010-07-28 15:43 ` Ramana Radhakrishnan 0 siblings, 2 replies; 13+ messages in thread From: Bernd Schmidt @ 2010-07-27 15:54 UTC (permalink / raw) To: ramana.radhakrishnan; +Cc: gcc-patches, rdsandiford On 07/27/2010 02:18 PM, Ramana Radhakrishnan wrote: > > On Mon, 2010-07-19 at 23:53 +0200, Bernd Schmidt wrote: >> That's pretty much what I had in mind. Ok if it passes testing. > > This broke bootstraps on arm-linux-gnueabi and caused > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45067 . > > Taking a brief look at it this morning, I think the problem here is that > the call to optab_for_tree_code is made with the wrong type of the > expression. The presence of a widening optab for a WIDEN_MULT_PLUS_EXPR > should be based on the type of ops->op2 rather than opnd0 because you > have something like intval0 = shortval1 * shortval2 + intval1 whereas > the expander is testing for a widening mult and plus where the results > are into a HImode value and thus effectively for a widening operation > into an HImode value. > > The assert that fails is because > gcc_assert (icode != CODE_FOR_nothing); > > Something like this fixes the problem for the test-cases under question > or would you prefer something else. A full bootstrap and test is > underway. > > Ok to commit if no regressions ? I think this is consistent with the code in tree-ssa-math-opts (there we use the type of the lhs, which should match that of operand 2). So, OK - I think the ARM testsuite has some widening-macc cases so you should notice if there's something wrong. > + { > + widen_pattern_optab = > + optab_for_tree_code (ops->code, TREE_TYPE (ops->op2), > optab_default); > + icode = (int) optab_handler (widen_pattern_optab, > TYPE_MODE (TREE_TYPE (ops->op2))); > + } > else > - icode = (int) optab_handler (widen_pattern_optab, tmode0); > + { > + widen_pattern_optab = > + optab_for_tree_code (ops->code, TREE_TYPE (oprnd0), > optab_default); > + icode = (int) optab_handler (widen_pattern_optab, tmode0); > + } This can be written slightly more simply assigning the type to some variable and then using that when computing widen_pattern_optab and icode. It would be nice to change that but I won't require it. Bernd ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Extend widening_mul pass to handle fixed-point types 2010-07-27 15:54 ` Bernd Schmidt @ 2010-07-27 17:10 ` Ramana Radhakrishnan 2010-07-28 15:43 ` Ramana Radhakrishnan 1 sibling, 0 replies; 13+ messages in thread From: Ramana Radhakrishnan @ 2010-07-27 17:10 UTC (permalink / raw) To: Bernd Schmidt; +Cc: gcc-patches, rdsandiford [-- Attachment #1: Type: text/plain, Size: 2913 bytes --] On Tue, 2010-07-27 at 17:51 +0200, Bernd Schmidt wrote: > On 07/27/2010 02:18 PM, Ramana Radhakrishnan wrote: > > > > On Mon, 2010-07-19 at 23:53 +0200, Bernd Schmidt wrote: > >> That's pretty much what I had in mind. Ok if it passes testing. > > > > This broke bootstraps on arm-linux-gnueabi and caused > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45067 . > > > > Taking a brief look at it this morning, I think the problem here is that > > the call to optab_for_tree_code is made with the wrong type of the > > expression. The presence of a widening optab for a WIDEN_MULT_PLUS_EXPR > > should be based on the type of ops->op2 rather than opnd0 because you > > have something like intval0 = shortval1 * shortval2 + intval1 whereas > > the expander is testing for a widening mult and plus where the results > > are into a HImode value and thus effectively for a widening operation > > into an HImode value. > > > > The assert that fails is because > > gcc_assert (icode != CODE_FOR_nothing); > > > > Something like this fixes the problem for the test-cases under question > > or would you prefer something else. A full bootstrap and test is > > underway. > > > > Ok to commit if no regressions ? > > I think this is consistent with the code in tree-ssa-math-opts (there we > use the type of the lhs, which should match that of operand 2). So, OK > - I think the ARM testsuite has some widening-macc cases so you should > notice if there's something wrong. Yep so I thought as well. Yes the tests in gcc.target/arm should cover these cases. Bootstrap proceeded further and ICE'd again in stage2 because we were applying gimple_assign_lhs on a non GIMPLE_ASSIGN statement. Attached is a simple test case that shows this problem with a cross-compiler as well. I am testing with this extra patch[1] applied on top of my previous patch which I think is obvious and will see what else is needed to be done to deal with the fallout of this patch with bootstrap on arm-linux-gnueabi. Verified with a simple testcase that we still do generate widening multiplies for the original testcase. I'll submit both these after I'm sure that bootstrap and regression testing completes. cheers Ramana 1. New patch being tested on top. 2010-07-27 Ramana Radhakrishnan <ramana.radhakrishnan@arm.com> * tree-ssa-math-opts.c (is_widening_mult_p): Return false if not an assignment. [ramrad01@e102325-lin gcc]$ svndiff tree-ssa-math-opts.c Index: tree-ssa-math-opts.c =================================================================== --- tree-ssa-math-opts.c (revision 162571) +++ tree-ssa-math-opts.c (working copy) @@ -1323,6 +1323,9 @@ is_widening_mult_p (gimple stmt, { tree type; + if (!is_gimple_assign (stmt)) + return false; + type = TREE_TYPE (gimple_assign_lhs (stmt)); if (TREE_CODE (type) != INTEGER_TYPE && TREE_CODE (type) != FIXED_POINT_TYPE) [-- Attachment #2: reduced.c --] [-- Type: text/x-csrc, Size: 2320 bytes --] /* ./cc1 -O2 -mcpu=cortex-a9 -quiet reduced.c */ typedef union tree_node *tree; enum tree_code { ERROR_MARK, }; enum tree_code_class { tcc_type, }; extern const enum tree_code_class tree_code_type[]; struct tree_base { __extension__ enum tree_code code : 16; unsigned unsigned_flag : 1; }; struct tree_type { unsigned int precision : 10; }; union tree_node { struct tree_base base; struct tree_type type; }; enum integer_type_kind { itk_int, itk_none }; extern tree integer_types[itk_none]; typedef struct cpp_reader cpp_reader; enum c_tree_index { CTI_INTMAX_TYPE, CTI_MAX }; extern tree c_global_trees[CTI_MAX]; static void builtin_define_constants (const char *, tree); static void builtin_define_stdint_macros (void) { builtin_define_constants ("__INTMAX_C", c_global_trees[CTI_INTMAX_TYPE]); } void c_cpp_builtins (cpp_reader *pfile) { builtin_define_stdint_macros (); } static const char * type_suffix (tree type) { static const char *const suffixes[] = { "", "U", "L", "UL", "LL", "ULL" }; int unsigned_suffix; int is_long; unsigned_suffix = (__extension__ ({ __typeof (type) const __t = (type); if (tree_code_type[(int) (((enum tree_code) (__t)->base.code))] != (tcc_type)) tree_class_check_failed (__t, (tcc_type), "/home/ramrad01/sources/trunk/gcc/c-family/c-cppbuiltin.c", 1084, __FUNCTION__); __t; })->base.unsigned_flag); if ((__extension__ ({ __typeof (type) const __t = (type); if (tree_code_type[(int) (((enum tree_code) (__t)->base.code))] != (tcc_type)) tree_class_check_failed (__t, (tcc_type), "/home/ramrad01/sources/trunk/gcc/c-family/c-cppbuiltin.c", 1085, __FUNCTION__); __t; })->type.precision) < (__extension__ ({ __typeof (integer_types[itk_int]) const __t = (integer_types[itk_int]); if (tree_code_type[(int) (((enum tree_code) (__t)->base.code))] != (tcc_type)) tree_class_check_failed (__t, (tcc_type), "/home/ramrad01/sources/trunk/gcc/c-family/c-cppbuiltin.c", 1085, __FUNCTION__); __t; })->type.precision)) return suffixes[is_long * 2 + unsigned_suffix]; } static void builtin_define_constants (const char *macro, tree type) { const char *suffix; char *buf; suffix = type_suffix (type); if (suffix[0] == 0) { buf = (char *) __builtin_alloca(strlen (macro) + 6); } } ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Extend widening_mul pass to handle fixed-point types 2010-07-27 15:54 ` Bernd Schmidt 2010-07-27 17:10 ` Ramana Radhakrishnan @ 2010-07-28 15:43 ` Ramana Radhakrishnan 2010-07-28 21:20 ` Richard Sandiford 1 sibling, 1 reply; 13+ messages in thread From: Ramana Radhakrishnan @ 2010-07-28 15:43 UTC (permalink / raw) To: Bernd Schmidt; +Cc: gcc-patches, rdsandiford On Tue, 2010-07-27 at 17:51 +0200, Bernd Schmidt wrote: > On 07/27/2010 02:18 PM, Ramana Radhakrishnan wrote: > > > > On Mon, 2010-07-19 at 23:53 +0200, Bernd Schmidt wrote: > >> That's pretty much what I had in mind. Ok if it passes testing. > > > > This broke bootstraps on arm-linux-gnueabi and caused > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45067 . > > > > Taking a brief look at it this morning, I think the problem here is that > > the call to optab_for_tree_code is made with the wrong type of the > > expression. The presence of a widening optab for a WIDEN_MULT_PLUS_EXPR > > should be based on the type of ops->op2 rather than opnd0 because you > > have something like intval0 = shortval1 * shortval2 + intval1 whereas > > the expander is testing for a widening mult and plus where the results > > are into a HImode value and thus effectively for a widening operation > > into an HImode value. > > > > The assert that fails is because > > gcc_assert (icode != CODE_FOR_nothing); > > > > Something like this fixes the problem for the test-cases under question > > or would you prefer something else. A full bootstrap and test is > > underway. > > > > Ok to commit if no regressions ? > > I think this is consistent with the code in tree-ssa-math-opts (there we > use the type of the lhs, which should match that of operand 2). So, OK > - I think the ARM testsuite has some widening-macc cases so you should > notice if there's something wrong. > > > + { > > + widen_pattern_optab = > > + optab_for_tree_code (ops->code, TREE_TYPE (ops->op2), > > optab_default); > > + icode = (int) optab_handler (widen_pattern_optab, > > TYPE_MODE (TREE_TYPE (ops->op2))); > > + } > > else > > - icode = (int) optab_handler (widen_pattern_optab, tmode0); > > + { > > + widen_pattern_optab = > > + optab_for_tree_code (ops->code, TREE_TYPE (oprnd0), > > optab_default); > > + icode = (int) optab_handler (widen_pattern_optab, tmode0); > > + } > > This can be written slightly more simply assigning the type to some > variable and then using that when computing widen_pattern_optab and > icode. It would be nice to change that but I won't require it. Hmmm there are regressions after my patch was applied. I had to do some rebuilds because trunk appears to be broken for other reasons at the minute for ARM which I shall investigate next. After these 2 patches to fix the ICEs on top of r162301 - I see breakages in the testsuite with execute/arith-rand.c and execute/arith-rand-ll.c . The difference in code generated is in the following case for -O2 is the following for arith-rand.c smlabb sl, r0, r7, sl | mla sl, r7, r0, sl with the failing case on the left. The problem appears to be as though the gimple pass for generating widening macs now generates a widening multiply and add for the case under question here while it didn't earlier. D.2130_36 = WIDEN_MULT_PLUS_EXPR <r1_30, yy_29, D.2129_35>; We weren't generating any widening operations before this patch (and I suspect my 2 patches are just for fixing the ICE's and doing the right thing). If it were just adding support for fixed point arithmetic then this is just broken because it shouldn't have changed the way in which code was generated for normal integer operations. Ideas ? I haven't looked at this in any great depth but the essential breakages in the C only testsuite are as below [2]. The native compiler has been configured with the following options - you can see the same result using a cross-compiler with the following parameters. --with-cpu=cortex-a9 --with-fpu=vfpv3-d16 --with-float=softfp cheers Ramana [1] Trunk as of this morning gives me a miscompare in stage2 and stage3 when my patches are applied which indicates a different underlying problem, the testsuite breakages are 162301 + the two patches applied. [2] FAIL: gcc.c-torture/execute/builtins/abs-1.c execution, -O2 -fwhopr FAIL: gcc.c-torture/execute/builtins/strstr-asm.c execution, -O2 -fwhopr FAIL: gcc.c-torture/execute/arith-rand-ll.c execution, -O2 FAIL: gcc.c-torture/execute/arith-rand-ll.c execution, -O3 -fomit-frame-pointer FAIL: gcc.c-torture/execute/arith-rand-ll.c execution, -O3 -fomit-frame-pointer -funroll-loops FAIL: gcc.c-torture/execute/arith-rand-ll.c execution, -O3 -fomit-frame-pointer -funroll-all-loops -finline-functions FAIL: gcc.c-torture/execute/arith-rand-ll.c execution, -O3 -g FAIL: gcc.c-torture/execute/arith-rand-ll.c execution, -Os FAIL: gcc.c-torture/execute/arith-rand-ll.c execution, -O2 -flto FAIL: gcc.c-torture/execute/arith-rand-ll.c execution, -O2 -fwhopr FAIL: gcc.c-torture/execute/arith-rand.c execution, -O2 FAIL: gcc.c-torture/execute/arith-rand.c execution, -O3 -fomit-frame-pointer FAIL: gcc.c-torture/execute/arith-rand.c execution, -O3 -fomit-frame-pointer -funroll-loops FAIL: gcc.c-torture/execute/arith-rand.c execution, -O3 -fomit-frame-pointer -funroll-all-loops -finline-functions FAIL: gcc.c-torture/execute/arith-rand.c execution, -O3 -g FAIL: gcc.c-torture/execute/arith-rand.c execution, -Os FAIL: gcc.c-torture/execute/arith-rand.c execution, -O2 -flto FAIL: gcc.c-torture/execute/arith-rand.c execution, -O2 -fwhopr FAIL: gcc.dg/c99-stdint-1.c (test for excess errors) FAIL: gcc.dg/c99-stdint-7.c (test for excess errors) FAIL: gcc.dg/cproj-fails-with-broken-glibc.c execution test FAIL: gcc.dg/uninit-13.c unconditional (test for warnings, line 8) FAIL: gcc.dg/uninit-13.c (test for excess errors) FAIL: gcc.dg/graphite/pr44391.c (test for excess errors) FAIL: gcc.dg/lto/20081111 c_lto_20081111_0.o-c_lto_20081111_1.o execute -O2 -fwhopr FAIL: gcc.dg/lto/20081112 c_lto_20081112_0.o-c_lto_20081112_1.o execute -O2 -fwhopr FAIL: gcc.dg/lto/ipareference2 c_lto_ipareference2_0.o-c_lto_ipareference2_1.o execute -O1 -fwhopr -fwhole-program FAIL: gcc.dg/torture/fp-int-convert-double.c -O1 execution test FAIL: gcc.dg/torture/fp-int-convert-long-double.c -O1 execution test FAIL: gcc.dg/torture/fp-int-convert-timode.c -O1 execution test FAIL: gcc.dg/torture/pr43879_1.c -O2 -fwhopr execution test FAIL: gcc.dg/tree-ssa/pr42585.c scan-tree-dump-times optimized "struct _fat_ptr _ans" 0 FAIL: gcc.dg/tree-ssa/pr42585.c scan-tree-dump-times optimized "struct _fat_ptr _T2" 0 FAIL: gcc.dg/vect/pr43430-1.c scan-tree-dump-times vect "vectorized 1 loops" 1 FAIL: gcc.dg/vect/vect-35.c scan-tree-dump-times vect "vectorized 2 loops" 1 FAIL: gcc.dg/vect/vect-peel-3.c scan-tree-dump-times vect "Vectorizing an unaligned access" 1 FAIL: gcc.dg/vect/vect-peel-4.c scan-tree-dump-times vect "Vectorizing an unaligned access" 1 FAIL: gcc.dg/vect/slp-reduc-3.c scan-tree-dump-times vect "vectorizing stmts using SLP" 1 FAIL: gcc.dg/vect/slp-reduc-6.c scan-tree-dump-times vect "vectorized 1 loops" 2 > > > Bernd ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Extend widening_mul pass to handle fixed-point types 2010-07-28 15:43 ` Ramana Radhakrishnan @ 2010-07-28 21:20 ` Richard Sandiford 2010-07-29 10:39 ` Ramana Radhakrishnan 2010-07-31 9:45 ` Richard Sandiford 0 siblings, 2 replies; 13+ messages in thread From: Richard Sandiford @ 2010-07-28 21:20 UTC (permalink / raw) To: ramana.radhakrishnan; +Cc: Bernd Schmidt, gcc-patches Ramana Radhakrishnan <ramana.radhakrishnan@arm.com> writes: > Hmmm there are regressions after my patch was applied. I had to do some > rebuilds because trunk appears to be broken for other reasons at the > minute for ARM which I shall investigate next. Yeah, I think it's the other way about: expand_widen_pattern_expr is using the right type and convert_plusminus_to_widen is using the wrong one. The type we pass to optab_for_tree_code determines the _sign_ of the multiplication, not the size, so we want to pass in the unwidened type. > We weren't generating any widening operations before this patch (and I > suspect my 2 patches are just for fixing the ICE's and doing the right > thing). If it were just adding support for fixed point arithmetic then > this is just broken because it shouldn't have changed the way in which > code was generated for normal integer operations. For the record, the patch wasn't just about adding fixed-point support. It was also about allowing multiply-accumulate to be used where no plain multiplication exists. So the problem for ARM was that the use of the wrong type in convert_plusminus_to_widen was being hidden by the use of the right type in convert_mult_widen (i.e. the patch exposed a latent bug). Could you give the attached patch a try? It also fixes a case where the code could create mixed sign-and-unsigned maccs (a bug that existed before my patch) and a botched call to is_widening_mult_p (a bug introduced by my patch, sorry). Richard gcc/ * tree-ssa-math-opts.c (convert_plusminus_to_widen): Fix type used in the call to optab_for_tree_code. Fix the second is_widening_mult_p call. Check that both unwidened operands have the same sign. Index: gcc/tree-ssa-math-opts.c =================================================================== --- gcc/tree-ssa-math-opts.c 2010-07-28 21:09:59.000000000 +0100 +++ gcc/tree-ssa-math-opts.c 2010-07-28 21:10:00.000000000 +0100 @@ -1414,13 +1414,6 @@ convert_plusminus_to_widen (gimple_stmt_ else wmult_code = WIDEN_MULT_PLUS_EXPR; - /* Verify that the machine can perform a widening multiply - accumulate in this mode/signedness combination, otherwise - this transformation is likely to pessimize code. */ - this_optab = optab_for_tree_code (wmult_code, type, optab_default); - if (optab_handler (this_optab, TYPE_MODE (type)) == CODE_FOR_nothing) - return false; - rhs1 = gimple_assign_rhs1 (stmt); rhs2 = gimple_assign_rhs2 (stmt); @@ -1447,37 +1440,49 @@ convert_plusminus_to_widen (gimple_stmt_ if (!is_widening_mult_p (rhs1_stmt, &type1, &mult_rhs1, &type2, &mult_rhs2)) return false; - mult_rhs1 = fold_convert (type1, mult_rhs1); - mult_rhs2 = fold_convert (type2, mult_rhs2); add_rhs = rhs2; } else if (rhs2_code == MULT_EXPR) { - if (!is_widening_mult_p (rhs1_stmt, &type1, &mult_rhs1, + if (!is_widening_mult_p (rhs2_stmt, &type1, &mult_rhs1, &type2, &mult_rhs2)) return false; - mult_rhs1 = fold_convert (type1, mult_rhs1); - mult_rhs2 = fold_convert (type2, mult_rhs2); add_rhs = rhs1; } else if (code == PLUS_EXPR && rhs1_code == WIDEN_MULT_EXPR) { mult_rhs1 = gimple_assign_rhs1 (rhs1_stmt); mult_rhs2 = gimple_assign_rhs2 (rhs1_stmt); + type1 = TREE_TYPE (mult_rhs1); + type2 = TREE_TYPE (mult_rhs2); add_rhs = rhs2; } else if (rhs2_code == WIDEN_MULT_EXPR) { mult_rhs1 = gimple_assign_rhs1 (rhs2_stmt); mult_rhs2 = gimple_assign_rhs2 (rhs2_stmt); + type1 = TREE_TYPE (mult_rhs1); + type2 = TREE_TYPE (mult_rhs2); add_rhs = rhs1; } else return false; + if (TYPE_UNSIGNED (type1) != TYPE_UNSIGNED (type2)) + return false; + + /* Verify that the machine can perform a widening multiply + accumulate in this mode/signedness combination, otherwise + this transformation is likely to pessimize code. */ + this_optab = optab_for_tree_code (wmult_code, type1, optab_default); + if (optab_handler (this_optab, TYPE_MODE (type)) == CODE_FOR_nothing) + return false; + /* ??? May need some type verification here? */ - gimple_assign_set_rhs_with_ops_1 (gsi, wmult_code, mult_rhs1, mult_rhs2, + gimple_assign_set_rhs_with_ops_1 (gsi, wmult_code, + fold_convert (type1, mult_rhs1), + fold_convert (type2, mult_rhs2), add_rhs); update_stmt (gsi_stmt (*gsi)); return true; ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Extend widening_mul pass to handle fixed-point types 2010-07-28 21:20 ` Richard Sandiford @ 2010-07-29 10:39 ` Ramana Radhakrishnan 2010-07-31 9:45 ` Richard Sandiford 2010-07-31 9:45 ` Richard Sandiford 1 sibling, 1 reply; 13+ messages in thread From: Ramana Radhakrishnan @ 2010-07-29 10:39 UTC (permalink / raw) To: Richard Sandiford; +Cc: gcc-patches, bernds [-- Attachment #1: Type: text/plain, Size: 6180 bytes --] Hi Richard, On Wed, 2010-07-28 at 21:15 +0100, Richard Sandiford wrote: > Ramana Radhakrishnan <ramana.radhakrishnan@arm.com> writes: > > Hmmm there are regressions after my patch was applied. I had to do some > > rebuilds because trunk appears to be broken for other reasons at the > > minute for ARM which I shall investigate next. > > Yeah, I think it's the other way about: expand_widen_pattern_expr > is using the right type and convert_plusminus_to_widen is using > the wrong one. The type we pass to optab_for_tree_code determines > the _sign_ of the multiplication, not the size, so we want to pass > in the unwidened type. Thanks for looking at this. Ah - ok. I see what you mean here. > > > We weren't generating any widening operations before this patch (and I > > suspect my 2 patches are just for fixing the ICE's and doing the right > > thing). If it were just adding support for fixed point arithmetic then > > this is just broken because it shouldn't have changed the way in which > > code was generated for normal integer operations. > > For the record, the patch wasn't just about adding fixed-point support. > It was also about allowing multiply-accumulate to be used where no plain > multiplication exists. So the problem for ARM was that the use of the > wrong type in convert_plusminus_to_widen was being hidden by the use of > the right type in convert_mult_widen (i.e. the patch exposed a latent bug). Thanks for the clarification , that makes more sense now. > > Could you give the attached patch a try? It also fixes a case where > the code could create mixed sign-and-unsigned maccs (a bug that existed > before my patch) and a botched call to is_widening_mult_p (a bug > introduced by my patch, sorry). Your patch didn't apply cleanly to pristine trunk or to 162431 and I had to do a manual merge so I'm not sure if I got all parts of it. Just to be absolutely clear, you still need to check for is_gimple_assign before using the stmt in gimple_assign_lhs in is_widening_mult_p or do you expect that to get subsumed by your patch ? Otherwise the compiler ICEs with this testcase [ice-on-gimple-phi.c]. Here's what I came up with that includes a manual merge of your patch assuming I got all the hunks, and my original trivial hunk to is_widening_mult_p which appeared to fix all the problems with simple testcases on trunk with a cross-compiler and what I'm bootstrapping and testing right now. cheers Ramana gcc/ * tree-ssa-math-opts.c (convert_plusminus_to_widen): Fix type used in the call to optab_for_tree_code. Fix the second is_widening_mult_p call. Check that both unwidened operands have the same sign. (is_widening_mult_p): Return false if stmt is not a GIMPLE_ASSIGN. [1] ./xgcc -B`pwd` -S -O2 -mcpu=cortex-a9 ~/ice-on-gimple-phi.c assignment./home/ramrad01/ice-on-gimple-phi.c:57:1: internal compiler error: gimple check: expected gimple_assign(error_mark), have gimple_phi() in gimple_assign_lhs, at gimple.h:1724 > > Richard > > > gcc/ > * tree-ssa-math-opts.c (convert_plusminus_to_widen): Fix type > used in the call to optab_for_tree_code. Fix the second > is_widening_mult_p call. Check that both unwidened operands > have the same sign. > > Index: gcc/tree-ssa-math-opts.c > =================================================================== > --- gcc/tree-ssa-math-opts.c 2010-07-28 21:09:59.000000000 +0100 > +++ gcc/tree-ssa-math-opts.c 2010-07-28 21:10:00.000000000 +0100 > @@ -1414,13 +1414,6 @@ convert_plusminus_to_widen (gimple_stmt_ > else > wmult_code = WIDEN_MULT_PLUS_EXPR; > > - /* Verify that the machine can perform a widening multiply > - accumulate in this mode/signedness combination, otherwise > - this transformation is likely to pessimize code. */ > - this_optab = optab_for_tree_code (wmult_code, type, optab_default); > - if (optab_handler (this_optab, TYPE_MODE (type)) == CODE_FOR_nothing) > - return false; > - > rhs1 = gimple_assign_rhs1 (stmt); > rhs2 = gimple_assign_rhs2 (stmt); > > @@ -1447,37 +1440,49 @@ convert_plusminus_to_widen (gimple_stmt_ > if (!is_widening_mult_p (rhs1_stmt, &type1, &mult_rhs1, > &type2, &mult_rhs2)) > return false; > - mult_rhs1 = fold_convert (type1, mult_rhs1); > - mult_rhs2 = fold_convert (type2, mult_rhs2); > add_rhs = rhs2; > } > else if (rhs2_code == MULT_EXPR) > { > - if (!is_widening_mult_p (rhs1_stmt, &type1, &mult_rhs1, > + if (!is_widening_mult_p (rhs2_stmt, &type1, &mult_rhs1, > &type2, &mult_rhs2)) > return false; > - mult_rhs1 = fold_convert (type1, mult_rhs1); > - mult_rhs2 = fold_convert (type2, mult_rhs2); > add_rhs = rhs1; > } > else if (code == PLUS_EXPR && rhs1_code == WIDEN_MULT_EXPR) > { > mult_rhs1 = gimple_assign_rhs1 (rhs1_stmt); > mult_rhs2 = gimple_assign_rhs2 (rhs1_stmt); > + type1 = TREE_TYPE (mult_rhs1); > + type2 = TREE_TYPE (mult_rhs2); > add_rhs = rhs2; > } > else if (rhs2_code == WIDEN_MULT_EXPR) > { > mult_rhs1 = gimple_assign_rhs1 (rhs2_stmt); > mult_rhs2 = gimple_assign_rhs2 (rhs2_stmt); > + type1 = TREE_TYPE (mult_rhs1); > + type2 = TREE_TYPE (mult_rhs2); > add_rhs = rhs1; > } > else > return false; > > + if (TYPE_UNSIGNED (type1) != TYPE_UNSIGNED (type2)) > + return false; > + > + /* Verify that the machine can perform a widening multiply > + accumulate in this mode/signedness combination, otherwise > + this transformation is likely to pessimize code. */ > + this_optab = optab_for_tree_code (wmult_code, type1, optab_default); > + if (optab_handler (this_optab, TYPE_MODE (type)) == CODE_FOR_nothing) > + return false; > + > /* ??? May need some type verification here? */ > > - gimple_assign_set_rhs_with_ops_1 (gsi, wmult_code, mult_rhs1, mult_rhs2, > + gimple_assign_set_rhs_with_ops_1 (gsi, wmult_code, > + fold_convert (type1, mult_rhs1), > + fold_convert (type2, mult_rhs2), > add_rhs); > update_stmt (gsi_stmt (*gsi)); > return true; [-- Attachment #2: pr45067-patch-take4.txt --] [-- Type: text/x-patch, Size: 3023 bytes --] Index: tree-ssa-math-opts.c =================================================================== --- tree-ssa-math-opts.c (revision 162431) +++ tree-ssa-math-opts.c (working copy) @@ -1323,6 +1323,9 @@ is_widening_mult_p (gimple stmt, { tree type; + if (!is_gimple_assign (stmt)) + return false; + type = TREE_TYPE (gimple_assign_lhs (stmt)); if (TREE_CODE (type) != INTEGER_TYPE && TREE_CODE (type) != FIXED_POINT_TYPE) @@ -1414,13 +1417,6 @@ convert_plusminus_to_widen (gimple_stmt_ else wmult_code = WIDEN_MULT_PLUS_EXPR; - /* Verify that the machine can perform a widening multiply - accumulate in this mode/signedness combination, otherwise - this transformation is likely to pessimize code. */ - this_optab = optab_for_tree_code (wmult_code, type, optab_default); - if (optab_handler (this_optab, TYPE_MODE (type)) == CODE_FOR_nothing) - return false; - rhs1 = gimple_assign_rhs1 (stmt); rhs2 = gimple_assign_rhs2 (stmt); @@ -1447,8 +1443,6 @@ convert_plusminus_to_widen (gimple_stmt_ if (!is_widening_mult_p (rhs1_stmt, &type1, &mult_rhs1, &type2, &mult_rhs2)) return false; - mult_rhs1 = fold_convert (type1, mult_rhs1); - mult_rhs2 = fold_convert (type2, mult_rhs2); add_rhs = rhs2; } else if (rhs2_code == MULT_EXPR) @@ -1456,14 +1450,14 @@ convert_plusminus_to_widen (gimple_stmt_ if (!is_widening_mult_p (rhs1_stmt, &type1, &mult_rhs1, &type2, &mult_rhs2)) return false; - mult_rhs1 = fold_convert (type1, mult_rhs1); - mult_rhs2 = fold_convert (type2, mult_rhs2); add_rhs = rhs1; } else if (code == PLUS_EXPR && rhs1_code == WIDEN_MULT_EXPR) { mult_rhs1 = gimple_assign_rhs1 (rhs1_stmt); mult_rhs2 = gimple_assign_rhs2 (rhs1_stmt); + type1 = TREE_TYPE (mult_rhs1); + type2 = TREE_TYPE (mult_rhs2); add_rhs = rhs2; } else if (rhs2_code == WIDEN_MULT_EXPR) @@ -1471,13 +1465,27 @@ convert_plusminus_to_widen (gimple_stmt_ mult_rhs1 = gimple_assign_rhs1 (rhs2_stmt); mult_rhs2 = gimple_assign_rhs2 (rhs2_stmt); add_rhs = rhs1; + type1 = TREE_TYPE (mult_rhs1); + type2 = TREE_TYPE (mult_rhs2); } else return false; + if (TYPE_UNSIGNED (type1) != TYPE_UNSIGNED (type2)) + return false; + + /* Verify that the machine can perform a widening multiply + accumulate in this mode/signedness combination, otherwise + this transformation is likely to pessimize code. */ + this_optab = optab_for_tree_code (wmult_code, type1, optab_default); + if (optab_handler (this_optab, TYPE_MODE (type)) == CODE_FOR_nothing) + return false; + /* ??? May need some type verification here? */ - gimple_assign_set_rhs_with_ops_1 (gsi, wmult_code, mult_rhs1, mult_rhs2, + gimple_assign_set_rhs_with_ops_1 (gsi, wmult_code, + fold_convert (type1, mult_rhs1), + fold_convert (type2, mult_rhs2), add_rhs); update_stmt (gsi_stmt (*gsi)); return true; [-- Attachment #3: ice-on-gimple-phi.c --] [-- Type: text/x-csrc, Size: 2320 bytes --] /* ./cc1 -O2 -mcpu=cortex-a9 -quiet reduced.c */ typedef union tree_node *tree; enum tree_code { ERROR_MARK, }; enum tree_code_class { tcc_type, }; extern const enum tree_code_class tree_code_type[]; struct tree_base { __extension__ enum tree_code code : 16; unsigned unsigned_flag : 1; }; struct tree_type { unsigned int precision : 10; }; union tree_node { struct tree_base base; struct tree_type type; }; enum integer_type_kind { itk_int, itk_none }; extern tree integer_types[itk_none]; typedef struct cpp_reader cpp_reader; enum c_tree_index { CTI_INTMAX_TYPE, CTI_MAX }; extern tree c_global_trees[CTI_MAX]; static void builtin_define_constants (const char *, tree); static void builtin_define_stdint_macros (void) { builtin_define_constants ("__INTMAX_C", c_global_trees[CTI_INTMAX_TYPE]); } void c_cpp_builtins (cpp_reader *pfile) { builtin_define_stdint_macros (); } static const char * type_suffix (tree type) { static const char *const suffixes[] = { "", "U", "L", "UL", "LL", "ULL" }; int unsigned_suffix; int is_long; unsigned_suffix = (__extension__ ({ __typeof (type) const __t = (type); if (tree_code_type[(int) (((enum tree_code) (__t)->base.code))] != (tcc_type)) tree_class_check_failed (__t, (tcc_type), "/home/ramrad01/sources/trunk/gcc/c-family/c-cppbuiltin.c", 1084, __FUNCTION__); __t; })->base.unsigned_flag); if ((__extension__ ({ __typeof (type) const __t = (type); if (tree_code_type[(int) (((enum tree_code) (__t)->base.code))] != (tcc_type)) tree_class_check_failed (__t, (tcc_type), "/home/ramrad01/sources/trunk/gcc/c-family/c-cppbuiltin.c", 1085, __FUNCTION__); __t; })->type.precision) < (__extension__ ({ __typeof (integer_types[itk_int]) const __t = (integer_types[itk_int]); if (tree_code_type[(int) (((enum tree_code) (__t)->base.code))] != (tcc_type)) tree_class_check_failed (__t, (tcc_type), "/home/ramrad01/sources/trunk/gcc/c-family/c-cppbuiltin.c", 1085, __FUNCTION__); __t; })->type.precision)) return suffixes[is_long * 2 + unsigned_suffix]; } static void builtin_define_constants (const char *macro, tree type) { const char *suffix; char *buf; suffix = type_suffix (type); if (suffix[0] == 0) { buf = (char *) __builtin_alloca(strlen (macro) + 6); } } ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Extend widening_mul pass to handle fixed-point types 2010-07-29 10:39 ` Ramana Radhakrishnan @ 2010-07-31 9:45 ` Richard Sandiford 0 siblings, 0 replies; 13+ messages in thread From: Richard Sandiford @ 2010-07-31 9:45 UTC (permalink / raw) To: ramana.radhakrishnan; +Cc: gcc-patches, bernds Ramana Radhakrishnan <ramana.radhakrishnan@arm.com> writes: >> Could you give the attached patch a try? It also fixes a case where >> the code could create mixed sign-and-unsigned maccs (a bug that existed >> before my patch) and a botched call to is_widening_mult_p (a bug >> introduced by my patch, sorry). > > Your patch didn't apply cleanly to pristine trunk or to 162431 and I had > to do a manual merge so I'm not sure if I got all parts of it. > > Just to be absolutely clear, you still need to check for > is_gimple_assign before using the stmt in gimple_assign_lhs in > is_widening_mult_p or do you expect that to get subsumed by your patch ? > Otherwise the compiler ICEs with this testcase [ice-on-gimple-phi.c]. For the record -- since I accidentally took this off-list when I started using the gmail web interface, sorry about that -- the problems Ramana had applying the patch were due to some kind of mailer issue. The ICE in ice-on-gimple-phi.c was due to the s/rhs1_stmt/rhs2_stmt/ part being missing from the manually-applied patch. Richard ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Extend widening_mul pass to handle fixed-point types 2010-07-28 21:20 ` Richard Sandiford 2010-07-29 10:39 ` Ramana Radhakrishnan @ 2010-07-31 9:45 ` Richard Sandiford 2010-07-31 13:03 ` Bernd Schmidt 1 sibling, 1 reply; 13+ messages in thread From: Richard Sandiford @ 2010-07-31 9:45 UTC (permalink / raw) To: ramana.radhakrishnan; +Cc: Bernd Schmidt, gcc-patches Richard Sandiford <rdsandiford@googlemail.com> writes: > Ramana Radhakrishnan <ramana.radhakrishnan@arm.com> writes: >> Hmmm there are regressions after my patch was applied. I had to do some >> rebuilds because trunk appears to be broken for other reasons at the >> minute for ARM which I shall investigate next. > > Yeah, I think it's the other way about: expand_widen_pattern_expr > is using the right type and convert_plusminus_to_widen is using > the wrong one. The type we pass to optab_for_tree_code determines > the _sign_ of the multiplication, not the size, so we want to pass > in the unwidened type. > >> We weren't generating any widening operations before this patch (and I >> suspect my 2 patches are just for fixing the ICE's and doing the right >> thing). If it were just adding support for fixed point arithmetic then >> this is just broken because it shouldn't have changed the way in which >> code was generated for normal integer operations. > > For the record, the patch wasn't just about adding fixed-point support. > It was also about allowing multiply-accumulate to be used where no plain > multiplication exists. So the problem for ARM was that the use of the > wrong type in convert_plusminus_to_widen was being hidden by the use of > the right type in convert_mult_widen (i.e. the patch exposed a latent bug). > > Could you give the attached patch a try? It also fixes a case where > the code could create mixed sign-and-unsigned maccs (a bug that existed > before my patch) and a botched call to is_widening_mult_p (a bug > introduced by my patch, sorry). Now bootstrapped & regression-tested on x86_64-linux-gnu. Ramana also confirms that it fixes the ICEs and incorrect arith-rand-ll.c code on ARM. OK to install? Richard > gcc/ > * tree-ssa-math-opts.c (convert_plusminus_to_widen): Fix type > used in the call to optab_for_tree_code. Fix the second > is_widening_mult_p call. Check that both unwidened operands > have the same sign. > > Index: gcc/tree-ssa-math-opts.c > =================================================================== > --- gcc/tree-ssa-math-opts.c 2010-07-28 21:09:59.000000000 +0100 > +++ gcc/tree-ssa-math-opts.c 2010-07-28 21:10:00.000000000 +0100 > @@ -1414,13 +1414,6 @@ convert_plusminus_to_widen (gimple_stmt_ > else > wmult_code = WIDEN_MULT_PLUS_EXPR; > > - /* Verify that the machine can perform a widening multiply > - accumulate in this mode/signedness combination, otherwise > - this transformation is likely to pessimize code. */ > - this_optab = optab_for_tree_code (wmult_code, type, optab_default); > - if (optab_handler (this_optab, TYPE_MODE (type)) == CODE_FOR_nothing) > - return false; > - > rhs1 = gimple_assign_rhs1 (stmt); > rhs2 = gimple_assign_rhs2 (stmt); > > @@ -1447,37 +1440,49 @@ convert_plusminus_to_widen (gimple_stmt_ > if (!is_widening_mult_p (rhs1_stmt, &type1, &mult_rhs1, > &type2, &mult_rhs2)) > return false; > - mult_rhs1 = fold_convert (type1, mult_rhs1); > - mult_rhs2 = fold_convert (type2, mult_rhs2); > add_rhs = rhs2; > } > else if (rhs2_code == MULT_EXPR) > { > - if (!is_widening_mult_p (rhs1_stmt, &type1, &mult_rhs1, > + if (!is_widening_mult_p (rhs2_stmt, &type1, &mult_rhs1, > &type2, &mult_rhs2)) > return false; > - mult_rhs1 = fold_convert (type1, mult_rhs1); > - mult_rhs2 = fold_convert (type2, mult_rhs2); > add_rhs = rhs1; > } > else if (code == PLUS_EXPR && rhs1_code == WIDEN_MULT_EXPR) > { > mult_rhs1 = gimple_assign_rhs1 (rhs1_stmt); > mult_rhs2 = gimple_assign_rhs2 (rhs1_stmt); > + type1 = TREE_TYPE (mult_rhs1); > + type2 = TREE_TYPE (mult_rhs2); > add_rhs = rhs2; > } > else if (rhs2_code == WIDEN_MULT_EXPR) > { > mult_rhs1 = gimple_assign_rhs1 (rhs2_stmt); > mult_rhs2 = gimple_assign_rhs2 (rhs2_stmt); > + type1 = TREE_TYPE (mult_rhs1); > + type2 = TREE_TYPE (mult_rhs2); > add_rhs = rhs1; > } > else > return false; > > + if (TYPE_UNSIGNED (type1) != TYPE_UNSIGNED (type2)) > + return false; > + > + /* Verify that the machine can perform a widening multiply > + accumulate in this mode/signedness combination, otherwise > + this transformation is likely to pessimize code. */ > + this_optab = optab_for_tree_code (wmult_code, type1, optab_default); > + if (optab_handler (this_optab, TYPE_MODE (type)) == CODE_FOR_nothing) > + return false; > + > /* ??? May need some type verification here? */ > > - gimple_assign_set_rhs_with_ops_1 (gsi, wmult_code, mult_rhs1, mult_rhs2, > + gimple_assign_set_rhs_with_ops_1 (gsi, wmult_code, > + fold_convert (type1, mult_rhs1), > + fold_convert (type2, mult_rhs2), > add_rhs); > update_stmt (gsi_stmt (*gsi)); > return true; ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Extend widening_mul pass to handle fixed-point types 2010-07-31 9:45 ` Richard Sandiford @ 2010-07-31 13:03 ` Bernd Schmidt 0 siblings, 0 replies; 13+ messages in thread From: Bernd Schmidt @ 2010-07-31 13:03 UTC (permalink / raw) To: ramana.radhakrishnan, gcc-patches, rdsandiford On 07/31/2010 09:30 AM, Richard Sandiford wrote: > Now bootstrapped & regression-tested on x86_64-linux-gnu. Ramana > also confirms that it fixes the ICEs and incorrect arith-rand-ll.c > code on ARM. OK to install? I think this is OK. While looking at it I've been somewhat confused by the fact that we choose an optab based on the type of the narrow operands, then choose the right entry in the optab based on the mode of the wide operand... but that seems unrelated to your patches. So, please install, and thanks. Bernd ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2010-07-31 11:41 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-07-18 12:03 Extend widening_mul pass to handle fixed-point types Richard Sandiford 2010-07-18 20:47 ` Bernd Schmidt 2010-07-19 18:35 ` Richard Sandiford 2010-07-19 21:53 ` Bernd Schmidt 2010-07-27 12:28 ` Ramana Radhakrishnan 2010-07-27 15:54 ` Bernd Schmidt 2010-07-27 17:10 ` Ramana Radhakrishnan 2010-07-28 15:43 ` Ramana Radhakrishnan 2010-07-28 21:20 ` Richard Sandiford 2010-07-29 10:39 ` Ramana Radhakrishnan 2010-07-31 9:45 ` Richard Sandiford 2010-07-31 9:45 ` Richard Sandiford 2010-07-31 13:03 ` Bernd Schmidt
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).