expr: Move reduce_bit_field target mode check [PR96151]

In some cases, expand_expr_real_2 prefers to use the mode of the
caller-suggested target instead of the mode of the expression when
passing values to reduce_to_bit_field_precision.  E.g.:

      else if (target == 0)
        op0 = convert_to_mode (mode, op0,
                               TYPE_UNSIGNED (TREE_TYPE
                                              (treeop0)));
      else
        {
          convert_move (target, op0,
                        TYPE_UNSIGNED (TREE_TYPE (treeop0)));
          op0 = target;
        }

where “op0” might not have “mode” for the “else” branch,
but does for all the others.

reduce_to_bit_field_precision discards the suggested target if it
has the wrong mode.  This patch moves that to expand_expr_real_2
instead (conditional on reduce_bit_field).

Sorry for the breakage.  This is what I'd done in the original
version of the patch, after checking all uses of REDUCE_BIT_FIELD.
I then forgot why it was necessary and tried to “simplify” the
patch for backports.

Tested on arm-linux-gnueabihf, where it restores bootstrap.
Other tests still ongoing.  OK to install if it passes?

Richard


gcc/
	PR middle-end/96151
	* expr.c (expand_expr_real_2): When reducing bit fields,
	clear the target if it has a different mode from the expression.
	(reduce_to_bit_field_precision): Don't do that here.  Instead
	assert that the target already has the correct mode.
---
 gcc/expr.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

On Fri, 2020-07-10 at 16:40 +0100, Richard Sandiford wrote:
> In some cases, expand_expr_real_2 prefers to use the mode of the
> caller-suggested target instead of the mode of the expression when
> passing values to reduce_to_bit_field_precision.  E.g.:
> 
>       else if (target == 0)
>         op0 = convert_to_mode (mode, op0,
>                                TYPE_UNSIGNED (TREE_TYPE
>                                               (treeop0)));
>       else
>         {
>           convert_move (target, op0,
>                         TYPE_UNSIGNED (TREE_TYPE (treeop0)));
>           op0 = target;
>         }
> 
> where “op0” might not have “mode” for the “else” branch,
> but does for all the others.
> 
> reduce_to_bit_field_precision discards the suggested target if it
> has the wrong mode.  This patch moves that to expand_expr_real_2
> instead (conditional on reduce_bit_field).
> 
> Sorry for the breakage.  This is what I'd done in the original
> version of the patch, after checking all uses of REDUCE_BIT_FIELD.
> I then forgot why it was necessary and tried to “simplify” the
> patch for backports.
> 
> Tested on arm-linux-gnueabihf, where it restores bootstrap.
> Other tests still ongoing.  OK to install if it passes?
> 
> Richard
> 
> 
> gcc/
> 	PR middle-end/96151
> 	* expr.c (expand_expr_real_2): When reducing bit fields,
> 	clear the target if it has a different mode from the expression.
> 	(reduce_to_bit_field_precision): Don't do that here.  Instead
> 	assert that the target already has the correct mode.
OK.  Note I think this is also affecting csky and mips too.

Jeff
>

This fixed the ICE I was seeing, thanks.

Aaron Sawdey, Ph.D. sawdey@linux.ibm.com
IBM Linux on POWER Toolchain
 

> On Jul 10, 2020, at 10:40 AM, Richard Sandiford <richard.sandiford@arm.com> wrote:
> 
> In some cases, expand_expr_real_2 prefers to use the mode of the
> caller-suggested target instead of the mode of the expression when
> passing values to reduce_to_bit_field_precision.  E.g.:
> 
>      else if (target == 0)
>        op0 = convert_to_mode (mode, op0,
>                               TYPE_UNSIGNED (TREE_TYPE
>                                              (treeop0)));
>      else
>        {
>          convert_move (target, op0,
>                        TYPE_UNSIGNED (TREE_TYPE (treeop0)));
>          op0 = target;
>        }
> 
> where “op0” might not have “mode” for the “else” branch,
> but does for all the others.
> 
> reduce_to_bit_field_precision discards the suggested target if it
> has the wrong mode.  This patch moves that to expand_expr_real_2
> instead (conditional on reduce_bit_field).
> 
> Sorry for the breakage.  This is what I'd done in the original
> version of the patch, after checking all uses of REDUCE_BIT_FIELD.
> I then forgot why it was necessary and tried to “simplify” the
> patch for backports.
> 
> Tested on arm-linux-gnueabihf, where it restores bootstrap.
> Other tests still ongoing.  OK to install if it passes?
> 
> Richard
> 
> 
> gcc/
> 	PR middle-end/96151
> 	* expr.c (expand_expr_real_2): When reducing bit fields,
> 	clear the target if it has a different mode from the expression.
> 	(reduce_to_bit_field_precision): Don't do that here.  Instead
> 	assert that the target already has the correct mode.
> ---
> gcc/expr.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/gcc/expr.c b/gcc/expr.c
> index 715edae819a..c7c3e9fd655 100644
> --- a/gcc/expr.c
> +++ b/gcc/expr.c
> @@ -8664,7 +8664,9 @@ expand_expr_real_2 (sepops ops, rtx target, machine_mode tmode,
>   reduce_bit_field = (INTEGRAL_TYPE_P (type)
> 		      && !type_has_mode_precision_p (type));
> 
> -  if (reduce_bit_field && modifier == EXPAND_STACK_PARM)
> +  if (reduce_bit_field
> +      && (modifier == EXPAND_STACK_PARM
> +	  || (target && GET_MODE (target) != mode)))
>     target = 0;
> 
>   /* Use subtarget as the target for operand 0 of a binary operation.  */
> @@ -11527,9 +11529,8 @@ reduce_to_bit_field_precision (rtx exp, rtx target, tree type)
> {
>   scalar_int_mode mode = SCALAR_INT_TYPE_MODE (type);
>   HOST_WIDE_INT prec = TYPE_PRECISION (type);
> -  gcc_assert (GET_MODE (exp) == VOIDmode || GET_MODE (exp) == mode);
> -  if (target && GET_MODE (target) != mode)
> -    target = 0;
> +  gcc_assert ((GET_MODE (exp) == VOIDmode || GET_MODE (exp) == mode)
> +	      && (!target || GET_MODE (target) == mode));
> 
>   /* For constant values, reduce using wide_int_to_tree. */
>   if (poly_int_rtx_p (exp))