Re: Don't query the frontend for unsupported types

[Richard Sandiford <richard dot sandiford at linaro dot org>]

Richard Biener <richard.guenther@gmail.com> writes:
> On Fri, Sep 22, 2017 at 6:42 PM, Richard Sandiford
> <richard.sandiford@linaro.org> wrote:
>> Richard Biener <richard.guenther@gmail.com> writes:
>>> On Thu, Sep 21, 2017 at 2:56 PM, Richard Sandiford
>>> <richard.sandiford@linaro.org> wrote:
>>>> Richard Biener <richard.guenther@gmail.com> writes:
>>>>> On September 20, 2017 2:36:03 PM GMT+02:00, Richard Sandiford
>>>>> <richard.sandiford@linaro.org> wrote:
>>>>>>When forcing a constant of mode MODE into memory, force_const_mem
>>>>>>asks the frontend to provide the type associated with that mode.
>>>>>>In principle type_for_mode is allowed to return null, and although
>>>>>>one use site correctly handled that, the other didn't.
>>>>>>
>>>>>>I think there's agreement that it's bogus to use type_for_mode for
>>>>>>this kind of thing, since it forces frontends to handle types that
>>>>>>don't exist in that language.  See e.g. http://gcc.gnu.org/PR46805
>>>>>>where the Go frontend was forced to handle vector types even though
>>>>>>Go doesn't have vector types.
>>>>>>
>>>>>>Also, the frontends use code like:
>>>>>>
>>>>>>  else if (VECTOR_MODE_P (mode))
>>>>>>    {
>>>>>>      machine_mode inner_mode = GET_MODE_INNER (mode);
>>>>>>      tree inner_type = c_common_type_for_mode (inner_mode, unsignedp);
>>>>>>      if (inner_type != NULL_TREE)
>>>>>>        return build_vector_type_for_mode (inner_type, mode);
>>>>>>    }
>>>>>>
>>>>>>and there's no guarantee that every vector mode M used by backend
>>>>>>rtl has an associated vector type whose TYPE_MODE is M.  I think
>>>>>>really the type_for_mode hook should only return trees that _do_ have
>>>>>>the requested TYPE_MODE, but PR46805 linked above shows that this is
>>>>>>likely to have too many knock-on consequences.  It doesn't make sense
>>>>>>for force_const_mem to ask about vector modes that aren't valid for
>>>>>>vector types, so this patch handles the condition there instead.
>>>>>>
>>>>>>This is needed for SVE multi-register modes, which are modelled as
>>>>>>vector modes but are not usable as vector types.
>>>>>>
>>>>>>Tested on aarch64-linux-gnu, x86_64-linux-gnu and
>>>>>>powerpc64le-linus-gnu.
>>>>>>OK to install?
>>>>>
>>>>> I think we should get rid of the use entirely.
>>>>
>>>> I first read this as not using type_for_mode at all in force_const_mem,
>>>> which sounded like a good thing :-)
>>>
>>> That's what I meant ;)  A mode doesn't really have a type...
>>>
>>>   I tried it overnight on the usual
>>>> at-least-one-target-per-CPU set and diffing the before and after
>>>> assembly for the testsuite.  And it looks like i686 relies on this
>>>> to get an alignment of 16 rather than 4 for XFmode constants:
>>>> GET_MODE_ALIGNMENT (XFmode) == 32 (as requested by i386-modes.def),
>>>> but i386's CONSTANT_ALIGNMENT increases it to 128 for static constants.
>>>
>>> Then the issue is that CONSTANT_ALIGNMENT takes a tree and not a mode...
>>> even worse than type_for_mode is a use of make_tree!  Incidentially
>>> ix86_constant_alignment _does_ look at the mode in the end...
>>
>> OK, I guess this means another target hook conversion.  The patch
>> below converts CONSTANT_ALIGNMENT with its current interface.
>> The definition:
>>
>>   #define CONSTANT_ALIGNMENT(EXP, ALIGN) \
>>     (TREE_CODE (EXP) == STRING_CST \
>>      && (ALIGN) < BITS_PER_WORD ? BITS_PER_WORD : (ALIGN))
>>
>> was very common, so the patch adds a canned definition for that,
>> called constant_alignment_word_strings.  Some ports had a variation
>> that used a port-local FASTEST_ALIGNMENT instead of BITS_PER_WORD;
>> the patch uses constant_alignment_word_strings if FASTEST_ALIGNMENT
>> was always BITS_PER_WORD and a port-local hook function otherwise.
>>
>> Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linux-gnu.
>> Also tested by comparing the testsuite assembly output on at least one
>> target per CPU directory.  I don't think this comes under Jeff's
>> preapproval due to the constant_alignment_word_strings thing, so:
>> OK to install?
>
> Ok.

