On Wed, 22 Nov 2017, Jakub Jelinek wrote: > Hi! > > On these two testcases, we end up expanding MULT_EXPR where both arguments > end up being VOIDmode. For smul_optab that isn't a problem, we have > the mode next to it, but in some cases we want to use {u,s}mul_widen_optab > which is a conversion optab which needs two modes expand_binop is called > just with one mode, the result mode, so the other mode is guessed from > the operands and if both are VOIDmode, then a fallback is chosen to use > return mode. The new find_widening* changes ICE on that though, previously > we'd just do something. > > In any case, I think we need to make sure this doesn't happen while we > still know both modes for the {u,s}mul_widen_optab. Bootstrapped/regtested > on x86_64-linux and i686-linux, ok for trunk? > > Perhaps additionally we could have somewhere a case which for both arguments > constant (unlikely case, as the gimple optimizers should usually optimize > that out) and selected optabs for which we know the corresponding RTL code > we could use simplify_const_binary_operation and see if it optimizes into a > constant and just return that. Though, these functions are large and it > is still possible a constant could be uncovered later, so I think we want > this patch even if we do something like that.
How much churn would it be to pass down a mode alongside the operands in expand_binop? Can't find it right now but didn't we introduce some rtx_with_mode pair-like stuff somewhen? Anyway, the patch looks ok to me but generally we of course want to pass down modes from where we still know them. Richard. > 2017-11-22 Jakub Jelinek <ja...@redhat.com> > > PR middle-end/82875 > * optabs.c (expand_doubleword_mult, expand_binop): Before calling > expand_binop with *mul_widen_optab, make sure at least one of the > operands doesn't have VOIDmode. > > * gcc.dg/pr82875.c: New test. > * gcc.c-torture/compile/pr82875.c: New test. > > --- gcc/optabs.c.jj 2017-11-09 20:23:51.000000000 +0100 > +++ gcc/optabs.c 2017-11-21 17:40:13.318673366 +0100 > @@ -861,6 +861,11 @@ expand_doubleword_mult (machine_mode mod > if (target && !REG_P (target)) > target = NULL_RTX; > > + /* *_widen_optab needs to determine operand mode, make sure at least > + one operand has non-VOID mode. */ > + if (GET_MODE (op0_low) == VOIDmode && GET_MODE (op1_low) == VOIDmode) > + op0_low = force_reg (word_mode, op0_low); > + > if (umulp) > product = expand_binop (mode, umul_widen_optab, op0_low, op1_low, > target, 1, OPTAB_DIRECT); > @@ -1199,6 +1204,10 @@ expand_binop (machine_mode mode, optab b > : smul_widen_optab), > wider_mode, mode) != CODE_FOR_nothing)) > { > + /* *_widen_optab needs to determine operand mode, make sure at least > + one operand has non-VOID mode. */ > + if (GET_MODE (op0) == VOIDmode && GET_MODE (op1) == VOIDmode) > + op0 = force_reg (mode, op0); > temp = expand_binop (wider_mode, > unsignedp ? umul_widen_optab : smul_widen_optab, > op0, op1, NULL_RTX, unsignedp, OPTAB_DIRECT); > --- gcc/testsuite/gcc.dg/pr82875.c.jj 2017-11-21 17:50:16.022274628 +0100 > +++ gcc/testsuite/gcc.dg/pr82875.c 2017-11-21 17:49:46.000000000 +0100 > @@ -0,0 +1,11 @@ > +/* PR middle-end/82875 */ > +/* { dg-do compile } */ > +/* { dg-options "-ftree-ter" } */ > + > +const int a = 100; > + > +void > +foo (void) > +{ > + int c[a]; > +} > --- gcc/testsuite/gcc.c-torture/compile/pr82875.c.jj 2017-11-21 > 17:48:46.409374708 +0100 > +++ gcc/testsuite/gcc.c-torture/compile/pr82875.c 2017-11-21 > 17:48:25.000000000 +0100 > @@ -0,0 +1,24 @@ > +/* PR middle-end/82875 */ > + > +signed char a; > +unsigned b; > +long c, d; > +long long e; > + > +void > +foo (void) > +{ > + short f = a = 6; > + while (0) > + while (a <= 7) > + { > + for (;;) > + ; > + lab: > + while (c <= 73) > + ; > + e = 20; > + d ? (a %= c) * (e *= a ? f : b) : 0; > + } > + goto lab; > +} > > Jakub > > -- Richard Biener <rguent...@suse.de> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)