Re: Fix vectorizer conditions on updating alignment

2014-11-23 Thread Eric Botcazou 
 It's quite recent and it will break Ada iirc.

I don't think so, it will break the case for which it was added, possibly 
something to do with Altivec.  But doing svn annotate is easy enough.

-- 
Eric Botcazou

Re: Fix vectorizer conditions on updating alignment

2014-11-22 Thread H.J. Lu 
On Thu, Jun 12, 2014 at 3:14 PM, Jan Hubicka hubi...@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 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.

It caused:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64024
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64028

H.J.

Re: Fix vectorizer conditions on updating alignment

2014-11-22 Thread Richard Biener 
On November 22, 2014 6:38:03 PM CET, H.J. Lu hjl.to...@gmail.com wrote:
On Thu, Jun 12, 2014 at 3:14 PM, Jan Hubicka hubi...@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.

It's quite recent and it will break Ada iirc.

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.

It caused:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64024
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64028

H.J.

Re: Fix vectorizer conditions on updating alignment

2014-06-24 Thread James Greenhalgh 
On Mon, Jun 16, 2014 at 10:24:26AM +0100, Jan Hubicka wrote:
  On Fri, Jun 13, 2014 at 12:14 AM, Jan Hubicka hubi...@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.
 
 It is middle-end/50494.
 
 I have re-instantiated the check and also added back TREE_ASM_WRITTEN that is 
 needed
 in the case of -fno-toplevel-reorder.  I will try to look into the constant 
 pool output
 machinery but indeed it is a nasty problem.
 
 Thanks,
 Honza

Hi Honza,

This patch has caused a number of vector tests to start failing for -fPIC
variants of the testsuite. (I'm running aarch64-none-elf,
aarch64-none-linux-gnu, arm-none-eabi, arm-none-linux-gnueabihf)

The full set of tests which have started failing for me are attached, a sample
failure looks like:

FAIL: gcc.dg/vect/vect-109.c scan-tree-dump-times vect Vectorizing an 
unaligned access 3

Looking on test-results I see similar problems on the i32coreavx target:

https://gcc.gnu.org/ml/gcc-testresults/2014-06/msg02097.html