Thanks.  A bit later than intended, but here's the follow-on to add
the new rtx hook.

Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linux-gnu.
Also tested by comparing the testsuite assembly output on at least one
target per CPU directory.  OK to install?

Richard


2017-10-01  Richard Sandiford  <richard.sandiford@linaro.org>

gcc/
	* target.def (static_rtx_alignment): New hook.
	* targhooks.h (default_static_rtx_alignment): Declare.
	* targhooks.c (default_static_rtx_alignment): New function.
	* doc/tm.texi.in (TARGET_STATIC_RTX_ALIGNMENT): New hook.
	* doc/tm.texi: Regenerate.
	* varasm.c (force_const_mem): Use targetm.static_rtx_alignment
	instead of targetm.constant_alignment.  Remove call to
	set_mem_attributes.
	* config/cris/cris.c (TARGET_STATIC_RTX_ALIGNMENT): Redefine.
	(cris_preferred_mininum_alignment): New function, split out from...
	(cris_constant_alignment): ...here.
	(cris_static_rtx_alignment): New function.
	* config/i386/i386.c (ix86_static_rtx_alignment): New function,
	split out from...
	(ix86_constant_alignment): ...here.
	(TARGET_STATIC_RTX_ALIGNMENT): Redefine.
	* config/mmix/mmix.c (TARGET_STATIC_RTX_ALIGNMENT): Redefine.
	(mmix_static_rtx_alignment): New function.
	* config/spu/spu.c (spu_static_rtx_alignment): New function.
	(TARGET_STATIC_RTX_ALIGNMENT): Redefine.

Index: gcc/target.def
===================================================================
--- gcc/target.def	2017-09-25 17:04:16.792359030 +0100
+++ gcc/target.def	2017-10-01 17:14:18.480815538 +0100
@@ -3336,6 +3336,15 @@ HOOK_VECTOR_END (addr_space)
 #define HOOK_PREFIX "TARGET_"
 
 DEFHOOK
+(static_rtx_alignment,
+ "This hook returns the preferred alignment in bits for a\n\
+statically-allocated rtx, such as a constant pool entry.  @var{mode}\n\
+is the mode of the rtx.  The default implementation returns\n\
+@samp{GET_MODE_ALIGNMENT (@var{mode})}.",
+ HOST_WIDE_INT, (machine_mode mode),
+ default_static_rtx_alignment)
+
+DEFHOOK
 (constant_alignment,
  "This hook returns the alignment in bits of a constant that is being\n\
 placed in memory.  @var{constant} is the constant and @var{basic_align}\n\
Index: gcc/targhooks.h
===================================================================
--- gcc/targhooks.h	2017-09-25 17:04:16.793358890 +0100
+++ gcc/targhooks.h	2017-10-01 17:14:18.481821912 +0100
@@ -93,6 +93,7 @@ extern int default_builtin_vectorization
 
 extern tree default_builtin_reciprocal (tree);
 
+extern HOST_WIDE_INT default_static_rtx_alignment (machine_mode);
 extern HOST_WIDE_INT default_constant_alignment (const_tree, HOST_WIDE_INT);
 extern HOST_WIDE_INT constant_alignment_word_strings (const_tree,
 						      HOST_WIDE_INT);
Index: gcc/targhooks.c
===================================================================
--- gcc/targhooks.c	2017-09-25 17:04:16.793358890 +0100
+++ gcc/targhooks.c	2017-10-01 17:14:18.481821912 +0100
@@ -1165,6 +1165,14 @@ tree default_mangle_decl_assembler_name
    return id;
 }
 
