Patchwork [wide-int] Another tweak to convert_modes

login
register
mail settings
Submitter Richard Sandiford
Date Nov. 2, 2013, 10:48 a.m.
Message ID <8738nfkrkb.fsf@talisman.default>
Download mbox | patch
Permalink /patch/287970/
State New
Headers show

Comments

Richard Sandiford - Nov. 2, 2013, 10:48 a.m.
It turns out that when doing a vector shift by 2, the optab routine
passes (const_int 2) to convert_modes with oldmode set to the mode
of the shift (e.g. something like V8HI).  When the target mode is a
real integer mode like SImode, mainline just ignores that oldmode
and returns a (const_int 2) regardless, but wide-int doesn't.

Saying that (const_int 2) has a vector mode is almost certainly a bug
that ought to be trapped by an assert, but we're not trying to fight
that battle here.  The current code:

  if (CONST_SCALAR_INT_P (x) 
      && GET_MODE_CLASS (mode) == MODE_INT
      && (oldmode == VOIDmode || GET_MODE_CLASS (oldmode) == MODE_INT))

is already coping with bogus oldmodes, just not in the way that
other routines seem to expect.

Tested on powerpc64-linux-gnu and x86_64-linux-gnu.  This fixed several
testsuite changes on the ARM targets.  OK to install?

Thanks,
Richard
Kenneth Zadeck - Nov. 2, 2013, 1:50 p.m.
On 11/02/2013 06:48 AM, Richard Sandiford wrote:
> It turns out that when doing a vector shift by 2, the optab routine
> passes (const_int 2) to convert_modes with oldmode set to the mode
> of the shift (e.g. something like V8HI).  When the target mode is a
> real integer mode like SImode, mainline just ignores that oldmode
> and returns a (const_int 2) regardless, but wide-int doesn't.
>
> Saying that (const_int 2) has a vector mode is almost certainly a bug
> that ought to be trapped by an assert, but we're not trying to fight
> that battle here.  The current code:
>
>    if (CONST_SCALAR_INT_P (x)
>        && GET_MODE_CLASS (mode) == MODE_INT
>        && (oldmode == VOIDmode || GET_MODE_CLASS (oldmode) == MODE_INT))
>
> is already coping with bogus oldmodes, just not in the way that
> other routines seem to expect.
>
> Tested on powerpc64-linux-gnu and x86_64-linux-gnu.  This fixed several
> testsuite changes on the ARM targets.  OK to install?
>
> Thanks,
> Richard
or we can fix it on trunk and pull the patch up.    If you as an 
(former) rtl maintainer want to go in this direction, then fine.
>
> Index: gcc/expr.c
> ===================================================================
> --- gcc/expr.c	2013-11-02 10:34:44.083635650 +0000
> +++ gcc/expr.c	2013-11-02 10:42:55.179233840 +0000
> @@ -712,13 +712,12 @@ convert_modes (enum machine_mode mode, e
>       return x;
>   
>     if (CONST_SCALAR_INT_P (x)
> -      && GET_MODE_CLASS (mode) == MODE_INT
> -      && (oldmode == VOIDmode || GET_MODE_CLASS (oldmode) == MODE_INT))
> +      && GET_MODE_CLASS (mode) == MODE_INT)
>       {
>         /* If the caller did not tell us the old mode, then there is
>   	 not much to do with respect to canonization.  We have to assume
>   	 that all the bits are significant.  */
> -      if (oldmode == VOIDmode)
> +      if (GET_MODE_CLASS (oldmode) != MODE_INT)
>   	oldmode = MAX_MODE_INT;
>         wide_int w = wide_int::from (std::make_pair (x, oldmode),
>   				   GET_MODE_PRECISION (mode),

Patch

Index: gcc/expr.c
===================================================================
--- gcc/expr.c	2013-11-02 10:34:44.083635650 +0000
+++ gcc/expr.c	2013-11-02 10:42:55.179233840 +0000
@@ -712,13 +712,12 @@  convert_modes (enum machine_mode mode, e
     return x;
 
   if (CONST_SCALAR_INT_P (x) 
-      && GET_MODE_CLASS (mode) == MODE_INT
-      && (oldmode == VOIDmode || GET_MODE_CLASS (oldmode) == MODE_INT))
+      && GET_MODE_CLASS (mode) == MODE_INT)
     {
       /* If the caller did not tell us the old mode, then there is
 	 not much to do with respect to canonization.  We have to assume
 	 that all the bits are significant.  */
-      if (oldmode == VOIDmode)
+      if (GET_MODE_CLASS (oldmode) != MODE_INT)
 	oldmode = MAX_MODE_INT;
       wide_int w = wide_int::from (std::make_pair (x, oldmode),
 				   GET_MODE_PRECISION (mode),