On Tue, 1 Dec 2020, Jakub Jelinek wrote: > Hi! > > Jeff has reported that my earlier patch broke rl78-elf, e.g. with > unsigned short foo (unsigned short x) { return x % 7; } > when compiled with -O2 -mg14. The problem is that rl78 is a BITS_PER_WORD > == 8 target which doesn't have 8-bit modulo or divmod optab, but has instead > 16-bit divmod, so my patch attempted to optimize it, then called > expand_divmod to do 8-bit modulo and that in turn tried to do 16-bit modulo > again. > > The following patch fixes it in two ways. > One is to not perform the optimization when we have {u,s}divmod_optab > handler for the double-word mode, in that case it is IMHO better to just > do whatever we used to do before. This alone should fix the infinite > recursion. But I'd be afraid some other target might have similar problem > and might not have a divmod pattern, but only say a library call. > So the patch also introduces a methods argument to expand_divmod such that > normally we allow everything that was allowed before (using libcalls and > widening), but when called from these expand_doubleword*mod routines we > restrict it to no widening and no libcalls. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK. Thanks, Richard. > 2020-12-01 Jakub Jelinek <ja...@redhat.com> > > * expmed.h (expand_divmod): Only declare if GCC_OPTABS_H is defined. > Add enum optabs_method argument defaulted to OPTAB_LIB_WIDEN. > * expmed.c: Include expmed.h after optabs.h. > (expand_divmod): Add methods argument, if it is not OPTAB_{,LIB_}WIDEN, > don't choose a wider mode, and pass it to other calls instead of > hardcoded OPTAB_LIB_WIDEN. Avoid emitting libcalls if not > OPTAB_LIB or OPTAB_LIB_WIDEN. > * optabs.c: Include expmed.h after optabs.h. > (expand_doubleword_mod, expand_doubleword_divmod): Pass OPTAB_DIRECT > as last argument to expand_divmod. > (expand_binop): Punt if {s,u}divmod_optab has handler for double-word > int_mode. > * expr.c: Include expmed.h after optabs.h. > * explow.c: Include expmed.h after optabs.h. > > --- gcc/expmed.h.jj 2020-01-12 11:54:36.565411115 +0100 > +++ gcc/expmed.h 2020-12-01 19:25:55.117128688 +0100 > @@ -716,8 +716,10 @@ extern rtx expand_variable_shift (enum t > rtx, tree, rtx, int); > extern rtx expand_shift (enum tree_code, machine_mode, rtx, poly_int64, rtx, > int); > +#ifdef GCC_OPTABS_H > extern rtx expand_divmod (int, enum tree_code, machine_mode, rtx, rtx, > - rtx, int); > + rtx, int, enum optab_methods = OPTAB_LIB_WIDEN); > +#endif > #endif > > extern void store_bit_field (rtx, poly_uint64, poly_uint64, > --- gcc/expmed.c.jj 2020-08-10 10:44:20.659560695 +0200 > +++ gcc/expmed.c 2020-12-01 19:30:40.993941266 +0100 > @@ -31,8 +31,8 @@ along with GCC; see the file COPYING3. > #include "predict.h" > #include "memmodel.h" > #include "tm_p.h" > -#include "expmed.h" > #include "optabs.h" > +#include "expmed.h" > #include "regs.h" > #include "emit-rtl.h" > #include "diagnostic-core.h" > @@ -4193,7 +4193,8 @@ expand_sdiv_pow2 (scalar_int_mode mode, > > rtx > expand_divmod (int rem_flag, enum tree_code code, machine_mode mode, > - rtx op0, rtx op1, rtx target, int unsignedp) > + rtx op0, rtx op1, rtx target, int unsignedp, > + enum optab_methods methods) > { > machine_mode compute_mode; > rtx tquotient; > @@ -4299,17 +4300,22 @@ expand_divmod (int rem_flag, enum tree_c > optab2 = (op1_is_pow2 ? optab1 > : (unsignedp ? udivmod_optab : sdivmod_optab)); > > - FOR_EACH_MODE_FROM (compute_mode, mode) > - if (optab_handler (optab1, compute_mode) != CODE_FOR_nothing > - || optab_handler (optab2, compute_mode) != CODE_FOR_nothing) > - break; > - > - if (compute_mode == VOIDmode) > - FOR_EACH_MODE_FROM (compute_mode, mode) > - if (optab_libfunc (optab1, compute_mode) > - || optab_libfunc (optab2, compute_mode)) > + if (methods == OPTAB_WIDEN || methods == OPTAB_LIB_WIDEN) > + { > + FOR_EACH_MODE_FROM (compute_mode, mode) > + if (optab_handler (optab1, compute_mode) != CODE_FOR_nothing > + || optab_handler (optab2, compute_mode) != CODE_FOR_nothing) > break; > > + if (compute_mode == VOIDmode && methods == OPTAB_LIB_WIDEN) > + FOR_EACH_MODE_FROM (compute_mode, mode) > + if (optab_libfunc (optab1, compute_mode) > + || optab_libfunc (optab2, compute_mode)) > + break; > + } > + else > + compute_mode = mode; > + > /* If we still couldn't find a mode, use MODE, but expand_binop will > probably die. */ > if (compute_mode == VOIDmode) > @@ -4412,8 +4418,7 @@ expand_divmod (int rem_flag, enum tree_c > remainder > = expand_binop (int_mode, and_optab, op0, > gen_int_mode (mask, int_mode), > - remainder, 1, > - OPTAB_LIB_WIDEN); > + remainder, 1, methods); > if (remainder) > return gen_lowpart (mode, remainder); > } > @@ -4721,7 +4726,7 @@ expand_divmod (int rem_flag, enum tree_c > remainder = expand_binop > (int_mode, and_optab, op0, > gen_int_mode (mask, int_mode), > - remainder, 0, OPTAB_LIB_WIDEN); > + remainder, 0, methods); > if (remainder) > return gen_lowpart (mode, remainder); > } > @@ -4846,7 +4851,7 @@ expand_divmod (int rem_flag, enum tree_c > do_cmp_and_jump (op1, const0_rtx, LT, compute_mode, label2); > do_cmp_and_jump (adjusted_op0, const0_rtx, LT, compute_mode, label1); > tem = expand_binop (compute_mode, sdiv_optab, adjusted_op0, op1, > - quotient, 0, OPTAB_LIB_WIDEN); > + quotient, 0, methods); > if (tem != quotient) > emit_move_insn (quotient, tem); > emit_jump_insn (targetm.gen_jump (label5)); > @@ -4858,7 +4863,7 @@ expand_divmod (int rem_flag, enum tree_c > emit_label (label2); > do_cmp_and_jump (adjusted_op0, const0_rtx, GT, compute_mode, label3); > tem = expand_binop (compute_mode, sdiv_optab, adjusted_op0, op1, > - quotient, 0, OPTAB_LIB_WIDEN); > + quotient, 0, methods); > if (tem != quotient) > emit_move_insn (quotient, tem); > emit_jump_insn (targetm.gen_jump (label5)); > @@ -4867,7 +4872,7 @@ expand_divmod (int rem_flag, enum tree_c > expand_dec (adjusted_op0, const1_rtx); > emit_label (label4); > tem = expand_binop (compute_mode, sdiv_optab, adjusted_op0, op1, > - quotient, 0, OPTAB_LIB_WIDEN); > + quotient, 0, methods); > if (tem != quotient) > emit_move_insn (quotient, tem); > expand_dec (quotient, const1_rtx); > @@ -4892,7 +4897,7 @@ expand_divmod (int rem_flag, enum tree_c > floor_log2 (d), tquotient, 1); > t2 = expand_binop (int_mode, and_optab, op0, > gen_int_mode (d - 1, int_mode), > - NULL_RTX, 1, OPTAB_LIB_WIDEN); > + NULL_RTX, 1, methods); > t3 = gen_reg_rtx (int_mode); > t3 = emit_store_flag (t3, NE, t2, const0_rtx, int_mode, 1, 1); > if (t3 == 0) > @@ -4963,7 +4968,7 @@ expand_divmod (int rem_flag, enum tree_c > emit_label (label1); > expand_dec (adjusted_op0, const1_rtx); > tem = expand_binop (compute_mode, udiv_optab, adjusted_op0, op1, > - quotient, 1, OPTAB_LIB_WIDEN); > + quotient, 1, methods); > if (tem != quotient) > emit_move_insn (quotient, tem); > expand_inc (quotient, const1_rtx); > @@ -4987,7 +4992,7 @@ expand_divmod (int rem_flag, enum tree_c > floor_log2 (d), tquotient, 0); > t2 = expand_binop (compute_mode, and_optab, op0, > gen_int_mode (d - 1, compute_mode), > - NULL_RTX, 1, OPTAB_LIB_WIDEN); > + NULL_RTX, 1, methods); > t3 = gen_reg_rtx (compute_mode); > t3 = emit_store_flag (t3, NE, t2, const0_rtx, > compute_mode, 1, 1); > @@ -5063,7 +5068,7 @@ expand_divmod (int rem_flag, enum tree_c > do_cmp_and_jump (adjusted_op0, const0_rtx, GT, > compute_mode, label1); > tem = expand_binop (compute_mode, sdiv_optab, adjusted_op0, op1, > - quotient, 0, OPTAB_LIB_WIDEN); > + quotient, 0, methods); > if (tem != quotient) > emit_move_insn (quotient, tem); > emit_jump_insn (targetm.gen_jump (label5)); > @@ -5076,7 +5081,7 @@ expand_divmod (int rem_flag, enum tree_c > do_cmp_and_jump (adjusted_op0, const0_rtx, LT, > compute_mode, label3); > tem = expand_binop (compute_mode, sdiv_optab, adjusted_op0, op1, > - quotient, 0, OPTAB_LIB_WIDEN); > + quotient, 0, methods); > if (tem != quotient) > emit_move_insn (quotient, tem); > emit_jump_insn (targetm.gen_jump (label5)); > @@ -5085,7 +5090,7 @@ expand_divmod (int rem_flag, enum tree_c > expand_inc (adjusted_op0, const1_rtx); > emit_label (label4); > tem = expand_binop (compute_mode, sdiv_optab, adjusted_op0, op1, > - quotient, 0, OPTAB_LIB_WIDEN); > + quotient, 0, methods); > if (tem != quotient) > emit_move_insn (quotient, tem); > expand_inc (quotient, const1_rtx); > @@ -5133,10 +5138,10 @@ expand_divmod (int rem_flag, enum tree_c > { > rtx tem; > quotient = expand_binop (int_mode, udiv_optab, op0, op1, > - quotient, 1, OPTAB_LIB_WIDEN); > + quotient, 1, methods); > tem = expand_mult (int_mode, quotient, op1, NULL_RTX, 1); > remainder = expand_binop (int_mode, sub_optab, op0, tem, > - remainder, 1, OPTAB_LIB_WIDEN); > + remainder, 1, methods); > } > tem = plus_constant (int_mode, op1, -1); > tem = expand_shift (RSHIFT_EXPR, int_mode, tem, 1, NULL_RTX, 1); > @@ -5158,10 +5163,10 @@ expand_divmod (int rem_flag, enum tree_c > { > rtx tem; > quotient = expand_binop (int_mode, sdiv_optab, op0, op1, > - quotient, 0, OPTAB_LIB_WIDEN); > + quotient, 0, methods); > tem = expand_mult (int_mode, quotient, op1, NULL_RTX, 0); > remainder = expand_binop (int_mode, sub_optab, op0, tem, > - remainder, 0, OPTAB_LIB_WIDEN); > + remainder, 0, methods); > } > abs_rem = expand_abs (int_mode, remainder, NULL_RTX, 1, 0); > abs_op1 = expand_abs (int_mode, op1, NULL_RTX, 1, 0); > @@ -5258,7 +5263,7 @@ expand_divmod (int rem_flag, enum tree_c > quotient = sign_expand_binop (compute_mode, > udiv_optab, sdiv_optab, > op0, op1, target, > - unsignedp, OPTAB_LIB_WIDEN); > + unsignedp, methods); > } > } > } > @@ -5273,10 +5278,11 @@ expand_divmod (int rem_flag, enum tree_c > /* No divide instruction either. Use library for remainder. */ > remainder = sign_expand_binop (compute_mode, umod_optab, smod_optab, > op0, op1, target, > - unsignedp, OPTAB_LIB_WIDEN); > + unsignedp, methods); > /* No remainder function. Try a quotient-and-remainder > function, keeping the remainder. */ > - if (!remainder) > + if (!remainder > + && (methods == OPTAB_LIB || methods == OPTAB_LIB_WIDEN)) > { > remainder = gen_reg_rtx (compute_mode); > if (!expand_twoval_binop_libfunc > @@ -5294,10 +5300,14 @@ expand_divmod (int rem_flag, enum tree_c > NULL_RTX, unsignedp); > remainder = expand_binop (compute_mode, sub_optab, op0, > remainder, target, unsignedp, > - OPTAB_LIB_WIDEN); > + methods); > } > } > > + if (methods != OPTAB_LIB_WIDEN > + && (rem_flag ? remainder : quotient) == NULL_RTX) > + return NULL_RTX; > + > return gen_lowpart (mode, rem_flag ? remainder : quotient); > } > > --- gcc/optabs.c.jj 2020-12-01 19:02:52.251555958 +0100 > +++ gcc/optabs.c 2020-12-01 19:26:25.069794805 +0100 > @@ -28,8 +28,8 @@ along with GCC; see the file COPYING3. > #include "memmodel.h" > #include "predict.h" > #include "tm_p.h" > -#include "expmed.h" > #include "optabs.h" > +#include "expmed.h" > #include "emit-rtl.h" > #include "recog.h" > #include "diagnostic-core.h" > @@ -1082,7 +1082,7 @@ expand_doubleword_mod (machine_mode mode > } > } > rtx remainder = expand_divmod (1, TRUNC_MOD_EXPR, word_mode, sum, op1, > - NULL_RTX, 1); > + NULL_RTX, 1, OPTAB_DIRECT); > if (remainder == NULL_RTX) > return NULL_RTX; > > @@ -1180,7 +1180,7 @@ expand_doubleword_divmod (machine_mode m > if (op11 != const1_rtx) > { > rtx rem2 = expand_divmod (1, TRUNC_MOD_EXPR, mode, quot1, op11, > - NULL_RTX, unsignedp); > + NULL_RTX, unsignedp, OPTAB_DIRECT); > if (rem2 == NULL_RTX) > return NULL_RTX; > > @@ -1195,7 +1195,7 @@ expand_doubleword_divmod (machine_mode m > return NULL_RTX; > > rtx quot2 = expand_divmod (0, TRUNC_DIV_EXPR, mode, quot1, op11, > - NULL_RTX, unsignedp); > + NULL_RTX, unsignedp, OPTAB_DIRECT); > if (quot2 == NULL_RTX) > return NULL_RTX; > > @@ -2100,6 +2100,9 @@ expand_binop (machine_mode mode, optab b > && CONST_INT_P (op1) > && is_int_mode (mode, &int_mode) > && GET_MODE_SIZE (int_mode) == 2 * UNITS_PER_WORD > + && optab_handler ((binoptab == umod_optab || binoptab == udiv_optab) > + ? udivmod_optab : sdivmod_optab, > + int_mode) == CODE_FOR_nothing > && optab_handler (and_optab, word_mode) != CODE_FOR_nothing > && optab_handler (add_optab, word_mode) != CODE_FOR_nothing > && optimize_insn_for_speed_p ()) > --- gcc/expr.c.jj 2020-11-22 19:11:44.088588689 +0100 > +++ gcc/expr.c 2020-12-01 19:26:45.521566815 +0100 > @@ -29,8 +29,8 @@ along with GCC; see the file COPYING3. > #include "memmodel.h" > #include "tm_p.h" > #include "ssa.h" > -#include "expmed.h" > #include "optabs.h" > +#include "expmed.h" > #include "regs.h" > #include "emit-rtl.h" > #include "recog.h" > --- gcc/explow.c.jj 2020-11-26 01:14:47.500082297 +0100 > +++ gcc/explow.c 2020-12-01 19:25:19.944520777 +0100 > @@ -27,9 +27,9 @@ along with GCC; see the file COPYING3. > #include "tree.h" > #include "memmodel.h" > #include "tm_p.h" > +#include "optabs.h" > #include "expmed.h" > #include "profile-count.h" > -#include "optabs.h" > #include "emit-rtl.h" > #include "recog.h" > #include "diagnostic-core.h" > > Jakub > > -- Richard Biener <rguent...@suse.de> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)