Ping.
Richard Sandiford <richard.sandif...@linaro.org> writes: > Richard Biener <richard.guent...@gmail.com> writes: >> On Fri, Sep 22, 2017 at 6:42 PM, Richard Sandiford >> <richard.sandif...@linaro.org> wrote: >>> Richard Biener <richard.guent...@gmail.com> writes: >>>> On Thu, Sep 21, 2017 at 2:56 PM, Richard Sandiford >>>> <richard.sandif...@linaro.org> wrote: >>>>> Richard Biener <richard.guent...@gmail.com> writes: >>>>>> On September 20, 2017 2:36:03 PM GMT+02:00, Richard Sandiford >>>>>> <richard.sandif...@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.sandif...@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 >
