From patchwork Sat Jul 9 14:43:53 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Stubbs X-Patchwork-Id: 2629 Return-Path: X-Original-To: patchwork@peony.canonical.com Delivered-To: patchwork@peony.canonical.com Received: from fiordland.canonical.com (fiordland.canonical.com [91.189.94.145]) by peony.canonical.com (Postfix) with ESMTP id 1ED62243B5 for ; Sat, 9 Jul 2011 14:44:05 +0000 (UTC) Received: from mail-qy0-f180.google.com (mail-qy0-f180.google.com [209.85.216.180]) by fiordland.canonical.com (Postfix) with ESMTP id A6816A18640 for ; Sat, 9 Jul 2011 14:44:04 +0000 (UTC) Received: by qyk30 with SMTP id 30so1893474qyk.11 for ; Sat, 09 Jul 2011 07:44:04 -0700 (PDT) Received: by 10.229.102.98 with SMTP id f34mr2402320qco.42.1310222643985; Sat, 09 Jul 2011 07:44:03 -0700 (PDT) X-Forwarded-To: linaro-patchwork@canonical.com X-Forwarded-For: patch@linaro.org linaro-patchwork@canonical.com Delivered-To: patches@linaro.org Received: by 10.229.48.135 with SMTP id r7cs159054qcf; Sat, 9 Jul 2011 07:44:03 -0700 (PDT) Received: by 10.68.19.106 with SMTP id d10mr4842604pbe.226.1310222641790; Sat, 09 Jul 2011 07:44:01 -0700 (PDT) Received: from mail.codesourcery.com (mail.codesourcery.com [38.113.113.100]) by mx.google.com with ESMTPS id v7si53174044wfk.83.2011.07.09.07.43.59 (version=TLSv1/SSLv3 cipher=OTHER); Sat, 09 Jul 2011 07:44:00 -0700 (PDT) Received-SPF: pass (google.com: domain of ams@codesourcery.com designates 38.113.113.100 as permitted sender) client-ip=38.113.113.100; Authentication-Results: mx.google.com; spf=pass (google.com: domain of ams@codesourcery.com designates 38.113.113.100 as permitted sender) smtp.mail=ams@codesourcery.com Received: (qmail 8279 invoked from network); 9 Jul 2011 14:43:56 -0000 Received: from unknown (HELO ?192.168.0.100?) (ams@127.0.0.2) by mail.codesourcery.com with ESMTPA; 9 Jul 2011 14:43:56 -0000 Message-ID: <4E186929.9080805@codesourcery.com> Date: Sat, 09 Jul 2011 15:43:53 +0100 From: Andrew Stubbs User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.17) Gecko/20110516 Lightning/1.0b2 Thunderbird/3.1.10 MIME-Version: 1.0 To: gcc-patches@gcc.gnu.org CC: patches@linaro.org Subject: Re: [PATCH (1/7)] New optab framework for widening multiplies References: <4E034EF2.3070503@codesourcery.com> <4E034F98.5000809@codesourcery.com> In-Reply-To: <4E034F98.5000809@codesourcery.com> On 23/06/11 15:37, Andrew Stubbs wrote: > This patch should have no effect on the compiler output. It merely > replaces one way to represent widening operations with another, and > refactors the other parts of the compiler to match. The rest of the > patch set uses this new framework to implement the optimization > improvements. > > I considered and discarded many approaches to this patch before arriving > at this solution, and I feel sure that there'll be somebody out there > who will think I chose the wrong one, so let me first explain how I got > here .... > > The aim is to be able to encode and query optabs that have any given > input mode, and any given output mode. This is similar to the > convert_optab, but not compatible with that optab since it is handled > completely differently in the code. > > (Just to be clear, the existing widening multiply support only covers > instructions that widen by *one* mode, so it's only ever been necessary > to know the output mode, up to now.) > > Option 1 was to add a second dimension to the handlers table in optab_d, > but I discarded this option because it would increase the memory usage > by the square of the number of modes, which is a bit much. > > Option 2 was to add a whole new optab, similar to optab_d, but with a > second dimension like convert_optab_d, however this turned out to cause > way too many pointer type mismatches in the code, and would have been > very difficult to fix up. > > Option 3 was to add new optab entries for widening by two modes, by > three modes, and so on. True, I would only need to add one extra set for > what I need, but there would be so many places in the code that compare > against smul_widen_optab, for example, that would need to be taught > about these, that it seemed like a bad idea. > > Option 4 was to have a separate table that contained the widening > operations, and refer to that whenever a widening entry in the main > optab is referenced, but I found that there was no easy way to do the > mapping without putting some sort of switch table in > widening_optab_handler, and that negates the other advantages. > > So, what I've done in the end is add a new pointer entry "widening" into > optab_d, and dynamically build the widening operations table for each > optab that needs it. I've then added new accessor functions that take > both input and output modes, and altered the code to use them where > appropriate. > > The down-side of this approach is that the optab entries for widening > operations now have two "handlers" tables, one of which is redundant. > That said, those cases are in the minority, and it is the smaller table > which is unused. > > If people find that very distasteful, it might be possible to remove the > *_widen_optab entries and unify smul_optab with smul_widen_optab, and so > on, and save space that way. I've not done so yet, but I expect I could > if people feel strongly about it. > > As a side-effect, it's now possible for any optab to be "widening", > should some target happen to have a widening add, shift, or whatever. > > Is this patch OK? This update has been rebaselined to fix some conflicts with other recent commits in this area. I also identified a small bug which resulted in the operands to some commutative operations being reversed. I don't believe the bug did any harm, logically speaking, but I suppose there could be a testcase that resulted in worse code being generated. With this fix, I now see exactly matching output in all my testcases. Andrew 2011-07-09 Andrew Stubbs gcc/ * expr.c (expand_expr_real_2): Use widening_optab_handler. * genopinit.c (optabs): Use set_widening_optab_handler for $N. (gen_insn): $N now means $a must be wider than $b, not consecutive. * optabs.c (expand_widen_pattern_expr): Use widening_optab_handler. (expand_binop_directly): Likewise. (expand_binop): Likewise. * optabs.h (widening_optab_handlers): New struct. (optab_d): New member, 'widening'. (widening_optab_handler): New function. (set_widening_optab_handler): New function. * tree-ssa-math-opts.c (convert_mult_to_widen): Use widening_optab_handler. (convert_plusminus_to_widen): Likewise. --- a/gcc/expr.c +++ b/gcc/expr.c @@ -7640,7 +7640,8 @@ expand_expr_real_2 (sepops ops, rtx target, enum machine_mode tmode, this_optab = usmul_widen_optab; if (mode == GET_MODE_2XWIDER_MODE (innermode)) { - if (optab_handler (this_optab, mode) != CODE_FOR_nothing) + if (widening_optab_handler (this_optab, mode, innermode) + != CODE_FOR_nothing) { if (TYPE_UNSIGNED (TREE_TYPE (treeop0))) expand_operands (treeop0, treeop1, NULL_RTX, &op0, &op1, @@ -7667,7 +7668,8 @@ expand_expr_real_2 (sepops ops, rtx target, enum machine_mode tmode, if (mode == GET_MODE_2XWIDER_MODE (innermode) && TREE_CODE (treeop0) != INTEGER_CST) { - if (optab_handler (this_optab, mode) != CODE_FOR_nothing) + if (widening_optab_handler (this_optab, mode, innermode) + != CODE_FOR_nothing) { expand_operands (treeop0, treeop1, NULL_RTX, &op0, &op1, EXPAND_NORMAL); @@ -7675,7 +7677,8 @@ expand_expr_real_2 (sepops ops, rtx target, enum machine_mode tmode, unsignedp, this_optab); return REDUCE_BIT_FIELD (temp); } - if (optab_handler (other_optab, mode) != CODE_FOR_nothing + if (widening_optab_handler (other_optab, mode, innermode) + != CODE_FOR_nothing && innermode == word_mode) { rtx htem, hipart; --- a/gcc/genopinit.c +++ b/gcc/genopinit.c @@ -46,10 +46,12 @@ along with GCC; see the file COPYING3. If not see used. $A and $B are replaced with the full name of the mode; $a and $b are replaced with the short form of the name, as above. - If $N is present in the pattern, it means the two modes must be consecutive - widths in the same mode class (e.g, QImode and HImode). $I means that - only full integer modes should be considered for the next mode, and $F - means that only float modes should be considered. + If $N is present in the pattern, it means the two modes must be in + the same mode class, and $b must be greater than $a (e.g, QImode + and HImode). + + $I means that only full integer modes should be considered for the + next mode, and $F means that only float modes should be considered. $P means that both full and partial integer modes should be considered. $Q means that only fixed-point modes should be considered. @@ -99,17 +101,17 @@ static const char * const optabs[] = "set_optab_handler (smulv_optab, $A, CODE_FOR_$(mulv$I$a3$))", "set_optab_handler (umul_highpart_optab, $A, CODE_FOR_$(umul$a3_highpart$))", "set_optab_handler (smul_highpart_optab, $A, CODE_FOR_$(smul$a3_highpart$))", - "set_optab_handler (smul_widen_optab, $B, CODE_FOR_$(mul$a$b3$)$N)", - "set_optab_handler (umul_widen_optab, $B, CODE_FOR_$(umul$a$b3$)$N)", - "set_optab_handler (usmul_widen_optab, $B, CODE_FOR_$(usmul$a$b3$)$N)", - "set_optab_handler (smadd_widen_optab, $B, CODE_FOR_$(madd$a$b4$)$N)", - "set_optab_handler (umadd_widen_optab, $B, CODE_FOR_$(umadd$a$b4$)$N)", - "set_optab_handler (ssmadd_widen_optab, $B, CODE_FOR_$(ssmadd$a$b4$)$N)", - "set_optab_handler (usmadd_widen_optab, $B, CODE_FOR_$(usmadd$a$b4$)$N)", - "set_optab_handler (smsub_widen_optab, $B, CODE_FOR_$(msub$a$b4$)$N)", - "set_optab_handler (umsub_widen_optab, $B, CODE_FOR_$(umsub$a$b4$)$N)", - "set_optab_handler (ssmsub_widen_optab, $B, CODE_FOR_$(ssmsub$a$b4$)$N)", - "set_optab_handler (usmsub_widen_optab, $B, CODE_FOR_$(usmsub$a$b4$)$N)", + "set_widening_optab_handler (smul_widen_optab, $B, $A, CODE_FOR_$(mul$a$b3$)$N)", + "set_widening_optab_handler (umul_widen_optab, $B, $A, CODE_FOR_$(umul$a$b3$)$N)", + "set_widening_optab_handler (usmul_widen_optab, $B, $A, CODE_FOR_$(usmul$a$b3$)$N)", + "set_widening_optab_handler (smadd_widen_optab, $B, $A, CODE_FOR_$(madd$a$b4$)$N)", + "set_widening_optab_handler (umadd_widen_optab, $B, $A, CODE_FOR_$(umadd$a$b4$)$N)", + "set_widening_optab_handler (ssmadd_widen_optab, $B, $A, CODE_FOR_$(ssmadd$a$b4$)$N)", + "set_widening_optab_handler (usmadd_widen_optab, $B, $A, CODE_FOR_$(usmadd$a$b4$)$N)", + "set_widening_optab_handler (smsub_widen_optab, $B, $A, CODE_FOR_$(msub$a$b4$)$N)", + "set_widening_optab_handler (umsub_widen_optab, $B, $A, CODE_FOR_$(umsub$a$b4$)$N)", + "set_widening_optab_handler (ssmsub_widen_optab, $B, $A, CODE_FOR_$(ssmsub$a$b4$)$N)", + "set_widening_optab_handler (usmsub_widen_optab, $B, $A, CODE_FOR_$(usmsub$a$b4$)$N)", "set_optab_handler (sdiv_optab, $A, CODE_FOR_$(div$a3$))", "set_optab_handler (ssdiv_optab, $A, CODE_FOR_$(ssdiv$Q$a3$))", "set_optab_handler (sdivv_optab, $A, CODE_FOR_$(div$V$I$a3$))", @@ -305,7 +307,7 @@ gen_insn (rtx insn) { int force_float = 0, force_int = 0, force_partial_int = 0; int force_fixed = 0; - int force_consec = 0; + int force_wider = 0; int matches = 1; for (pp = optabs[pindex]; pp[0] != '$' || pp[1] != '('; pp++) @@ -323,7 +325,7 @@ gen_insn (rtx insn) switch (*++pp) { case 'N': - force_consec = 1; + force_wider = 1; break; case 'I': force_int = 1; @@ -392,7 +394,10 @@ gen_insn (rtx insn) || mode_class[i] == MODE_VECTOR_FRACT || mode_class[i] == MODE_VECTOR_UFRACT || mode_class[i] == MODE_VECTOR_ACCUM - || mode_class[i] == MODE_VECTOR_UACCUM)) + || mode_class[i] == MODE_VECTOR_UACCUM) + && (! force_wider + || *pp == 'a' + || m1 < i)) break; } @@ -412,8 +417,7 @@ gen_insn (rtx insn) } if (matches && pp[0] == '$' && pp[1] == ')' - && *np == 0 - && (! force_consec || (int) GET_MODE_WIDER_MODE(m1) == m2)) + && *np == 0) break; } --- a/gcc/optabs.c +++ b/gcc/optabs.c @@ -515,8 +515,8 @@ expand_widen_pattern_expr (sepops ops, rtx op0, rtx op1, rtx wide_op, 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 = optab_handler (widen_pattern_optab, - TYPE_MODE (TREE_TYPE (ops->op2))); + icode = widening_optab_handler (widen_pattern_optab, + TYPE_MODE (TREE_TYPE (ops->op2)), tmode0); else icode = optab_handler (widen_pattern_optab, tmode0); gcc_assert (icode != CODE_FOR_nothing); @@ -1242,7 +1242,8 @@ expand_binop_directly (enum machine_mode mode, optab binoptab, rtx target, int unsignedp, enum optab_methods methods, rtx last) { - enum insn_code icode = optab_handler (binoptab, mode); + enum machine_mode from_mode = GET_MODE (op0); + enum insn_code icode = widening_optab_handler (binoptab, mode, from_mode); enum machine_mode xmode0 = insn_data[(int) icode].operand[1].mode; enum machine_mode xmode1 = insn_data[(int) icode].operand[2].mode; enum machine_mode mode0, mode1, tmp_mode; @@ -1389,7 +1390,8 @@ expand_binop (enum machine_mode mode, optab binoptab, rtx op0, rtx op1, /* If we can do it with a three-operand insn, do so. */ if (methods != OPTAB_MUST_WIDEN - && optab_handler (binoptab, mode) != CODE_FOR_nothing) + && widening_optab_handler (binoptab, mode, GET_MODE (op0)) + != CODE_FOR_nothing) { temp = expand_binop_directly (mode, binoptab, op0, op1, target, unsignedp, methods, last); @@ -1429,8 +1431,9 @@ expand_binop (enum machine_mode mode, optab binoptab, rtx op0, rtx op1, if (binoptab == smul_optab && GET_MODE_2XWIDER_MODE (mode) != VOIDmode - && (optab_handler ((unsignedp ? umul_widen_optab : smul_widen_optab), - GET_MODE_2XWIDER_MODE (mode)) + && (widening_optab_handler ((unsignedp ? umul_widen_optab + : smul_widen_optab), + GET_MODE_2XWIDER_MODE (mode), mode) != CODE_FOR_nothing)) { temp = expand_binop (GET_MODE_2XWIDER_MODE (mode), @@ -1457,12 +1460,14 @@ expand_binop (enum machine_mode mode, optab binoptab, rtx op0, rtx op1, wider_mode != VOIDmode; wider_mode = GET_MODE_WIDER_MODE (wider_mode)) { - if (optab_handler (binoptab, wider_mode) != CODE_FOR_nothing + if (optab_handler (binoptab, wider_mode) + != CODE_FOR_nothing || (binoptab == smul_optab && GET_MODE_WIDER_MODE (wider_mode) != VOIDmode - && (optab_handler ((unsignedp ? umul_widen_optab - : smul_widen_optab), - GET_MODE_WIDER_MODE (wider_mode)) + && (widening_optab_handler ((unsignedp ? umul_widen_optab + : smul_widen_optab), + GET_MODE_WIDER_MODE (wider_mode), + mode) != CODE_FOR_nothing))) { rtx xop0 = op0, xop1 = op1; @@ -1895,8 +1900,8 @@ expand_binop (enum machine_mode mode, optab binoptab, rtx op0, rtx op1, && optab_handler (add_optab, word_mode) != CODE_FOR_nothing) { rtx product = NULL_RTX; - - if (optab_handler (umul_widen_optab, mode) != CODE_FOR_nothing) + if (widening_optab_handler (umul_widen_optab, mode, word_mode) + != CODE_FOR_nothing) { product = expand_doubleword_mult (mode, op0, op1, target, true, methods); @@ -1905,7 +1910,8 @@ expand_binop (enum machine_mode mode, optab binoptab, rtx op0, rtx op1, } if (product == NULL_RTX - && optab_handler (smul_widen_optab, mode) != CODE_FOR_nothing) + && widening_optab_handler (smul_widen_optab, mode, word_mode) + != CODE_FOR_nothing) { product = expand_doubleword_mult (mode, op0, op1, target, false, methods); @@ -1996,7 +2002,8 @@ expand_binop (enum machine_mode mode, optab binoptab, rtx op0, rtx op1, wider_mode != VOIDmode; wider_mode = GET_MODE_WIDER_MODE (wider_mode)) { - if (optab_handler (binoptab, wider_mode) != CODE_FOR_nothing + if (widening_optab_handler (binoptab, wider_mode, mode) + != CODE_FOR_nothing || (methods == OPTAB_LIB && optab_libfunc (binoptab, wider_mode))) { --- a/gcc/optabs.h +++ b/gcc/optabs.h @@ -42,6 +42,11 @@ struct optab_handlers int insn_code; }; +struct widening_optab_handlers +{ + struct optab_handlers handlers[NUM_MACHINE_MODES][NUM_MACHINE_MODES]; +}; + struct optab_d { enum rtx_code code; @@ -50,6 +55,7 @@ struct optab_d void (*libcall_gen)(struct optab_d *, const char *name, char suffix, enum machine_mode); struct optab_handlers handlers[NUM_MACHINE_MODES]; + struct widening_optab_handlers *widening; }; typedef struct optab_d * optab; @@ -876,6 +882,23 @@ optab_handler (optab op, enum machine_mode mode) + (int) CODE_FOR_nothing); } +/* Like optab_handler, but for widening_operations that have a TO_MODE and + a FROM_MODE. */ + +static inline enum insn_code +widening_optab_handler (optab op, enum machine_mode to_mode, + enum machine_mode from_mode) +{ + if (to_mode == from_mode) + return optab_handler (op, to_mode); + + if (op->widening) + return (enum insn_code) (op->widening->handlers[(int) to_mode][(int) from_mode].insn_code + + (int) CODE_FOR_nothing); + + return CODE_FOR_nothing; +} + /* Record that insn CODE should be used to implement mode MODE of OP. */ static inline void @@ -884,6 +907,26 @@ set_optab_handler (optab op, enum machine_mode mode, enum insn_code code) op->handlers[(int) mode].insn_code = (int) code - (int) CODE_FOR_nothing; } +/* Like set_optab_handler, but for widening operations that have a TO_MODE + and a FROM_MODE. */ + +static inline void +set_widening_optab_handler (optab op, enum machine_mode to_mode, + enum machine_mode from_mode, enum insn_code code) +{ + if (to_mode == from_mode) + set_optab_handler (op, to_mode, code); + else + { + if (op->widening == NULL) + op->widening = (struct widening_optab_handlers *) + xcalloc (1, sizeof (struct widening_optab_handlers)); + + op->widening->handlers[(int) to_mode][(int) from_mode].insn_code + = (int) code - (int) CODE_FOR_nothing; + } +} + /* Return the insn used to perform conversion OP from mode FROM_MODE to mode TO_MODE; return CODE_FOR_nothing if the target does not have such an insn. */ --- a/gcc/tree-ssa-math-opts.c +++ b/gcc/tree-ssa-math-opts.c @@ -2055,6 +2055,8 @@ convert_mult_to_widen (gimple stmt) { tree lhs, rhs1, rhs2, type, type1, type2; enum insn_code handler; + enum machine_mode to_mode, from_mode; + optab op; lhs = gimple_assign_lhs (stmt); type = TREE_TYPE (lhs); @@ -2064,12 +2066,17 @@ convert_mult_to_widen (gimple stmt) if (!is_widening_mult_p (stmt, &type1, &rhs1, &type2, &rhs2)) return false; + to_mode = TYPE_MODE (type); + from_mode = TYPE_MODE (type1); + if (TYPE_UNSIGNED (type1) && TYPE_UNSIGNED (type2)) - handler = optab_handler (umul_widen_optab, TYPE_MODE (type)); + op = umul_widen_optab; else if (!TYPE_UNSIGNED (type1) && !TYPE_UNSIGNED (type2)) - handler = optab_handler (smul_widen_optab, TYPE_MODE (type)); + op = smul_widen_optab; else - handler = optab_handler (usmul_widen_optab, TYPE_MODE (type)); + op = usmul_widen_optab; + + handler = widening_optab_handler (op, to_mode, from_mode); if (handler == CODE_FOR_nothing) return false; @@ -2171,7 +2178,8 @@ convert_plusminus_to_widen (gimple_stmt_iterator *gsi, gimple stmt, 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) + if (widening_optab_handler (this_optab, TYPE_MODE (type), TYPE_MODE (type1)) + == CODE_FOR_nothing) return false; /* ??? May need some type verification here? */