Fix subreg fallout in simplify_unary_operation()

Geoff Keating geoffk@geoffk.org
Thu Jan 8 23:02:00 GMT 2004


Richard Sandiford <rsandifo@redhat.com> writes:

> This patch fixes what appears to be a latent bug in
> simplify_unary_operation().  It was exposed by the
> recent simplify_immed_subreg patch and is causing
> mips64vrel-elf builds of newlib to fail.
> 
> A reduced test case is:
> 
>     unsigned long long foo ()
>     {
>       union { double d; unsigned int i[2]; } u;
>       u.d = 1.0;
>       return u.i[0];
>     }
> 
> compiled with -meb -mips16 -mabi=o64.
> 
> We enter combine with:
> 
>     (set (subreg:DF (reg:DI u) 0)
>          (mem:DF (symbol_ref "$LC0")))
> 
>     (set (reg:DI r1)
>          (lshiftrt:DI (reg:DI u)
>                       (const_int 32)))
> 
> and combine.c:subst() is called on to simplify:
> 
>     (lshiftrt:DI (mem:DI (symbol_ref "$LC0"))
>                  (const_int 32))
> 
> Specifically, the loop is:
> 
>   /* Try to simplify X.  If the simplification changed the code, it is likely
>      that further simplification will help, so loop, but limit the number
>      of repetitions that will be performed.  */
> 
>   for (i = 0; i < 4; i++)
>     {
>       /* If X is sufficiently simple, don't bother trying to do anything
>          with it.  */
>       if (code != CONST_INT && code != REG && code != CLOBBER)
>         x = combine_simplify_rtx (x, op0_mode, i == 3, in_dest);
> 
>       if (GET_CODE (x) == code)
>         break;
> 
>       code = GET_CODE (x);
> 
>       /* We no longer know the original mode of operand 0 since we
>          have changed the form of X)  */
>       op0_mode = VOIDmode;
>     }
> 
> First time around, the expression is correctly simplified to:
> 
>     (zero_extend:DI (mem:SI (symbol_ref "$LC0")))
> 
> and the next iteration tries to simplify it again.  This time, because
> of the code at the end of the loop, op0_mode will be VOIDmode.  This
> results in a call to:
> 
>     simplify_unary_operation (ZERO_EXTEND, DImode,
>                               (mem:SI (symbol_ref "$LC0")), VOIDmode)
> 
> and the first thing it does is:
> 
>     rtx trueop = avoid_constant_pool_reference (op);
> 
> Here, OP is an SImode reference to a DFmode constant pool entry.  Before
> the simplify_immed_subreg change, the subreg code couldn't simplify this
> reference, but now it can.  trueop therefore ends up being a CONST_INT
> for the high 32 bits of the double constant.  This leads to an abort in
> the code that handles CONST_INTs:
> 
>   if (GET_CODE (trueop) == CONST_INT
>       && width <= HOST_BITS_PER_WIDE_INT && width > 0)
>     {
>       [...]
>         case ZERO_EXTEND:
>           /* When zero-extending a CONST_INT, we need to know its
>              original mode.  */
>           if (op_mode == VOIDmode)
>             abort ();
> 
> Perhaps it could be argued that the caller shouldn't pass VOIDmode here.
> But the function _was_ given a correct mode by means of the original
> MEM operand.  The interface to the function seems more robust if it
> falls back on GET_MODE (op) when op0_mode is VOIDmode.  That's what
> the caller would have to do anyway.
> 
> Oh well.  Long story for a short patch...
> 
> Tested on mips64vrel-elf, fixes the build failure.  Since this was my
> first build this year, I don't have fully-up-to-date test results to
> compare against, but the failures are in line with what I was seeing
> a few weeks ago.
> 
> OK to install?

No, I don't like this; it complicates the interface of
simplify_unary_operation.  Instead, have combine_simplify_rtx
compute the mode if it gets passed VOIDmode and is going to call
simplify_unary_operation.

> 	* simplify-rtx.c (simplify_unary_operation): Use OP's mode instead
> 	of OP_MODE when the latter is VOIDmode.
> 
> Index: simplify-rtx.c
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/simplify-rtx.c,v
> retrieving revision 1.168
> diff -u -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.168 simplify-rtx.c
> --- simplify-rtx.c	6 Jan 2004 22:51:00 -0000	1.168
> +++ simplify-rtx.c	7 Jan 2004 22:26:54 -0000
> @@ -375,6 +375,8 @@ simplify_unary_operation (enum rtx_code 
>    unsigned int width = GET_MODE_BITSIZE (mode);
>    rtx trueop = avoid_constant_pool_reference (op);
>  
> +  if (op_mode == VOIDmode)
> +    op_mode = GET_MODE (op);
>    if (code == VEC_DUPLICATE)
>      {
>        if (!VECTOR_MODE_P (mode))
> 

-- 
- Geoffrey Keating <geoffk@geoffk.org>



More information about the Gcc-patches mailing list