RE: [PATCH][1/N] Change GET_MODE_INNER to always return a non-void mode

RE: [PATCH][1/N] Change GET_MODE_INNER to always return a non-void mode

[David Sherwood]

Hi,

Sorry, I forgot to mention I tested this on:

aarch64 and aarch64_be - no regressions in gcc testsuite
x86_64 - bootstrap build, no testsuite regressions
arm-none-eabi - no regressions in gcc testsuite

I will also make sure that I can do cross builds on a variety of different targets.

Regards,
David.

> -----Original Message-----
> From: David Sherwood [mailto:david.sherwood@arm.com]
> Sent: 27 July 2015 11:25
> To: gcc-patches@gcc.gnu.org
> Subject: [PATCH][1/N] Change GET_MODE_INNER to always return a non-void mode
> 
> Hi,
> 
> Part 1 of this change is a clean-up. I have changed calls to GET_MODE_INNER (m)
> so that it returns m in cases where there is no inner mode. This simplifies some
> of the calling code by removing the need to check for VOIDmode and allows
> calling it unconditionally. I also removed element_precision () as it was only
> called in one place and thought it neater to call GET_MODE_PRECISION explicitly.
> 
> Parts 2-4 will include further tidy-ups and optimisations based on [1/N].
> 
> Good to go?
> 
> Regards,
> David Sherwood.
> 
> 2015-07-17  David Sherwood  <david.sherwood@arm.com>
> 
>     gcc/
>         * config/arm/arm.c (neon_element_bits, neon_valid_immediate): Call
>         GET_MODE_INNER unconditionally.
>         * config/spu/spu.c (arith_immediate_p): Likewise.
>         * config/i386/i386.c (ix86_build_signbit_mask): Likewise.  New variable.
>         * expmed.c (synth_mult): Remove check for VOIDmode result from
>         GET_MODE_INNER.
>         (expand_mult_const): Likewise.
>         * fold-const.c (): Replace call to element_precision with call to
>         GET_MODE_PRECISION.
>         * genmodes.c (emit_mode_inner_inline): Replace void_mode->name with
>         m->name.
>         (emit_mode_inner): Likewise.
>         * lto-streamer-out.c (lto_write_mode_table): Update GET_MODE_INNER
>         result check.
>         * machmode.h (GET_MODE_UNIT_SIZE): Simplify.
>         (GET_MODE_UNIT_PRECISION): Likewise.
>         * rtlanal.c (subreg_get_info): Call GET_MODE_INNER unconditionally.
>         * simplify-rtx.c (simplify_immed_subreg): Likewise.
>         * stor-layout.c (bitwise_type_for_mode): Update assert.
>         (element_precision): Remove.

Re: [PATCH][1/N] Change GET_MODE_INNER to always return a non-void mode

[Jeff Law]

On 07/28/2015 03:47 AM, David Sherwood wrote:
> Sorry, the comment should reflect complex types too. How about this?
>
> /* Return the mode of the basic parts of MODE.  For vector modes this is the
>      mode of the vector elements.  For complex modes it is the mode of the real
>      and imaginary parts.  For other modes it is MODE itself.  */
Even better :-)

jeff

Re: [PATCH][1/N] Change GET_MODE_INNER to always return a non-void mode

[Jeff Law]

On 07/28/2015 03:35 AM, David Sherwood wrote:
>>
>> On 07/27/2015 04:25 AM, David Sherwood wrote:
>>> Hi,
>>>
>>> Part 1 of this change is a clean-up. I have changed calls to GET_MODE_INNER (m)
>>> so that it returns m in cases where there is no inner mode. This simplifies some
>>> of the calling code by removing the need to check for VOIDmode and allows
>>> calling it unconditionally. I also removed element_precision () as it was only
>>> called in one place and thought it neater to call GET_MODE_PRECISION explicitly.
>>>
>>> Parts 2-4 will include further tidy-ups and optimisations based on [1/N].
>>>
>>> Good to go?
>>>
>>> Regards,
>>> David Sherwood.
>>>
>>> 2015-07-17  David Sherwood<david.sherwood@arm.com>
>>>
>>>       gcc/
>>>           * config/arm/arm.c (neon_element_bits, neon_valid_immediate): Call
>>>           GET_MODE_INNER unconditionally.
>>>           * config/spu/spu.c (arith_immediate_p): Likewise.
>>>           * config/i386/i386.c (ix86_build_signbit_mask): Likewise.  New variable.
>>>           * expmed.c (synth_mult): Remove check for VOIDmode result from
>>>           GET_MODE_INNER.
>>>           (expand_mult_const): Likewise.
>>>           * fold-const.c (): Replace call to element_precision with call to
>>>           GET_MODE_PRECISION.
>>>           * genmodes.c (emit_mode_inner_inline): Replace void_mode->name with
>>>           m->name.
>>>           (emit_mode_inner): Likewise.
>>>           * lto-streamer-out.c (lto_write_mode_table): Update GET_MODE_INNER
>>>           result check.
>>>           * machmode.h (GET_MODE_UNIT_SIZE): Simplify.
>>>           (GET_MODE_UNIT_PRECISION): Likewise.
>>>           * rtlanal.c (subreg_get_info): Call GET_MODE_INNER unconditionally.
>>>           * simplify-rtx.c (simplify_immed_subreg): Likewise.
>>>           * stor-layout.c (bitwise_type_for_mode): Update assert.
>>>           (element_precision): Remove.
>> Somehow my brain kept translating INNER into NARROWER.  Naturally I was
>> having considerable trouble seeing how the patch could be correct ;-)
>> Looking at insn-modes.h cleared things up quickly.
>>
>> In a lot of ways this makes GET_INNER_MODE act more like
>> GET_MODE_NUNITS, which is probably good.
>>
>> You need to update the comment for GET_MODE_INNER in machmode.h to
>> reflect the change in its return value for non-vector modes.
> Thanks for the quick response! Before I post a new patch, does this new
> comment seem ok?
>
> /* Where MODE represents a vector return the mode of the inner elements,
> otherwise just return MODE.  */
Yes, that's fine.

