On Fri, Jun 13, 2014 at 12:14 AM, Jan Hubicka <[email protected]> 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");
