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]

[RFC] Should SRA stop producing COMPONENT_REF for non-bit-fields (again)?


Hi everyone, especially Richi and Eric,

I'd like to know what is your attitude to changing SRA's
build_ref_for_model to what it once looked like, so that it produces
COMPONENT_REFs only for bit-fields.  The non-bit field handling was
added in order to work-around problems when expanding non-aligned
MEM_REFs on strict-alignment targets but that should not be a problem
now and my experiments confirm that.  Last week I successfully
bootstrapped and tested this patch on sparc64-linux (with the
temporary MEM_EXPR patch, not including Java), ia64-linux (without
Ada), x86_64-linux, i686-linux and tested it on hppa-linux (only C and
C++).

The main downside of this change I see is that dumps would be a bit
more difficult to read and understand when the fields disappear from
them.

The upsides are:

  - the expr field of SRA access was originally intended only for
    debugging (meaning both for compiler-produced debug info and
    debugging SRA).  It was never intended to influence the memory
    accesses produced by SRA and when we create them artificially, the
    effects of the particular form are hard to reason about.  If we
    ever lower bit-field accesses on gimple level, build_ref_for_model
    could go away completely (yeah, I know I'm getting carried way
    here).

  - If something like PR 51528 creeps up again and we need to create
    replacements of type returned by lang_hooks.types.type_for_mode,
    the produced MEM_REFs could simply have this type.  OTOH, the
    current COMPONENT_REFs would require to be encapsulated in V_C_Es
    and that is quite a nightmare.  I tried it in December, even made
    it work, but it was particularly ugly and needed some quite
    questionable uses of V_C_Es.

  - Well, it does the same thing and is much simpler, is it not?

The patch fulfills the criteria to be committed and I can do it soon.
OTOH, keeping it so on a number of platforms takes quite a lot of time
(and has uncovered some non-related bugs) so I'd like to know whether
it's worth it.

Thanks,

Martin



2012-03-20 Martin Jambor <mjambor@suse.cz>

	* tree-sra.c (build_ref_for_model): Create COMPONENT_REFs only for
	bit-fields.

Index: src/gcc/tree-sra.c
===================================================================
*** src.orig/gcc/tree-sra.c
--- src/gcc/tree-sra.c
*************** build_ref_for_offset (location_t loc, tr
*** 1489,1558 ****
    return fold_build2_loc (loc, MEM_REF, exp_type, base, off);
  }
  
- DEF_VEC_ALLOC_P_STACK (tree);
- #define VEC_tree_stack_alloc(alloc) VEC_stack_alloc (tree, alloc)
- 
  /* Construct a memory reference to a part of an aggregate BASE at the given
!    OFFSET and of the type of MODEL.  In case this is a chain of references
!    to component, the function will replicate the chain of COMPONENT_REFs of
!    the expression of MODEL to access it.  GSI and INSERT_AFTER have the same
!    meaning as in build_ref_for_offset.  */
  
  static tree
  build_ref_for_model (location_t loc, tree base, HOST_WIDE_INT offset,
  		     struct access *model, gimple_stmt_iterator *gsi,
  		     bool insert_after)
  {
!   tree type = model->type, t;
!   VEC(tree,stack) *cr_stack = NULL;
! 
!   if (TREE_CODE (model->expr) == COMPONENT_REF)
      {
!       tree expr = model->expr;
! 
!       /* Create a stack of the COMPONENT_REFs so later we can walk them in
! 	 order from inner to outer.  */
!       cr_stack = VEC_alloc (tree, stack, 6);
! 
!       do {
! 	tree field = TREE_OPERAND (expr, 1);
! 	tree cr_offset = component_ref_field_offset (expr);
! 	HOST_WIDE_INT bit_pos
! 	  = tree_low_cst (cr_offset, 1) * BITS_PER_UNIT
! 	      + TREE_INT_CST_LOW (DECL_FIELD_BIT_OFFSET (field));
  
! 	/* We can be called with a model different from the one associated
! 	   with BASE so we need to avoid going up the chain too far.  */
! 	if (offset - bit_pos < 0)
! 	  break;
! 
! 	offset -= bit_pos;
! 	VEC_safe_push (tree, stack, cr_stack, expr);
! 
! 	expr = TREE_OPERAND (expr, 0);
! 	type = TREE_TYPE (expr);
!       } while (TREE_CODE (expr) == COMPONENT_REF);
      }
! 
!   t = build_ref_for_offset (loc, base, offset, type, gsi, insert_after);
! 
!   if (TREE_CODE (model->expr) == COMPONENT_REF)
!     {
!       unsigned i;
!       tree expr;
! 
!       /* Now replicate the chain of COMPONENT_REFs from inner to outer.  */
!       FOR_EACH_VEC_ELT_REVERSE (tree, cr_stack, i, expr)
! 	{
! 	  tree field = TREE_OPERAND (expr, 1);
! 	  t = fold_build3_loc (loc, COMPONENT_REF, TREE_TYPE (field), t, field,
! 			       TREE_OPERAND (expr, 2));
! 	}
! 
!       VEC_free (tree, stack, cr_stack);
!     }
! 
!   return t;
  }
  
  /* Construct a memory reference consisting of component_refs and array_refs to
--- 1489,1520 ----
    return fold_build2_loc (loc, MEM_REF, exp_type, base, off);
  }
  
  /* Construct a memory reference to a part of an aggregate BASE at the given
!    OFFSET and of the same type as MODEL.  In case this is a reference to a
!    bit-field, the function will replicate the last component_ref of model's
!    expr to access it.  GSI and INSERT_AFTER have the same meaning as in
!    build_ref_for_offset.  */
  
  static tree
  build_ref_for_model (location_t loc, tree base, HOST_WIDE_INT offset,
  		     struct access *model, gimple_stmt_iterator *gsi,
  		     bool insert_after)
  {
!   if (TREE_CODE (model->expr) == COMPONENT_REF
!       && DECL_BIT_FIELD (TREE_OPERAND (model->expr, 1)))
      {
!       /* This access represents a bit-field.  */
!       tree t, exp_type;
  
!       offset -= int_bit_position (TREE_OPERAND (model->expr, 1));
!       exp_type = TREE_TYPE (TREE_OPERAND (model->expr, 0));
!       t = build_ref_for_offset (loc, base, offset, exp_type, gsi, insert_after);
!       return fold_build3_loc (loc, COMPONENT_REF, model->type, t,
! 			      TREE_OPERAND (model->expr, 1), NULL_TREE);
      }
!   else
!     return build_ref_for_offset (loc, base, offset, model->type,
! 				 gsi, insert_after);
  }
  
  /* Construct a memory reference consisting of component_refs and array_refs to


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