On Mon, May 23, 2016 at 10:58 AM, Prathamesh Kulkarni <prathamesh.kulka...@linaro.org> wrote: > Hi, > I have updated my patch for divmod (attached), which was originally > based on Kugan's patch. > The patch transforms stmts with code TRUNC_DIV_EXPR and TRUNC_MOD_EXPR > having same operands to divmod representation, so we can cse computation of > mod. > > t1 = a TRUNC_DIV_EXPR b; > t2 = a TRUNC_MOD_EXPR b > is transformed to: > complex_tmp = DIVMOD (a, b); > t1 = REALPART_EXPR (complex_tmp); > t2 = IMAGPART_EXPR (complex_tmp); > > * New hook divmod_expand_libfunc > The rationale for introducing the hook is that different targets have > incompatible calling conventions for divmod libfunc. > Currently three ports define divmod libfunc: c6x, spu and arm. > c6x and spu follow the convention of libgcc2.c:__udivmoddi4: > return quotient and store remainder in argument passed as pointer, > while the arm version takes two arguments and returns both > quotient and remainder having mode double the size of the operand mode. > The port should hence override the hook expand_divmod_libfunc > to generate call to target-specific divmod. > Ports should define this hook if: > a) The port does not have divmod or div insn for the given mode. > b) The port defines divmod libfunc for the given mode. > The default hook default_expand_divmod_libfunc() generates call > to libgcc2.c:__udivmoddi4 provided the operands are unsigned and > are of DImode. > > Patch passes bootstrap+test on x86_64-unknown-linux-gnu and > cross-tested on arm*-*-*. > Bootstrap+test in progress on arm-linux-gnueabihf. > Does this patch look OK ?
diff --git a/gcc/targhooks.c b/gcc/targhooks.c index 6b4601b..e4a021a 100644 --- a/gcc/targhooks.c +++ b/gcc/targhooks.c @@ -1965,4 +1965,31 @@ default_optab_supported_p (int, machine_mode, machine_mode, optimization_type) return true; } +void +default_expand_divmod_libfunc (bool unsignedp, machine_mode mode, + rtx op0, rtx op1, + rtx *quot_p, rtx *rem_p) functions need a comment. ISTR it was suggested that ARM change to libgcc2.c__udivmoddi4 style? In that case we could avoid the target hook. + /* If target overrides expand_divmod_libfunc hook + then perform divmod by generating call to the target-specifc divmod libfunc. */ + if (targetm.expand_divmod_libfunc != default_expand_divmod_libfunc) + return true; + + /* Fall back to using libgcc2.c:__udivmoddi4. */ + return (mode == DImode && unsignedp); I don't understand this - we know optab_libfunc returns non-NULL for 'mode' but still restrict this to DImode && unsigned? Also if targetm.expand_divmod_libfunc is not the default we expect the target to handle all modes? That said - I expected the above piece to be simply a 'return true;' ;) Usually we use some can_expand_XXX helper in optabs.c to query if the target supports a specific operation (for example SImode divmod would use DImode divmod by means of widening operands - for the unsigned case of course). + /* Disable the transform if either is a constant, since division-by-constant + may have specialized expansion. */ + if (TREE_CONSTANT (op1) || TREE_CONSTANT (op2)) + return false; please use CONSTANT_CLASS_P (op1) || CONSTANT_CLASS_P (op2) + if (TYPE_OVERFLOW_TRAPS (type)) + return false; why's that? Generally please first test cheap things (trapping, constant-ness) before checking expensive stuff (target_supports_divmod_p). +static bool +convert_to_divmod (gassign *stmt) +{ + if (!divmod_candidate_p (stmt)) + return false; + + tree op1 = gimple_assign_rhs1 (stmt); + tree op2 = gimple_assign_rhs2 (stmt); + + vec<gimple *> stmts = vNULL; use an auto_vec <gimple *> - you currently leak it in at least one place. + if (maybe_clean_or_replace_eh_stmt (use_stmt, use_stmt)) + cfg_changed = true; note that this suggests you should check whether any of the stmts may throw internally as you don't update / transfer EH info correctly. So for 'stmt' and all 'use_stmt' check stmt_can_throw_internal and if so do not add it to the list of stmts to modify. Btw, I think you should not add 'stmt' immediately but when iterating over all uses also gather uses in TRUNC_MOD_EXPR. Otherwise looks ok. Thanks, Richard. > Thanks, > Prathamesh