This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Fix vectorizer conditions on updating alignment


On Mon, Jun 16, 2014 at 10:24:26AM +0100, Jan Hubicka wrote:
> > 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.
> 
> 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  <hubicka@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  <hubicka@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))
>      {
> 

Attachment: vect-fails.log
Description: Text document


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]