Thanks,
jeff

Re: [PATCH][1/N] Change GET_MODE_INNER to always return a non-void mode

[Richard Sandiford]

"David Sherwood" <david.sherwood@arm.com> writes:
> Hi,
>
> I have updated the comment above GET_MODE_INNER and while there I have
> fixed a spelling mistake in the comment above GET_MODE_UNIT_SIZE.
>
> Tested:
> aarch64 and aarch64_be - no regressions in gcc testsuite
> x86_64 - bootstrap build, no testsuite regressions
> arm-none-eabi - no regressions in gcc testsuite
> Run contrib/config-list.mk - only build failures are ones that fail anyway with
> warnings being treated as errors.
>
> Hope this is ok.
>
> Cheers,
> Dave.

Since Jeff conditionally approved the patch, I went ahead and applied it.

Thanks,
Richard

RE: [PATCH][1/N] Change GET_MODE_INNER to always return a non-void mode

[David Sherwood]

[-- Attachment #1: Type: text/plain, Size: 4122 bytes --]

Hi,

I have updated the comment above GET_MODE_INNER and while there I have
fixed a spelling mistake in the comment above GET_MODE_UNIT_SIZE.

Tested:
aarch64 and aarch64_be - no regressions in gcc testsuite
x86_64 - bootstrap build, no testsuite regressions
arm-none-eabi - no regressions in gcc testsuite
Run contrib/config-list.mk - only build failures are ones that fail anyway with
warnings being treated as errors.

Hope this is ok.

Cheers,
Dave.

2015-07-28  David Sherwood  <david.sherwood@arm.com>

    gcc/
        * config/arm/arm.c (neon_element_bits, neon_valid_immediate): Call
        GET_MODE_INNER unconditionally.
        * config/spu/spu.c (arith_immediate_p): Likewise.
        * config/i386/i386.c (ix86_build_signbit_mask): Likewise.  New variable.
        * expmed.c (synth_mult): Remove check for VOIDmode result from
        GET_MODE_INNER.
        (expand_mult_const): Likewise.
        * fold-const.c (): Replace call to element_precision with call to
        GET_MODE_PRECISION.
        * genmodes.c (emit_mode_inner_inline): Replace void_mode->name with
        m->name.
        (emit_mode_inner): Likewise.
        * lto-streamer-out.c (lto_write_mode_table): Update GET_MODE_INNER
        result check.
        * machmode.h (GET_MODE_INNER): Update comment.
        (GET_MODE_UNIT_SIZE): Simplify and fix spelling mistake in comment.
        (GET_MODE_UNIT_PRECISION): Simplify.
        (element_precision): Remove.
        * rtlanal.c (subreg_get_info): Call GET_MODE_INNER unconditionally.
        * simplify-rtx.c (simplify_immed_subreg): Likewise.
        * stor-layout.c (bitwise_type_for_mode): Update assert.
        (element_precision): Remove.

> 
> On 07/27/2015 04:25 AM, David Sherwood wrote:
> > Hi,
> >
> > Part 1 of this change is a clean-up. I have changed calls to GET_MODE_INNER (m)
> > so that it returns m in cases where there is no inner mode. This simplifies some
> > of the calling code by removing the need to check for VOIDmode and allows
> > calling it unconditionally. I also removed element_precision () as it was only
> > called in one place and thought it neater to call GET_MODE_PRECISION explicitly.
> >
> > Parts 2-4 will include further tidy-ups and optimisations based on [1/N].
> >
> > Good to go?
> >
> > Regards,
> > David Sherwood.
> >
> > 2015-07-17  David Sherwood<david.sherwood@arm.com>
> >
> >      gcc/
> >          * config/arm/arm.c (neon_element_bits, neon_valid_immediate): Call
> >          GET_MODE_INNER unconditionally.
> >          * config/spu/spu.c (arith_immediate_p): Likewise.
> >          * config/i386/i386.c (ix86_build_signbit_mask): Likewise.  New variable.
> >          * expmed.c (synth_mult): Remove check for VOIDmode result from
> >          GET_MODE_INNER.
> >          (expand_mult_const): Likewise.
> >          * fold-const.c (): Replace call to element_precision with call to
> >          GET_MODE_PRECISION.
> >          * genmodes.c (emit_mode_inner_inline): Replace void_mode->name with
> >          m->name.
> >          (emit_mode_inner): Likewise.
> >          * lto-streamer-out.c (lto_write_mode_table): Update GET_MODE_INNER
> >          result check.
> >          * machmode.h (GET_MODE_UNIT_SIZE): Simplify.
> >          (GET_MODE_UNIT_PRECISION): Likewise.
> >          * rtlanal.c (subreg_get_info): Call GET_MODE_INNER unconditionally.
> >          * simplify-rtx.c (simplify_immed_subreg): Likewise.
> >          * stor-layout.c (bitwise_type_for_mode): Update assert.
> >          (element_precision): Remove.
> Somehow my brain kept translating INNER into NARROWER.  Naturally I was
> having considerable trouble seeing how the patch could be correct ;-)
> Looking at insn-modes.h cleared things up quickly.
> 
> In a lot of ways this makes GET_INNER_MODE act more like
> GET_MODE_NUNITS, which is probably good.
> 
> You need to update the comment for GET_MODE_INNER in machmode.h to
> reflect the change in its return value for non-vector modes.
> 
> With that update, this patch is fine.
> 
> jeff


