public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix PRE insertion wrt alignment/aliasing
@ 2016-07-07  7:46 Richard Biener
  0 siblings, 0 replies; only message in thread
From: Richard Biener @ 2016-07-07  7:46 UTC (permalink / raw)
  To: gcc-patches


This finally tries to solve the issue that PRE uses the VN tables
and their expression representation for insertion.  While for VN
things like alignment and aliasing do not matter it is quite important
to insert expressions that are compatible with the original one which
means not blindly take the canonical expression from the VN tables.

This patch fixes the issue in the least invasive way by only making
sure the canonical expression is conservative for all expressions
visited during PRE avail computation.

The issue hits via gcc.dg/torture/pr65270-1.c (formerly a tailmerging
bug) and code hoisting but I'm sure that with enough luck one can
construct a PRE testcase as well (it may need artificial disabling
of earlier memory CSE passes though).

Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.

Richard.

2016-07-07  Richard Biener  <rguenther@suse.de>

	* tree-ssa-pre.c: Include alias.h.
	(compute_avail): If we have multiple VN_REFERENCEs with the
	same hashtable entry adjust that to make it a valid replacement
	for all of them with respect to alignment and aliasing
	when doing insertion.
	* tree-ssa-sccvn.h (vn_reference_operands_for_lookup): Declare.
	* tree-ssa-sccvn.c (vn_reference_operands_for_lookup): New function.

Index: gcc/tree-ssa-pre.c
===================================================================
*** gcc/tree-ssa-pre.c	(revision 238003)
--- gcc/tree-ssa-pre.c	(working copy)
*************** along with GCC; see the file COPYING3.
*** 53,58 ****
--- 53,59 ----
  #include "ipa-utils.h"
  #include "tree-cfgcleanup.h"
  #include "langhooks.h"
+ #include "alias.h"
  
  /* TODO:
  
*************** compute_avail (void)
*** 3724,3735 ****
  
  		  case VN_REFERENCE:
  		    {
  		      vn_reference_t ref;
! 		      vn_reference_lookup (gimple_assign_rhs1 (stmt),
! 					   gimple_vuse (stmt),
! 					   VN_WALK, &ref, true);
  		      if (!ref)
! 			continue;
  
  		      /* If the value of the reference is not invalidated in
  			 this block until it is computed, add the expression
--- 3725,3743 ----
  
  		  case VN_REFERENCE:
  		    {
+ 		      tree rhs1 = gimple_assign_rhs1 (stmt);
+ 		      alias_set_type set = get_alias_set (rhs1);
+ 		      vec<vn_reference_op_s> operands
+ 			= vn_reference_operands_for_lookup (rhs1);
  		      vn_reference_t ref;
! 		      vn_reference_lookup_pieces (gimple_vuse (stmt), set,
! 						  TREE_TYPE (rhs1),
! 						  operands, &ref, VN_WALK);
  		      if (!ref)
! 			{
! 			  operands.release ();
! 			  continue;
! 			}
  
  		      /* If the value of the reference is not invalidated in
  			 this block until it is computed, add the expression
*************** compute_avail (void)
*** 3753,3759 ****
  				= SSA_NAME_DEF_STMT (gimple_vuse (def_stmt));
  			    }
  			  if (!ok)
! 			    continue;
  			}
  
  		      result = pre_expr_pool.allocate ();
--- 3761,3828 ----
  				= SSA_NAME_DEF_STMT (gimple_vuse (def_stmt));
  			    }
  			  if (!ok)
! 			    {
! 			      operands.release ();
! 			      continue;
! 			    }
! 			}
! 
! 		      /* If the load was value-numbered to another
! 			 load make sure we do not use its expression
! 			 for insertion if it wouldn't be a valid
! 			 replacement.  */
! 		      /* At the momemt we have a testcase
! 			 for hoist insertion of aligned vs. misaligned
! 			 variants in gcc.dg/torture/pr65270-1.c thus
! 			 with just alignment to be considered we can
! 			 simply replace the expression in the hashtable
! 			 with the most conservative one.  */
! 		      vn_reference_op_t ref1 = &ref->operands.last ();
! 		      while (ref1->opcode != TARGET_MEM_REF
! 			     && ref1->opcode != MEM_REF
! 			     && ref1 != &ref->operands[0])
! 			--ref1;
! 		      vn_reference_op_t ref2 = &operands.last ();
! 		      while (ref2->opcode != TARGET_MEM_REF
! 			     && ref2->opcode != MEM_REF
! 			     && ref2 != &operands[0])
! 			--ref2;
! 		      if ((ref1->opcode == TARGET_MEM_REF
! 			   || ref1->opcode == MEM_REF)
! 			  && (TYPE_ALIGN (ref1->type)
! 			      > TYPE_ALIGN (ref2->type)))
! 			{
! 			  ref->operands.release ();
! 			  ref->operands = operands;
! 			  ref1 = ref2;
! 			}
! 		      else
! 			operands.release ();
! 		      /* TBAA behavior is an obvious part so make sure
! 		         that the hashtable one covers this as well
! 			 by adjusting the ref alias set and its base.  */
! 		      if (ref->set == set
! 			  || alias_set_subset_of (set, ref->set))
! 			;
! 		      else if (alias_set_subset_of (ref->set, set))
! 			{
! 			  ref->set = set;
! 			  if (ref1->opcode == MEM_REF)
! 			    ref1->op0 = fold_convert (TREE_TYPE (ref2->op0),
! 						      ref1->op0);
! 			  else
! 			    ref1->op2 = fold_convert (TREE_TYPE (ref2->op2),
! 						      ref1->op2);
! 			}
! 		      else
! 			{
! 			  ref->set = 0;
! 			  if (ref1->opcode == MEM_REF)
! 			    ref1->op0 = fold_convert (ptr_type_node,
! 						      ref1->op0);
! 			  else
! 			    ref1->op2 = fold_convert (ptr_type_node,
! 						      ref1->op2);
  			}
  
  		      result = pre_expr_pool.allocate ();
