Re: [PATCH] Fix mult expansion ICE (PR middle-end/82875)

[Richard Biener <rguenther@suse.de>]

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  <jakub@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 <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)