+/* The default implementation of TARGET_STATIC_RTX_ALIGNMENT.  */
+
+HOST_WIDE_INT
+default_static_rtx_alignment (machine_mode mode)
+{
+  return GET_MODE_ALIGNMENT (mode);
+}
+
 /* The default implementation of TARGET_CONSTANT_ALIGNMENT.  */
 
 HOST_WIDE_INT
Index: gcc/doc/tm.texi.in
===================================================================
--- gcc/doc/tm.texi.in	2017-09-25 17:04:16.792359030 +0100
+++ gcc/doc/tm.texi.in	2017-10-01 17:14:18.480815538 +0100
@@ -1026,6 +1026,8 @@ On 32-bit ELF the largest supported sect
 @samp{(0x80000000 * 8)}, but this is not representable on 32-bit hosts.
 @end defmac
 
+@hook TARGET_STATIC_RTX_ALIGNMENT
+
 @defmac DATA_ALIGNMENT (@var{type}, @var{basic-align})
 If defined, a C expression to compute the alignment for a variable in
 the static store.  @var{type} is the data type, and @var{basic-align} is
Index: gcc/doc/tm.texi
===================================================================
--- gcc/doc/tm.texi	2017-09-25 17:04:16.791359170 +0100
+++ gcc/doc/tm.texi	2017-10-01 17:14:18.479809163 +0100
@@ -1078,6 +1078,13 @@ On 32-bit ELF the largest supported sect
 @samp{(0x80000000 * 8)}, but this is not representable on 32-bit hosts.
 @end defmac
 
+@deftypefn {Target Hook} HOST_WIDE_INT TARGET_STATIC_RTX_ALIGNMENT (machine_mode @var{mode})
+This hook returns the preferred alignment in bits for a
+statically-allocated rtx, such as a constant pool entry.  @var{mode}
+is the mode of the rtx.  The default implementation returns
+@samp{GET_MODE_ALIGNMENT (@var{mode})}.
+@end deftypefn
+
 @defmac DATA_ALIGNMENT (@var{type}, @var{basic-align})
 If defined, a C expression to compute the alignment for a variable in
 the static store.  @var{type} is the data type, and @var{basic-align} is