[-- Attachment #2: mode_inner1.patch --]
[-- Type: application/octet-stream, Size: 10222 bytes --]

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index e1bc727..ab96824 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -12234,18 +12234,16 @@ neon_valid_immediate (rtx op, machine_mode mode, int inverse,
   bool vector = GET_CODE (op) == CONST_VECTOR;
 
   if (vector)
-    {
-      n_elts = CONST_VECTOR_NUNITS (op);
-      innersize = GET_MODE_SIZE (GET_MODE_INNER (mode));
-    }
+    n_elts = CONST_VECTOR_NUNITS (op);
   else
     {
       n_elts = 1;
       if (mode == VOIDmode)
 	mode = DImode;
-      innersize = GET_MODE_SIZE (mode);
     }
 
+  innersize = GET_MODE_SIZE (GET_MODE_INNER (mode));
+
   /* Vectors of float constants.  */
   if (GET_MODE_CLASS (mode) == MODE_VECTOR_FLOAT)
     {
@@ -12821,10 +12819,7 @@ neon_const_bounds (rtx operand, HOST_WIDE_INT low, HOST_WIDE_INT high)
 HOST_WIDE_INT
 neon_element_bits (machine_mode mode)
 {
-  if (mode == DImode)
-    return GET_MODE_BITSIZE (mode);
-  else
-    return GET_MODE_BITSIZE (GET_MODE_INNER (mode));
+  return GET_MODE_BITSIZE (GET_MODE_INNER (mode));
 }
 
 \f
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 55e1e2d..c68e284 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -19626,7 +19626,6 @@ ix86_build_signbit_mask (machine_mode mode, bool vect, bool invert)
     case V8SFmode:
     case V4SFmode:
       vec_mode = mode;
-      mode = GET_MODE_INNER (mode);
       imode = SImode;
       break;
 
@@ -19637,7 +19636,6 @@ ix86_build_signbit_mask (machine_mode mode, bool vect, bool invert)
     case V4DFmode:
     case V2DFmode:
       vec_mode = mode;
-      mode = GET_MODE_INNER (mode);
       imode = DImode;
       break;
 
@@ -19651,17 +19649,18 @@ ix86_build_signbit_mask (machine_mode mode, bool vect, bool invert)
       gcc_unreachable ();
     }
 
-  w = wi::set_bit_in_zero (GET_MODE_BITSIZE (mode) - 1,
-			   GET_MODE_BITSIZE (mode));
+  machine_mode inner_mode = GET_MODE_INNER (mode);
+  w = wi::set_bit_in_zero (GET_MODE_BITSIZE (inner_mode) - 1,
+			   GET_MODE_BITSIZE (inner_mode));
   if (invert)
     w = wi::bit_not (w);
 
   /* Force this value into the low part of a fp vector constant.  */
   mask = immed_wide_int_const (w, imode);
-  mask = gen_lowpart (mode, mask);
+  mask = gen_lowpart (inner_mode, mask);
 
   if (vec_mode == VOIDmode)
-    return force_reg (mode, mask);
+    return force_reg (inner_mode, mask);
 
   v = ix86_build_const_vector (vec_mode, vect, mask);
   return force_reg (vec_mode, v);
diff --git a/gcc/config/spu/spu.c b/gcc/config/spu/spu.c
index 99efb67..68840f5 100644
--- a/gcc/config/spu/spu.c
+++ b/gcc/config/spu/spu.c
@@ -3391,9 +3391,7 @@ arith_immediate_p (rtx op, machine_mode mode,
 
   constant_to_array (mode, op, arr);
 
-  if (VECTOR_MODE_P (mode))
-    mode = GET_MODE_INNER (mode);
-
+  mode = GET_MODE_INNER (mode);
   bytes = GET_MODE_SIZE (mode);
   mode = mode_for_size (GET_MODE_BITSIZE (mode), MODE_INT, 0);
 
diff --git a/gcc/expmed.c b/gcc/expmed.c
index 557da33..3c70cde 100644
--- a/gcc/expmed.c
+++ b/gcc/expmed.c
@@ -2427,8 +2427,6 @@ synth_mult (struct algorithm *alg_out, unsigned HOST_WIDE_INT t,
 
   /* Be prepared for vector modes.  */
   imode = GET_MODE_INNER (mode);
-  if (imode == VOIDmode)
-    imode = mode;
 
   maxm = MIN (BITS_PER_WORD, GET_MODE_BITSIZE (imode));
 
@@ -3088,8 +3086,6 @@ expand_mult_const (machine_mode mode, rtx op0, HOST_WIDE_INT val,
   /* Compare only the bits of val and val_so_far that are significant
      in the result mode, to avoid sign-/zero-extension confusion.  */
   nmode = GET_MODE_INNER (mode);
-  if (nmode == VOIDmode)
-    nmode = mode;
   val &= GET_MODE_MASK (nmode);
   val_so_far &= GET_MODE_MASK (nmode);
   gcc_assert (val == val_so_far);
diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 93dd29d..12b15f8 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -9503,7 +9503,7 @@ fold_binary_loc (location_t loc,
 	    /* Only create rotates in complete modes.  Other cases are not
 	       expanded properly.  */
 	    && (element_precision (rtype)
-		== element_precision (TYPE_MODE (rtype))))
+		== GET_MODE_PRECISION (GET_MODE_INNER (TYPE_MODE (rtype)))))
 	  {
 	    tree tree01, tree11;
 	    enum tree_code code01, code11;
diff --git a/gcc/genmodes.c b/gcc/genmodes.c
index 8e9337c..f4db427 100644
--- a/gcc/genmodes.c
+++ b/gcc/genmodes.c
@@ -1039,7 +1039,7 @@ mode_inner_inline (machine_mode mode)\n\
   for_all_modes (c, m)
     printf ("    case %smode: return %smode;\n", m->name,
 	    c != MODE_PARTIAL_INT && m->component
-	    ? m->component->name : void_mode->name);
+	    ? m->component->name : m->name);
 
   puts ("\
     default: return mode_inner[mode];\n\
@@ -1338,7 +1338,7 @@ emit_mode_inner (void)
   for_all_modes (c, m)
     tagged_printf ("%smode",
 		   c != MODE_PARTIAL_INT && m->component
-		   ? m->component->name : void_mode->name,
+		   ? m->component->name : m->name,
 		   m->name);
 
   print_closer ();
diff --git a/gcc/lto-streamer-out.c b/gcc/lto-streamer-out.c
index 2d048e8..1b88115 100644
--- a/gcc/lto-streamer-out.c
+++ b/gcc/lto-streamer-out.c
@@ -2682,7 +2682,7 @@ lto_write_mode_table (void)
     if (streamer_mode_table[i])
       {
 	machine_mode m = (machine_mode) i;
-	if (GET_MODE_INNER (m) != VOIDmode)
+	if (GET_MODE_INNER (m) != m)
 	  streamer_mode_table[(int) GET_MODE_INNER (m)] = 1;
       }
   /* First stream modes that have GET_MODE_INNER (m) == VOIDmode,
@@ -2692,7 +2692,7 @@ lto_write_mode_table (void)
       if (streamer_mode_table[i] && i != (int) VOIDmode && i != (int) BLKmode)
 	{
 	  machine_mode m = (machine_mode) i;
-	  if ((GET_MODE_INNER (m) == VOIDmode) ^ (pass == 0))
+	  if ((GET_MODE_INNER (m) == m) ^ (pass == 0))
 	    continue;
 	  bp_pack_value (&bp, m, 8);
 	  bp_pack_enum (&bp, mode_class, MAX_MODE_CLASS, GET_MODE_CLASS (m));
diff --git a/gcc/machmode.h b/gcc/machmode.h
index 5ab7eeb..6575517 100644
--- a/gcc/machmode.h
+++ b/gcc/machmode.h
@@ -209,7 +209,9 @@ extern const unsigned HOST_WIDE_INT mode_mask_array[NUM_MACHINE_MODES];
 
 #define GET_MODE_MASK(MODE) mode_mask_array[MODE]
 
-/* Return the mode of the inner elements in a vector.  */
+/* Return the mode of the basic parts of MODE.  For vector modes this is the
+   mode of the vector elements.  For complex modes it is the mode of the real
+   and imaginary parts.  For other modes it is MODE itself.  */
 
 extern const unsigned char mode_inner[NUM_MACHINE_MODES];
 #if GCC_VERSION >= 4001
@@ -220,21 +222,15 @@ extern const unsigned char mode_inner[NUM_MACHINE_MODES];
 #define GET_MODE_INNER(MODE) ((machine_mode) mode_inner[MODE])
 #endif
 
-/* Get the size in bytes or bites of the basic parts of an
+/* Get the size in bytes or bits of the basic parts of an
    object of mode MODE.  */
 
-#define GET_MODE_UNIT_SIZE(MODE)		\
-  (GET_MODE_INNER (MODE) == VOIDmode		\
-   ? GET_MODE_SIZE (MODE)			\
-   : GET_MODE_SIZE (GET_MODE_INNER (MODE)))
+#define GET_MODE_UNIT_SIZE(MODE) GET_MODE_SIZE (GET_MODE_INNER (MODE))
 
 #define GET_MODE_UNIT_BITSIZE(MODE) \
   ((unsigned short) (GET_MODE_UNIT_SIZE (MODE) * BITS_PER_UNIT))
 
-#define GET_MODE_UNIT_PRECISION(MODE)		\
-  (GET_MODE_INNER (MODE) == VOIDmode		\
-   ? GET_MODE_PRECISION (MODE)			\
-   : GET_MODE_PRECISION (GET_MODE_INNER (MODE)))
+#define GET_MODE_UNIT_PRECISION(MODE) GET_MODE_PRECISION (GET_MODE_INNER (MODE))
 
 /* Get the number of units in the object.  */
 
@@ -320,10 +316,6 @@ extern unsigned get_mode_alignment (machine_mode);
 
 #define GET_MODE_ALIGNMENT(MODE) get_mode_alignment (MODE)
 
-/* Get the precision of the mode or its inner mode if it has one.  */
-
-extern unsigned int element_precision (machine_mode);
-
 /* For each class, get the narrowest mode in that class.  */
 
 extern const unsigned char class_narrowest_mode[MAX_MODE_CLASS];
diff --git a/gcc/rtlanal.c b/gcc/rtlanal.c
index 0bba878..4269a71 100644
--- a/gcc/rtlanal.c
+++ b/gcc/rtlanal.c
@@ -3571,10 +3571,7 @@ subreg_get_info (unsigned int xregno, machine_mode xmode,
       machine_mode xmode_unit;
 
       nregs_xmode = HARD_REGNO_NREGS_WITH_PADDING (xregno, xmode);
-      if (GET_MODE_INNER (xmode) == VOIDmode)
-	xmode_unit = xmode;
-      else
-	xmode_unit = GET_MODE_INNER (xmode);
+      xmode_unit = GET_MODE_INNER (xmode);
       gcc_assert (HARD_REGNO_NREGS_HAS_PADDING (xregno, xmode_unit));
       gcc_assert (nregs_xmode
 		  == (GET_MODE_NUNITS (xmode)
diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
index fde9944..70c1308 100644
--- a/gcc/simplify-rtx.c
+++ b/gcc/simplify-rtx.c
@@ -5622,21 +5622,17 @@ simplify_immed_subreg (machine_mode outermode, rtx op,
   value_start = byte * (BITS_PER_UNIT / value_bit);
 
   /* Re-pack the value.  */
+  num_elem = GET_MODE_NUNITS (outermode);
 
   if (VECTOR_MODE_P (outermode))
     {
-      num_elem = GET_MODE_NUNITS (outermode);
       result_v = rtvec_alloc (num_elem);
       elems = &RTVEC_ELT (result_v, 0);
-      outer_submode = GET_MODE_INNER (outermode);
     }
   else
-    {
-      num_elem = 1;
-      elems = &result_s;
-      outer_submode = outermode;
-    }
+    elems = &result_s;
 
+  outer_submode = GET_MODE_INNER (outermode);
   outer_class = GET_MODE_CLASS (outer_submode);
   elem_bitsize = GET_MODE_BITSIZE (outer_submode);
 
diff --git a/gcc/stor-layout.c b/gcc/stor-layout.c
index 0d4f4a4..9757777 100644
--- a/gcc/stor-layout.c
+++ b/gcc/stor-layout.c
@@ -491,7 +491,7 @@ bitwise_type_for_mode (machine_mode mode)
   if (COMPLEX_MODE_P (mode))
     return build_complex_type (inner_type);
 
-  gcc_checking_assert (GET_MODE_INNER (mode) == VOIDmode);
+  gcc_checking_assert (GET_MODE_INNER (mode) == mode);
   return inner_type;
 }
 
@@ -548,18 +548,6 @@ get_mode_alignment (machine_mode mode)
   return MIN (BIGGEST_ALIGNMENT, MAX (1, mode_base_align[mode]*BITS_PER_UNIT));
 }
 
-/* Return the precision of the mode, or for a complex or vector mode the
-   precision of the mode of its elements.  */
-
-unsigned int
-element_precision (machine_mode mode)
-{
-  if (COMPLEX_MODE_P (mode) || VECTOR_MODE_P (mode))
-    mode = GET_MODE_INNER (mode);
-
-  return GET_MODE_PRECISION (mode);
-}
-
 /* Return the natural mode of an array, given that it is SIZE bytes in
    total and has elements of type ELEM_TYPE.  */
 

RE: [PATCH][1/N] Change GET_MODE_INNER to always return a non-void mode

[David Sherwood]

> 
> On 07/27/2015 04:25 AM, David Sherwood wrote:
> > Hi,
> >
> > Part 1 of this change is a clean-up. I have changed calls to GET_MODE_INNER (m)
> > so that it returns m in cases where there is no inner mode. This simplifies some
> > of the calling code by removing the need to check for VOIDmode and allows
> > calling it unconditionally. I also removed element_precision () as it was only
> > called in one place and thought it neater to call GET_MODE_PRECISION explicitly.
> >
> > Parts 2-4 will include further tidy-ups and optimisations based on [1/N].
> >
> > Good to go?
> >
> > Regards,
> > David Sherwood.
> >
> > 2015-07-17  David Sherwood<david.sherwood@arm.com>
> >
> >      gcc/
> >          * config/arm/arm.c (neon_element_bits, neon_valid_immediate): Call
> >          GET_MODE_INNER unconditionally.
> >          * config/spu/spu.c (arith_immediate_p): Likewise.
> >          * config/i386/i386.c (ix86_build_signbit_mask): Likewise.  New variable.
> >          * expmed.c (synth_mult): Remove check for VOIDmode result from
> >          GET_MODE_INNER.
> >          (expand_mult_const): Likewise.
> >          * fold-const.c (): Replace call to element_precision with call to
> >          GET_MODE_PRECISION.
> >          * genmodes.c (emit_mode_inner_inline): Replace void_mode->name with
> >          m->name.
> >          (emit_mode_inner): Likewise.
> >          * lto-streamer-out.c (lto_write_mode_table): Update GET_MODE_INNER
> >          result check.
> >          * machmode.h (GET_MODE_UNIT_SIZE): Simplify.
> >          (GET_MODE_UNIT_PRECISION): Likewise.
> >          * rtlanal.c (subreg_get_info): Call GET_MODE_INNER unconditionally.
> >          * simplify-rtx.c (simplify_immed_subreg): Likewise.
> >          * stor-layout.c (bitwise_type_for_mode): Update assert.
> >          (element_precision): Remove.
> Somehow my brain kept translating INNER into NARROWER.  Naturally I was
> having considerable trouble seeing how the patch could be correct ;-)
> Looking at insn-modes.h cleared things up quickly.
> 
> In a lot of ways this makes GET_INNER_MODE act more like
> GET_MODE_NUNITS, which is probably good.
> 
> You need to update the comment for GET_MODE_INNER in machmode.h to
> reflect the change in its return value for non-vector modes.
Thanks for the quick response! Before I post a new patch, does this new
comment seem ok?

/* Where MODE represents a vector return the mode of the inner elements,
otherwise just return MODE.  */

Dave.

> 
> With that update, this patch is fine.
> 
> jeff

Re: [PATCH][1/N] Change GET_MODE_INNER to always return a non-void mode

[Jeff Law]

On 07/27/2015 04:25 AM, David Sherwood wrote:
> Hi,
>
> Part 1 of this change is a clean-up. I have changed calls to GET_MODE_INNER (m)
> so that it returns m in cases where there is no inner mode. This simplifies some
> of the calling code by removing the need to check for VOIDmode and allows
> calling it unconditionally. I also removed element_precision () as it was only
> called in one place and thought it neater to call GET_MODE_PRECISION explicitly.
>
> Parts 2-4 will include further tidy-ups and optimisations based on [1/N].
>
> Good to go?
>
> Regards,
> David Sherwood.
>
> 2015-07-17  David Sherwood<david.sherwood@arm.com>
>
>      gcc/
>          * config/arm/arm.c (neon_element_bits, neon_valid_immediate): Call
>          GET_MODE_INNER unconditionally.
>          * config/spu/spu.c (arith_immediate_p): Likewise.
>          * config/i386/i386.c (ix86_build_signbit_mask): Likewise.  New variable.
>          * expmed.c (synth_mult): Remove check for VOIDmode result from
>          GET_MODE_INNER.
>          (expand_mult_const): Likewise.
>          * fold-const.c (): Replace call to element_precision with call to
>          GET_MODE_PRECISION.
>          * genmodes.c (emit_mode_inner_inline): Replace void_mode->name with
>          m->name.
>          (emit_mode_inner): Likewise.
>          * lto-streamer-out.c (lto_write_mode_table): Update GET_MODE_INNER
>          result check.
>          * machmode.h (GET_MODE_UNIT_SIZE): Simplify.
>          (GET_MODE_UNIT_PRECISION): Likewise.
>          * rtlanal.c (subreg_get_info): Call GET_MODE_INNER unconditionally.
>          * simplify-rtx.c (simplify_immed_subreg): Likewise.
>          * stor-layout.c (bitwise_type_for_mode): Update assert.
>          (element_precision): Remove.
Somehow my brain kept translating INNER into NARROWER.  Naturally I was 
having considerable trouble seeing how the patch could be correct ;-) 
Looking at insn-modes.h cleared things up quickly.

In a lot of ways this makes GET_INNER_MODE act more like 
GET_MODE_NUNITS, which is probably good.

You need to update the comment for GET_MODE_INNER in machmode.h to 
reflect the change in its return value for non-vector modes.

With that update, this patch is fine.

jeff

[PATCH][1/N] Change GET_MODE_INNER to always return a non-void mode

[David Sherwood]

[-- Attachment #1: Type: text/plain, Size: 1670 bytes --]

Hi,

Part 1 of this change is a clean-up. I have changed calls to GET_MODE_INNER (m)
so that it returns m in cases where there is no inner mode. This simplifies some
of the calling code by removing the need to check for VOIDmode and allows
calling it unconditionally. I also removed element_precision () as it was only
called in one place and thought it neater to call GET_MODE_PRECISION explicitly.

Parts 2-4 will include further tidy-ups and optimisations based on [1/N].

Good to go?

Regards,
David Sherwood.

2015-07-17  David Sherwood  <david.sherwood@arm.com>

    gcc/
        * config/arm/arm.c (neon_element_bits, neon_valid_immediate): Call
        GET_MODE_INNER unconditionally.
        * config/spu/spu.c (arith_immediate_p): Likewise.
        * config/i386/i386.c (ix86_build_signbit_mask): Likewise.  New variable.
        * expmed.c (synth_mult): Remove check for VOIDmode result from
        GET_MODE_INNER.
        (expand_mult_const): Likewise.
        * fold-const.c (): Replace call to element_precision with call to
        GET_MODE_PRECISION.
        * genmodes.c (emit_mode_inner_inline): Replace void_mode->name with
        m->name.
        (emit_mode_inner): Likewise.
        * lto-streamer-out.c (lto_write_mode_table): Update GET_MODE_INNER
        result check.
        * machmode.h (GET_MODE_UNIT_SIZE): Simplify.
        (GET_MODE_UNIT_PRECISION): Likewise.
        * rtlanal.c (subreg_get_info): Call GET_MODE_INNER unconditionally.
        * simplify-rtx.c (simplify_immed_subreg): Likewise.
        * stor-layout.c (bitwise_type_for_mode): Update assert.
        (element_precision): Remove.


[-- Attachment #2: mode_inner1.patch --]
[-- Type: application/octet-stream, Size: 9575 bytes --]

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index e1bc727..ab96824 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -12234,18 +12234,16 @@ neon_valid_immediate (rtx op, machine_mode mode, int inverse,
   bool vector = GET_CODE (op) == CONST_VECTOR;
 
   if (vector)
-    {
-      n_elts = CONST_VECTOR_NUNITS (op);
-      innersize = GET_MODE_SIZE (GET_MODE_INNER (mode));
-    }
+    n_elts = CONST_VECTOR_NUNITS (op);
   else
     {
       n_elts = 1;
       if (mode == VOIDmode)
 	mode = DImode;
-      innersize = GET_MODE_SIZE (mode);
     }
 
+  innersize = GET_MODE_SIZE (GET_MODE_INNER (mode));
+
   /* Vectors of float constants.  */
   if (GET_MODE_CLASS (mode) == MODE_VECTOR_FLOAT)
     {
@@ -12821,10 +12819,7 @@ neon_const_bounds (rtx operand, HOST_WIDE_INT low, HOST_WIDE_INT high)
 HOST_WIDE_INT
 neon_element_bits (machine_mode mode)
 {
-  if (mode == DImode)
-    return GET_MODE_BITSIZE (mode);
-  else
-    return GET_MODE_BITSIZE (GET_MODE_INNER (mode));
+  return GET_MODE_BITSIZE (GET_MODE_INNER (mode));
 }
 
 \f
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 55e1e2d..c68e284 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -19626,7 +19626,6 @@ ix86_build_signbit_mask (machine_mode mode, bool vect, bool invert)
     case V8SFmode:
     case V4SFmode:
       vec_mode = mode;
-      mode = GET_MODE_INNER (mode);
       imode = SImode;
       break;
 
@@ -19637,7 +19636,6 @@ ix86_build_signbit_mask (machine_mode mode, bool vect, bool invert)
     case V4DFmode:
     case V2DFmode:
       vec_mode = mode;
-      mode = GET_MODE_INNER (mode);
       imode = DImode;
       break;
 
@@ -19651,17 +19649,18 @@ ix86_build_signbit_mask (machine_mode mode, bool vect, bool invert)
       gcc_unreachable ();
     }
 
-  w = wi::set_bit_in_zero (GET_MODE_BITSIZE (mode) - 1,
-			   GET_MODE_BITSIZE (mode));
+  machine_mode inner_mode = GET_MODE_INNER (mode);
+  w = wi::set_bit_in_zero (GET_MODE_BITSIZE (inner_mode) - 1,
+			   GET_MODE_BITSIZE (inner_mode));
   if (invert)
     w = wi::bit_not (w);
 
   /* Force this value into the low part of a fp vector constant.  */
   mask = immed_wide_int_const (w, imode);
-  mask = gen_lowpart (mode, mask);
+  mask = gen_lowpart (inner_mode, mask);
 
   if (vec_mode == VOIDmode)
-    return force_reg (mode, mask);
+    return force_reg (inner_mode, mask);
 
   v = ix86_build_const_vector (vec_mode, vect, mask);
   return force_reg (vec_mode, v);
diff --git a/gcc/config/spu/spu.c b/gcc/config/spu/spu.c
index 99efb67..68840f5 100644
--- a/gcc/config/spu/spu.c
+++ b/gcc/config/spu/spu.c
@@ -3391,9 +3391,7 @@ arith_immediate_p (rtx op, machine_mode mode,
 
   constant_to_array (mode, op, arr);
 
-  if (VECTOR_MODE_P (mode))
-    mode = GET_MODE_INNER (mode);
-
+  mode = GET_MODE_INNER (mode);
   bytes = GET_MODE_SIZE (mode);
   mode = mode_for_size (GET_MODE_BITSIZE (mode), MODE_INT, 0);
 
diff --git a/gcc/expmed.c b/gcc/expmed.c
index 557da33..3c70cde 100644
--- a/gcc/expmed.c
+++ b/gcc/expmed.c
@@ -2427,8 +2427,6 @@ synth_mult (struct algorithm *alg_out, unsigned HOST_WIDE_INT t,
 
   /* Be prepared for vector modes.  */
   imode = GET_MODE_INNER (mode);
-  if (imode == VOIDmode)
-    imode = mode;
 
   maxm = MIN (BITS_PER_WORD, GET_MODE_BITSIZE (imode));
 
@@ -3088,8 +3086,6 @@ expand_mult_const (machine_mode mode, rtx op0, HOST_WIDE_INT val,
   /* Compare only the bits of val and val_so_far that are significant
      in the result mode, to avoid sign-/zero-extension confusion.  */
   nmode = GET_MODE_INNER (mode);
-  if (nmode == VOIDmode)
-    nmode = mode;
   val &= GET_MODE_MASK (nmode);
   val_so_far &= GET_MODE_MASK (nmode);
   gcc_assert (val == val_so_far);
diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 93dd29d..12b15f8 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -9503,7 +9503,7 @@ fold_binary_loc (location_t loc,
 	    /* Only create rotates in complete modes.  Other cases are not
 	       expanded properly.  */
 	    && (element_precision (rtype)
-		== element_precision (TYPE_MODE (rtype))))
+		== GET_MODE_PRECISION (GET_MODE_INNER (TYPE_MODE (rtype)))))
 	  {
 	    tree tree01, tree11;
 	    enum tree_code code01, code11;
diff --git a/gcc/genmodes.c b/gcc/genmodes.c
index 8e9337c..f4db427 100644
--- a/gcc/genmodes.c
+++ b/gcc/genmodes.c
@@ -1039,7 +1039,7 @@ mode_inner_inline (machine_mode mode)\n\
   for_all_modes (c, m)
     printf ("    case %smode: return %smode;\n", m->name,
 	    c != MODE_PARTIAL_INT && m->component
-	    ? m->component->name : void_mode->name);
+	    ? m->component->name : m->name);
 
   puts ("\
     default: return mode_inner[mode];\n\
@@ -1338,7 +1338,7 @@ emit_mode_inner (void)
   for_all_modes (c, m)
     tagged_printf ("%smode",
 		   c != MODE_PARTIAL_INT && m->component
-		   ? m->component->name : void_mode->name,
+		   ? m->component->name : m->name,
 		   m->name);
 
   print_closer ();
diff --git a/gcc/lto-streamer-out.c b/gcc/lto-streamer-out.c
index 2d048e8..1b88115 100644
--- a/gcc/lto-streamer-out.c
+++ b/gcc/lto-streamer-out.c
@@ -2682,7 +2682,7 @@ lto_write_mode_table (void)
     if (streamer_mode_table[i])
       {
 	machine_mode m = (machine_mode) i;
-	if (GET_MODE_INNER (m) != VOIDmode)
+	if (GET_MODE_INNER (m) != m)
 	  streamer_mode_table[(int) GET_MODE_INNER (m)] = 1;
       }
   /* First stream modes that have GET_MODE_INNER (m) == VOIDmode,
@@ -2692,7 +2692,7 @@ lto_write_mode_table (void)
       if (streamer_mode_table[i] && i != (int) VOIDmode && i != (int) BLKmode)
 	{
 	  machine_mode m = (machine_mode) i;
-	  if ((GET_MODE_INNER (m) == VOIDmode) ^ (pass == 0))
+	  if ((GET_MODE_INNER (m) == m) ^ (pass == 0))
 	    continue;
 	  bp_pack_value (&bp, m, 8);
 	  bp_pack_enum (&bp, mode_class, MAX_MODE_CLASS, GET_MODE_CLASS (m));
diff --git a/gcc/machmode.h b/gcc/machmode.h
index 5ab7eeb..c6b4064 100644
--- a/gcc/machmode.h
+++ b/gcc/machmode.h
@@ -223,18 +223,12 @@ extern const unsigned char mode_inner[NUM_MACHINE_MODES];
 /* Get the size in bytes or bites of the basic parts of an
    object of mode MODE.  */
 
-#define GET_MODE_UNIT_SIZE(MODE)		\
-  (GET_MODE_INNER (MODE) == VOIDmode		\
-   ? GET_MODE_SIZE (MODE)			\
-   : GET_MODE_SIZE (GET_MODE_INNER (MODE)))
+#define GET_MODE_UNIT_SIZE(MODE) GET_MODE_SIZE (GET_MODE_INNER (MODE))
 
 #define GET_MODE_UNIT_BITSIZE(MODE) \
   ((unsigned short) (GET_MODE_UNIT_SIZE (MODE) * BITS_PER_UNIT))
 
-#define GET_MODE_UNIT_PRECISION(MODE)		\
-  (GET_MODE_INNER (MODE) == VOIDmode		\
-   ? GET_MODE_PRECISION (MODE)			\
-   : GET_MODE_PRECISION (GET_MODE_INNER (MODE)))
+#define GET_MODE_UNIT_PRECISION(MODE) GET_MODE_PRECISION (GET_MODE_INNER (MODE))
 
 /* Get the number of units in the object.  */
 
@@ -320,10 +314,6 @@ extern unsigned get_mode_alignment (machine_mode);
 
 #define GET_MODE_ALIGNMENT(MODE) get_mode_alignment (MODE)
 
-/* Get the precision of the mode or its inner mode if it has one.  */
-
-extern unsigned int element_precision (machine_mode);
-
 /* For each class, get the narrowest mode in that class.  */
 
 extern const unsigned char class_narrowest_mode[MAX_MODE_CLASS];
diff --git a/gcc/rtlanal.c b/gcc/rtlanal.c
index 0bba878..4269a71 100644
--- a/gcc/rtlanal.c
+++ b/gcc/rtlanal.c
@@ -3571,10 +3571,7 @@ subreg_get_info (unsigned int xregno, machine_mode xmode,
       machine_mode xmode_unit;
 
       nregs_xmode = HARD_REGNO_NREGS_WITH_PADDING (xregno, xmode);
-      if (GET_MODE_INNER (xmode) == VOIDmode)
-	xmode_unit = xmode;
-      else
-	xmode_unit = GET_MODE_INNER (xmode);
+      xmode_unit = GET_MODE_INNER (xmode);
       gcc_assert (HARD_REGNO_NREGS_HAS_PADDING (xregno, xmode_unit));
       gcc_assert (nregs_xmode
 		  == (GET_MODE_NUNITS (xmode)
diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
index fde9944..70c1308 100644
--- a/gcc/simplify-rtx.c
+++ b/gcc/simplify-rtx.c
@@ -5622,21 +5622,17 @@ simplify_immed_subreg (machine_mode outermode, rtx op,
   value_start = byte * (BITS_PER_UNIT / value_bit);
 
   /* Re-pack the value.  */
+  num_elem = GET_MODE_NUNITS (outermode);
 
   if (VECTOR_MODE_P (outermode))
     {
-      num_elem = GET_MODE_NUNITS (outermode);
       result_v = rtvec_alloc (num_elem);
       elems = &RTVEC_ELT (result_v, 0);
-      outer_submode = GET_MODE_INNER (outermode);
     }
   else
-    {
-      num_elem = 1;
-      elems = &result_s;
-      outer_submode = outermode;
-    }
+    elems = &result_s;
 
+  outer_submode = GET_MODE_INNER (outermode);
   outer_class = GET_MODE_CLASS (outer_submode);
   elem_bitsize = GET_MODE_BITSIZE (outer_submode);
 
diff --git a/gcc/stor-layout.c b/gcc/stor-layout.c
index 0d4f4a4..9757777 100644
--- a/gcc/stor-layout.c
+++ b/gcc/stor-layout.c
@@ -491,7 +491,7 @@ bitwise_type_for_mode (machine_mode mode)
   if (COMPLEX_MODE_P (mode))
     return build_complex_type (inner_type);
 
-  gcc_checking_assert (GET_MODE_INNER (mode) == VOIDmode);
+  gcc_checking_assert (GET_MODE_INNER (mode) == mode);
   return inner_type;
 }
 
@@ -548,18 +548,6 @@ get_mode_alignment (machine_mode mode)
   return MIN (BIGGEST_ALIGNMENT, MAX (1, mode_base_align[mode]*BITS_PER_UNIT));
 }
 
-/* Return the precision of the mode, or for a complex or vector mode the
-   precision of the mode of its elements.  */
-
-unsigned int
-element_precision (machine_mode mode)
-{
-  if (COMPLEX_MODE_P (mode) || VECTOR_MODE_P (mode))
-    mode = GET_MODE_INNER (mode);
-
-  return GET_MODE_PRECISION (mode);
-}
-
 /* Return the natural mode of an array, given that it is SIZE bytes in
    total and has elements of type ELEM_TYPE.  */