Hi,
this patch makes vectorizer to increase alignments correctly in presence
of aliases. Currently increase happens at two places, first in
increase_alignment where we increase alignment of the DECL of alias as well
as alias target. This is not safe: it is possible that alias is static
and binds to current defintion, while target is not and may be interposed.
In this case we can not increase DECL_ALIGNMENT of target.
In the other place only alias DECL_ALIGN is bumped up. this however does not
make the definition (of alias target) to come out aligned and leads to segfault
in the testcase.
This patch moves the alignment decision to symbol table.
node->definition_alignment returns the desired alignment of the defintiion that
is maximal alignment needed by the declarations of symbol and its aliases.
I also moved logic deciding how to increase alignment into
node->can_increase_alignment_p predicate and node->increase_alignment that
increases alignment of all aliases that binds to current def.
I unforutnately spot another problem. With presence of section anchors we
can not increase alignment of symbol that was already placed into the section.
I think this is reason why we still have pass_increase_alignments that runs
only on target with anchors.
I added the following check:
+ /* If target is already placed in an anchor, we can not touch its
+ alignment. */
+ if (DECL_RTL_SET_P (target->decl)
+ && MEM_P (DECL_RTL (target->decl))
+ && SYMBOL_REF_HAS_BLOCK_INFO_P (XEXP (DECL_RTL (target->decl), 0)))
+ return false;
but found it breaks several vectorization testcases with LTO. This is
becuase notice_global_symbol computes DECL_RTL even before
node->increase_alignment is run and we are stuck.
I decided to track this separately from this patch that snowballed quite a bit
already. I do not see how to solve it short of making notice_global_symbol to
not compute DECL_RTL. It was never quite clear to me why notice_global_symbol
is actually doing this at first place. Is it becuase of target mangling
happing only at RTL generation time?
Bootstrapped/regtested x86_64-linux, ppc64-linux, bootstrapped on AIX, will
commit it later today.
Honza
PR ipa/65334
* cgraph.h (symtab_node): Add definition_alignment,
can_increase_alignment_p and increase_alignment.
* symtab.c (symtab_node::can_increase_alignment_p,
increase_alignment_1, symtab_node::increase_alignment,
symtab_node::definition_alignment): New.
* tree-vect-data-refs.c (vect_can_force_dr_alignment_p): Use
can_increase_alignment_p.
* tree-vectorizer.c (increase_alignment): Use increase_alignment.
* tree-vect-stmts.c (ensure_base_align): Likewise.
Index: tree-vect-data-refs.c
===================================================================
--- tree-vect-data-refs.c (revision 221223)
+++ tree-vect-data-refs.c (working copy)
@@ -5703,58 +5703,10 @@ vect_can_force_dr_alignment_p (const_tre
if (TREE_CODE (decl) != VAR_DECL)
return false;
- /* With -fno-toplevel-reorder we may have already output the constant. */
- if (TREE_ASM_WRITTEN (decl))
+ if (decl_in_symtab_p (decl)
+ && !symtab_node::get (decl)->can_increase_alignment_p ())
return false;
- /* Constant pool entries may be shared and not properly merged by LTO. */
- if (DECL_IN_CONSTANT_POOL (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_node::get (decl);
- if (flag_ltrans
- && (snode->in_other_partition
- || snode->get_partitioning_class () == SYMBOL_DUPLICATE))
- return false;
- }
-
- /* Do not override the alignment as specified by the ABI when the used
- attribute is set. */
- if (DECL_PRESERVE_P (decl))
- return false;
-
- /* Do not override explicit alignment set by the user when an explicit
- section name is also used. This is a common idiom used by many
- software projects. */
- if (TREE_STATIC (decl)
- && DECL_SECTION_NAME (decl) != NULL
- && !symtab_node::get (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_node::get (decl)->ultimate_alias_target ()->decl;
- if (target != decl)
- {
- if (DECL_PRESERVE_P (target))
- return false;
- decl = target;
- }
- }
-
if (TREE_STATIC (decl))
return (alignment <= MAX_OFILE_ALIGNMENT);
else
Index: symtab.c
===================================================================
--- symtab.c (revision 221223)
+++ symtab.c (working copy)
@@ -1908,3 +1908,99 @@ symtab_node::address_matters_p ()
gcc_assert (!alias);
return call_for_symbol_and_aliases (address_matters_1, NULL, true);
}
+
+/* Return ture if symbol's alignment may be increased. */
+
+bool
+symtab_node::can_increase_alignment_p (void)
+{
+ symtab_node *target = ultimate_alias_target ();
+
+ /* For now support only variables. */
+ if (TREE_CODE (decl) != VAR_DECL)
+ return false;
+
+ /* With -fno-toplevel-reorder we may have already output the constant. */
+ if (TREE_ASM_WRITTEN (target->decl))
+ return false;
+
+ /* Constant pool entries may be shared. */
+ if (DECL_IN_CONSTANT_POOL (target->decl))
+ return false;
+
+ /* 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. */
+ if (flag_ltrans
+ && (target->in_other_partition
+ || target->get_partitioning_class () == SYMBOL_DUPLICATE))
+ return false;
+
+ /* Do not override the alignment as specified by the ABI when the used
+ attribute is set. */
+ if (DECL_PRESERVE_P (decl) || DECL_PRESERVE_P (target->decl))
+ return false;
+
+ /* Do not override explicit alignment set by the user when an explicit
+ section name is also used. This is a common idiom used by many
+ software projects. */
+ if (DECL_SECTION_NAME (target->decl) != NULL && !target->implicit_section)
+ return false;
+
+ return true;
+}
+
+/* Worker for symtab_node::increase_alignment. */
+
+static bool
+increase_alignment_1 (symtab_node *n, void *v)
+{
+ unsigned int align = (size_t)v;
+ if (DECL_ALIGN (n->decl) < align
+ && n->can_increase_alignment_p ())
+ {
+ DECL_ALIGN (n->decl) = align;
+ DECL_USER_ALIGN (n->decl) = 1;
+ }
+ return false;
+}
+
+/* Increase alignment of THIS to ALIGN. */
+
+void
+symtab_node::increase_alignment (unsigned int align)
+{
+ gcc_assert (can_increase_alignment_p () && align < MAX_OFILE_ALIGNMENT);
+ ultimate_alias_target()->call_for_symbol_and_aliases (increase_alignment_1,
+ (void *)(size_t) align,
+ true);
+ gcc_assert (DECL_ALIGN (decl) >= align);
+}
+
+/* Helper for symtab_node::definition_alignment. */
+
+static bool
+get_alignment_1 (symtab_node *n, void *v)
+{
+ *((unsigned int *)v) = MAX (*((unsigned int *)v), DECL_ALIGN (n->decl));
+ return false;
+}
+
+/* Return desired alignment of the definition. This is NOT alignment useful
+ to access THIS, because THIS may be interposable and DECL_ALIGN should
+ be used instead. It however must be guaranteed when output definition
+ of THIS. */
+
+unsigned int
+symtab_node::definition_alignment ()
+{
+ unsigned int align = 0;
+ gcc_assert (!alias);
+ call_for_symbol_and_aliases (get_alignment_1, &align, true);
+ return align;
+}
Index: tree-vectorizer.c
===================================================================
--- tree-vectorizer.c (revision 221223)
+++ tree-vectorizer.c (working copy)
@@ -719,14 +719,7 @@ increase_alignment (void)
if (vect_can_force_dr_alignment_p (decl, alignment))
{
- DECL_ALIGN (decl) = TYPE_ALIGN (vectype);
- DECL_USER_ALIGN (decl) = 1;
- if (TREE_STATIC (decl))
- {
- tree target = symtab_node::get (decl)->ultimate_alias_target
()->decl;
- DECL_ALIGN (target) = TYPE_ALIGN (vectype);
- DECL_USER_ALIGN (target) = 1;
- }
+ vnode->increase_alignment (TYPE_ALIGN (vectype));
dump_printf (MSG_NOTE, "Increasing alignment of decl: ");
dump_generic_expr (MSG_NOTE, TDF_SLIM, decl);
dump_printf (MSG_NOTE, "\n");
Index: cgraph.h
===================================================================
--- cgraph.h (revision 221223)
+++ cgraph.h (working copy)
@@ -289,6 +289,18 @@ public:
/* Make DECL local. */
void make_decl_local (void);
+ /* Return desired alignment of the definition. This is NOT alignment useful
+ to access THIS, because THIS may be interposable and DECL_ALIGN should
+ be used instead. It however must be guaranteed when output definition
+ of THIS. */
+ unsigned int definition_alignment ();
+
+ /* Return true if alignment can be increased. */
+ bool can_increase_alignment_p ();
+
+ /* Increase alignment of symbol to ALIGN. */
+ void increase_alignment (unsigned int align);
+
/* Return true if list contains an alias. */
bool has_aliases_p (void);
Index: tree-vect-stmts.c
===================================================================
--- tree-vect-stmts.c (revision 221223)
+++ tree-vect-stmts.c (working copy)
@@ -4956,8 +4956,13 @@ ensure_base_align (stmt_vec_info stmt_in
tree vectype = STMT_VINFO_VECTYPE (stmt_info);
tree base_decl = ((dataref_aux *)dr->aux)->base_decl;
- DECL_ALIGN (base_decl) = TYPE_ALIGN (vectype);
- DECL_USER_ALIGN (base_decl) = 1;
+ if (decl_in_symtab_p (base_decl))
+ symtab_node::get (base_decl)->increase_alignment (TYPE_ALIGN (vectype));
+ else
+ {
+ DECL_ALIGN (base_decl) = TYPE_ALIGN (vectype);
+ DECL_USER_ALIGN (base_decl) = 1;
+ }
((dataref_aux *)dr->aux)->base_misaligned = false;
}
}