Index: gcc/varasm.c
===================================================================
--- gcc/varasm.c	2017-09-25 17:04:16.793358890 +0100
+++ gcc/varasm.c	2017-10-01 17:14:18.481821912 +0100
@@ -3784,11 +3784,8 @@ force_const_mem (machine_mode mode, rtx
   *slot = desc;
 
   /* Align the location counter as required by EXP's data type.  */
-  align = GET_MODE_ALIGNMENT (mode == VOIDmode ? word_mode : mode);
-
-  tree type = lang_hooks.types.type_for_mode (mode, 0);
-  if (type != NULL_TREE)
-    align = targetm.constant_alignment (make_tree (type, x), align);
+  machine_mode align_mode = (mode == VOIDmode ? word_mode : mode);
+  align = targetm.static_rtx_alignment (align_mode);
 
   pool->offset += (align / BITS_PER_UNIT) - 1;
   pool->offset &= ~ ((align / BITS_PER_UNIT) - 1);
@@ -3830,7 +3827,6 @@ force_const_mem (machine_mode mode, rtx
 
   /* Construct the MEM.  */
   desc->mem = def = gen_const_mem (mode, symbol);
-  set_mem_attributes (def, lang_hooks.types.type_for_mode (mode, 0), 1);
   set_mem_align (def, align);
 
   /* If we're dropping a label to the constant pool, make sure we
Index: gcc/config/cris/cris.c
===================================================================
--- gcc/config/cris/cris.c	2017-09-25 17:04:16.762363228 +0100
+++ gcc/config/cris/cris.c	2017-10-01 17:14:18.472764540 +0100
@@ -165,6 +165,7 @@ static bool cris_function_value_regno_p
 static void cris_file_end (void);
 static unsigned int cris_hard_regno_nregs (unsigned int, machine_mode);
 static bool cris_hard_regno_mode_ok (unsigned int, machine_mode);
+static HOST_WIDE_INT cris_static_rtx_alignment (machine_mode);
 static HOST_WIDE_INT cris_constant_alignment (const_tree, HOST_WIDE_INT);
 
 /* This is the parsed result of the "-max-stack-stackframe=" option.  If
@@ -288,6 +289,8 @@ #define TARGET_HARD_REGNO_NREGS cris_har
 #undef TARGET_HARD_REGNO_MODE_OK
 #define TARGET_HARD_REGNO_MODE_OK cris_hard_regno_mode_ok
 
+#undef TARGET_STATIC_RTX_ALIGNMENT
+#define TARGET_STATIC_RTX_ALIGNMENT cris_static_rtx_alignment
 #undef TARGET_CONSTANT_ALIGNMENT
 #define TARGET_CONSTANT_ALIGNMENT cris_constant_alignment
 
@@ -4329,6 +4332,26 @@ cris_hard_regno_mode_ok (unsigned int re
 	      || (regno != CRIS_MOF_REGNUM && regno != CRIS_ACR_REGNUM)));
 }
 
+/* Return the preferred minimum alignment for a static object.  */
+
+static HOST_WIDE_INT
+cris_preferred_mininum_alignment (void)
+{
+  if (!TARGET_CONST_ALIGN)
+    return 8;
+  if (TARGET_ALIGN_BY_32)
+    return 32;
+  return 16;
+}
+
+/* Implement TARGET_STATIC_RTX_ALIGNMENT.  */
+
+static HOST_WIDE_INT
+cris_static_rtx_alignment (machine_mode mode)
+{
+  return MAX (cris_preferred_mininum_alignment (), GET_MODE_ALIGNMENT (mode));
+}
+
 /* Implement TARGET_CONSTANT_ALIGNMENT.  Note that this hook has the
    effect of making gcc believe that ALL references to constant stuff
    (in code segment, like strings) have this alignment.  That is a rather
@@ -4339,11 +4362,7 @@ cris_hard_regno_mode_ok (unsigned int re
 static HOST_WIDE_INT
 cris_constant_alignment (const_tree, HOST_WIDE_INT basic_align)
 {
-  if (!TARGET_CONST_ALIGN)
-    return basic_align;
-  if (TARGET_ALIGN_BY_32)
-    return MAX (basic_align, 32);
-  return MAX (basic_align, 16);
+  return MAX (cris_preferred_mininum_alignment (), basic_align);
 }
 
 #if 0
Index: gcc/config/i386/i386.c
===================================================================
--- gcc/config/i386/i386.c	2017-09-25 17:04:16.768362388 +0100
+++ gcc/config/i386/i386.c	2017-10-01 17:14:18.477796414 +0100
@@ -31563,6 +31563,18 @@ ix86_sched_init_global (FILE *, int, int
 }
 
 
+/* Implement TARGET_STATIC_RTX_ALIGNMENT.  */
+
+static HOST_WIDE_INT
+ix86_static_rtx_alignment (machine_mode mode)
+{
+  if (mode == DFmode)
+    return 64;
+  if (ALIGN_MODE_128 (mode))
+    return MAX (128, GET_MODE_ALIGNMENT (mode));
+  return GET_MODE_ALIGNMENT (mode);
+}
+
 /* Implement TARGET_CONSTANT_ALIGNMENT.  */
 
 static HOST_WIDE_INT
@@ -31571,10 +31583,9 @@ ix86_constant_alignment (const_tree exp,
   if (TREE_CODE (exp) == REAL_CST || TREE_CODE (exp) == VECTOR_CST
       || TREE_CODE (exp) == INTEGER_CST)
     {
-      if (TYPE_MODE (TREE_TYPE (exp)) == DFmode && align < 64)
-	return 64;
-      else if (ALIGN_MODE_128 (TYPE_MODE (TREE_TYPE (exp))) && align < 128)
-	return 128;
+      machine_mode mode = TYPE_MODE (TREE_TYPE (exp));
+      HOST_WIDE_INT mode_align = ix86_static_rtx_alignment (mode);
+      return MAX (mode_align, align);
     }
   else if (!optimize_size && TREE_CODE (exp) == STRING_CST
 	   && TREE_STRING_LENGTH (exp) >= 31 && align < BITS_PER_WORD)
@@ -53605,6 +53616,8 @@ #define TARGET_HARD_REGNO_CALL_PART_CLOB
 #undef TARGET_CAN_CHANGE_MODE_CLASS
 #define TARGET_CAN_CHANGE_MODE_CLASS ix86_can_change_mode_class
 
+#undef TARGET_STATIC_RTX_ALIGNMENT
+#define TARGET_STATIC_RTX_ALIGNMENT ix86_static_rtx_alignment
 #undef TARGET_CONSTANT_ALIGNMENT
 #define TARGET_CONSTANT_ALIGNMENT ix86_constant_alignment
 
Index: gcc/config/mmix/mmix.c
===================================================================
--- gcc/config/mmix/mmix.c	2017-09-25 17:04:16.774361549 +0100
+++ gcc/config/mmix/mmix.c	2017-10-01 17:14:18.477796414 +0100
@@ -168,6 +168,7 @@ static void mmix_print_operand (FILE *,
 static void mmix_print_operand_address (FILE *, machine_mode, rtx);
 static bool mmix_print_operand_punct_valid_p (unsigned char);
 static void mmix_conditional_register_usage (void);
+static HOST_WIDE_INT mmix_static_rtx_alignment (machine_mode);
 static HOST_WIDE_INT mmix_constant_alignment (const_tree, HOST_WIDE_INT);
 
 /* Target structure macros.  Listed by node.  See `Using and Porting GCC'
@@ -283,6 +284,8 @@ #define TARGET_TRAMPOLINE_INIT mmix_tram
 #undef TARGET_OPTION_OVERRIDE
 #define TARGET_OPTION_OVERRIDE mmix_option_override
 
+#undef TARGET_STATIC_RTX_ALIGNMENT
+#define TARGET_STATIC_RTX_ALIGNMENT mmix_static_rtx_alignment
 #undef TARGET_CONSTANT_ALIGNMENT
 #define TARGET_CONSTANT_ALIGNMENT mmix_constant_alignment
 
@@ -338,6 +341,14 @@ mmix_data_alignment (tree type ATTRIBUTE
   return basic_align;
 }
 
+/* Implement TARGET_STATIC_RTX_ALIGNMENT.  */
+
+static HOST_WIDE_INT
+mmix_static_rtx_alignment (machine_mode mode)
+{
+  return MAX (GET_MODE_ALIGNMENT (mode), 32);
+}
+
 /* Implement tARGET_CONSTANT_ALIGNMENT.  */
 
 static HOST_WIDE_INT
Index: gcc/config/spu/spu.c
===================================================================
--- gcc/config/spu/spu.c	2017-09-25 17:04:16.787359730 +0100
+++ gcc/config/spu/spu.c	2017-10-01 17:14:18.478802788 +0100
@@ -7194,6 +7194,18 @@ spu_truly_noop_truncation (unsigned int
   return inprec <= 32 && outprec <= inprec;
 }
 
+/* Implement TARGET_STATIC_RTX_ALIGNMENT.
+
+   Make all static objects 16-byte aligned.  This allows us to assume
+   they are also padded to 16 bytes, which means we can use a single
+   load or store instruction to access them.  */
+
+static HOST_WIDE_INT
+spu_static_rtx_alignment (machine_mode mode)
+{
+  return MAX (GET_MODE_ALIGNMENT (mode), 128);
+}
+
 /* Implement TARGET_CONSTANT_ALIGNMENT.
 
    Make all static objects 16-byte aligned.  This allows us to assume
@@ -7445,6 +7457,8 @@ #define TARGET_CAN_CHANGE_MODE_CLASS spu
 #undef TARGET_TRULY_NOOP_TRUNCATION
 #define TARGET_TRULY_NOOP_TRUNCATION spu_truly_noop_truncation
 
+#undef TARGET_STATIC_RTX_ALIGNMENT
+#define TARGET_STATIC_RTX_ALIGNMENT spu_static_rtx_alignment
 #undef TARGET_CONSTANT_ALIGNMENT
 #define TARGET_CONSTANT_ALIGNMENT spu_constant_alignment