Index: gcc/tree-ssa-sccvn.c
===================================================================
*** gcc/tree-ssa-sccvn.c	(revision 238003)
--- gcc/tree-ssa-sccvn.c	(working copy)
*************** vn_reference_lookup_3 (ao_ref *ref, tree
*** 2285,2290 ****
--- 2285,2301 ----
    return (void *)-1;
  }
  
+ /* Return a reference op vector from OP that can be used for
+    vn_reference_lookup_pieces.  The caller is responsible for releasing
+    the vector.  */
+ 
+ vec<vn_reference_op_s>
+ vn_reference_operands_for_lookup (tree op)
+ {
+   bool valueized;
+   return valueize_shared_reference_ops_from_ref (op, &valueized).copy ();
+ }
+ 
  /* Lookup a reference operation by it's parts, in the current hash table.
     Returns the resulting value number if it exists in the hash table,
     NULL_TREE otherwise.  VNRESULT will be filled in with the actual
Index: gcc/tree-ssa-sccvn.h
===================================================================
*** gcc/tree-ssa-sccvn.h	(revision 238003)
--- gcc/tree-ssa-sccvn.h	(working copy)
*************** vn_nary_op_t vn_nary_op_insert_pieces (u
*** 214,219 ****
--- 214,220 ----
  				       tree, tree *, tree, unsigned int);
  bool ao_ref_init_from_vn_reference (ao_ref *, alias_set_type, tree,
  				    vec<vn_reference_op_s> );
+ vec<vn_reference_op_s> vn_reference_operands_for_lookup (tree);
  tree vn_reference_lookup_pieces (tree, alias_set_type, tree,
  				 vec<vn_reference_op_s> ,
  				 vn_reference_t *, vn_lookup_kind);

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2016-07-07  7:46 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-07  7:46 [PATCH] Fix PRE insertion wrt alignment/aliasing Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).