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)

Reply via email to