Thanks,
James


 
 Index: ChangeLog
 ===
 --- ChangeLog (revision 211689)
 +++ ChangeLog (working copy)
 @@ -1,5 +1,10 @@
  2014-06-15  Jan Hubicka  hubi...@ucw.cz
  
 + * tree-vect-data-refs.c (vect_can_force_dr_alignment_p): Check again
 + DECL_IN_CONSTANT_POOL and TREE_ASM_WRITTEN.
 +
 +2014-06-15  Jan Hubicka  hubi...@ucw.cz
 +
   * c-family/c-common.c (handle_tls_model_attribute): Use 
 set_decl_tls_model.
   * cgraph.h (struct varpool_node): Add tls_model.
   * tree.c (decl_tls_model, set_decl_tls_model): New functions.
 Index: tree-vect-data-refs.c
 ===
 --- tree-vect-data-refs.c (revision 211688)
 +++ tree-vect-data-refs.c (working copy)
 @@ -5317,7 +5317,13 @@ vect_can_force_dr_alignment_p (const_tre
if (TREE_CODE (decl) != VAR_DECL)
  return false;
  
 -  gcc_assert (!TREE_ASM_WRITTEN (decl));
 +  /* With -fno-toplevel-reorder we may have already output the constant.  */
 +  if (TREE_ASM_WRITTEN (decl))
 +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))
  {
 Test Run By pdtltest on Mon Jun 23 20:00:58 2014
Target is aarch64-none-elf
Host   is x86_64-unknown-linux-gnu

=== gcc Tests for aarch64-elf-foundation/-mcmodel=tiny/-fPIC ===

FAIL: gcc.dg/vect/vect-109.c scan-tree-dump-times vect Vectorizing an 
unaligned access 3
FAIL: gcc.dg/vect/vect-13.c scan-tree-dump-times vect Vectorizing an unaligned 
access 0
FAIL: gcc.dg/vect/vect-17.c scan-tree-dump-times vect Vectorizing an unaligned 
access 0
FAIL: gcc.dg/vect/vect-18.c scan-tree-dump-times vect Vectorizing an unaligned 
access 0
FAIL: gcc.dg/vect/vect-19.c scan-tree-dump-times vect Vectorizing an unaligned 
access 0
FAIL: gcc.dg/vect/vect-2-big-array.c scan-tree-dump-times vect Vectorizing an 
unaligned access 0
FAIL: gcc.dg/vect/vect-2.c scan-tree-dump-times vect Vectorizing an unaligned 
access 0
FAIL: gcc.dg/vect/vect-20.c scan-tree-dump-times vect Vectorizing an unaligned 
access 0
FAIL: gcc.dg/vect/vect-21.c scan-tree-dump-times vect Vectorizing an unaligned 
access 0
FAIL: gcc.dg/vect/vect-22.c scan-tree-dump-times vect Vectorizing an unaligned 
access 0
FAIL: gcc.dg/vect/vect-27.c scan-tree-dump-times vect Alignment of access 
forced using peeling 0
FAIL: gcc.dg/vect/vect-29.c scan-tree-dump-times vect Alignment of access 
forced using peeling 0
FAIL: gcc.dg/vect/vect-3.c scan-tree-dump-times vect Vectorizing an unaligned 
access 0
FAIL: gcc.dg/vect/vect-4.c scan-tree-dump-times vect Vectorizing an unaligned 
access 0
FAIL: gcc.dg/vect/vect-5.c scan-tree-dump-times vect Vectorizing an unaligned 
access 0
FAIL: gcc.dg/vect/vect-7.c scan-tree-dump-times vect Vectorizing an unaligned 
access 0
FAIL: gcc.dg/vect/vect-72.c scan-tree-dump-times vect Alignment of access 
forced using peeling 0
FAIL: gcc.dg/vect/vect-73-big-array.c scan-tree-dump-times vect Vectorizing an 
unaligned access 0
FAIL:

Re: Fix vectorizer conditions on updating alignment

2014-06-16 Thread Jan Hubicka 
 On Fri, Jun 13, 2014 at 12:14 AM, Jan Hubicka hubi...@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.

It is middle-end/50494.

I have re-instantiated the check and also added back TREE_ASM_WRITTEN that is 
needed
in the case of -fno-toplevel-reorder.  I will try to look into the constant 
pool output
machinery but indeed it is a nasty problem.

Thanks,
Honza

Index: ChangeLog
===
--- ChangeLog   (revision 211689)
+++ ChangeLog   (working copy)
@@ -1,5 +1,10 @@
 2014-06-15  Jan Hubicka  hubi...@ucw.cz
 
+   * tree-vect-data-refs.c (vect_can_force_dr_alignment_p): Check again
+   DECL_IN_CONSTANT_POOL and TREE_ASM_WRITTEN.
+
+2014-06-15  Jan Hubicka  hubi...@ucw.cz
+
* c-family/c-common.c (handle_tls_model_attribute): Use 
set_decl_tls_model.
* cgraph.h (struct varpool_node): Add tls_model.
* tree.c (decl_tls_model, set_decl_tls_model): New functions.
Index: tree-vect-data-refs.c
===
--- tree-vect-data-refs.c   (revision 211688)
+++ tree-vect-data-refs.c   (working copy)
@@ -5317,7 +5317,13 @@ vect_can_force_dr_alignment_p (const_tre
   if (TREE_CODE (decl) != VAR_DECL)
 return false;
 
-  gcc_assert (!TREE_ASM_WRITTEN (decl));
+  /* With -fno-toplevel-reorder we may have already output the constant.  */
+  if (TREE_ASM_WRITTEN (decl))
+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))
 {

Re: Fix vectorizer conditions on updating alignment

2014-06-13 Thread Richard Biener 
On Fri, Jun 13, 2014 at 12:14 AM, Jan Hubicka hubi...@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);