Fix vectorizer conditions on updating alignment

[Richard Biener]

On Fri, Jun 13, 2014 at 12:14 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
> Hi,
> while updating vect_can_force_dr_alignment_p for section API I noticed the
> predicate is bit confused about when it can update the alignment.
>
> We need to check that decl_binds_to_current_def_p and in case we compile
> a partition also that the symbol is not homed in other partition.
> Previous code was wrong i.e. for COMDATs, weaks or -fpic.
>
> Also when having an alias, only way to promote the alignment is to bump
> up alignment of target.
>
> On the other hand comment about DECL_IN_CONSTANT_POOL seems confused - we have
> no sharing across partitions. I assume it was old hack and removed it.

I don't think that code was confused.  It's because of the way we emit
the constant pool and use its hash (we duplicate the decl).  Do a svn
blame and see for the associated PR which you now broke again
I guess.  It wasn't about LTO I think.

Richard.

> I also see no reason for disregarding DECL_PRESERVE - we only update
> alignment that should not disturb whatever magic user does. But I kept
> it.
>
> We probably should separate the logic into symtab predicate - it just checks if
> we can change definition of variable to meet our needs. I can do that
> incrementally.
>
> Bootstrapped/regtested x86_64-linux, comitted.
>
> Honza
>
>         * tree-vect-data-refs.c (vect_can_force_dr_alignment_p): Reorg
>         to use symtab and decl_binds_to_current_def_p
>         * tree-vectorizer.c (increase_alignment): Increase alignment
>         of alias target, too.
> Index: tree-vect-data-refs.c
> ===================================================================
> --- tree-vect-data-refs.c       (revision 211489)
> +++ tree-vect-data-refs.c       (working copy)
> @@ -58,6 +58,7 @@ along with GCC; see the file COPYING3.
>  #include "expr.h"
>  #include "optabs.h"
>  #include "builtins.h"
> +#include "varasm.h"
>
>  /* Return true if load- or store-lanes optab OPTAB is implemented for
>     COUNT vectors of type VECTYPE.  NAME is the name of OPTAB.  */
> @@ -5316,19 +5317,26 @@ vect_can_force_dr_alignment_p (const_tre
>    if (TREE_CODE (decl) != VAR_DECL)
>      return false;
>
> -  /* We cannot change alignment of common or external symbols as another
> -     translation unit may contain a definition with lower alignment.
> -     The rules of common symbol linking mean that the definition
> -     will override the common symbol.  The same is true for constant
> -     pool entries which may be shared and are not properly merged
> -     by LTO.  */
> -  if (DECL_EXTERNAL (decl)
> -      || DECL_COMMON (decl)
> -      || DECL_IN_CONSTANT_POOL (decl))
> -    return false;
> +  gcc_assert (!TREE_ASM_WRITTEN (decl));
>
> -  if (TREE_ASM_WRITTEN (decl))
> -    return false;
> +  if (TREE_PUBLIC (decl) || DECL_EXTERNAL (decl))
> +    {
> +      symtab_node *snode;
> +
> +      /* We cannot change alignment of symbols that may bind to symbols
> +        in other translation unit that may contain a definition with lower
> +        alignment.  */
> +      if (!decl_binds_to_current_def_p (decl))
> +       return false;
> +
> +      /* When compiling partition, be sure the symbol is not output by other
> +        partition.  */
> +      snode = symtab_get_node (decl);
> +      if (flag_ltrans
> +         && (snode->in_other_partition
> +             || symtab_get_symbol_partitioning_class (snode) == SYMBOL_DUPLICATE))
> +       return false;
> +    }
>
>    /* Do not override the alignment as specified by the ABI when the used
>       attribute is set.  */
> @@ -5343,6 +5351,18 @@ vect_can_force_dr_alignment_p (const_tre
>        && !symtab_get_node (decl)->implicit_section)
>      return false;
>
> +  /* If symbol is an alias, we need to check that target is OK.  */
> +  if (TREE_STATIC (decl))
> +    {
> +      tree target = symtab_alias_ultimate_target (symtab_get_node (decl))->decl;
> +      if (target != decl)
> +       {
> +         if (DECL_PRESERVE_P (target))
> +           return false;
> +         decl = target;
> +       }
> +    }
> +
>    if (TREE_STATIC (decl))
>      return (alignment <= MAX_OFILE_ALIGNMENT);
>    else
> Index: tree-vectorizer.c
> ===================================================================
> --- tree-vectorizer.c   (revision 211488)
> +++ tree-vectorizer.c   (working copy)
> @@ -686,6 +686,12 @@ increase_alignment (void)
>          {
>            DECL_ALIGN (decl) = TYPE_ALIGN (vectype);
>            DECL_USER_ALIGN (decl) = 1;
> +         if (TREE_STATIC (decl))
> +           {
> +             tree target = symtab_alias_ultimate_target (symtab_get_node (decl))->decl;
> +              DECL_ALIGN (target) = TYPE_ALIGN (vectype);
> +              DECL_USER_ALIGN (target) = 1;
> +           }
>            dump_printf (MSG_NOTE, "Increasing alignment of decl: ");
>            dump_generic_expr (MSG_NOTE, TDF_SLIM, decl);
>            dump_printf (MSG_NOTE, "\n");