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: [PING] tighten gimple addressability checks


[Thanks for your feedback]

Richard Guenther wrote:
> So, why are there ADDR_EXPRs of non-addressable variables at all? 

 Because the gimplifier takes them for misc purposes and that's what the patch
 is trying to address.

 The initial case (with the patch submission) was gimplify_modify_expr
 turning an assignment into a call to memcpy or memset without looking too much
 (and part of the patch changes that).

 In the other case I posted (the one with a comparison), the comparison is turned
 into a memcmp from

      gimplify_expr (tree *expr_p, tree *pre_p, tree *post_p,
      ...
	    case tcc_comparison:
	      /* Handle comparison of objects of non scalar mode aggregates
	     	 with a call to memcmp.  [...]

> I think in the end we want the frontend to sort out what is addressable and what not
> (and we have TREE_ADDRESSABLE for that). 

> Do you say, that an outer component a.b is not addressable, but a.b.c is?

 Hummm, not really. 

 Suppose you have an 'int' at byte position 1 in a struct. What I was saying (or
 trying to say) is that

   it is not generally safe to take and use the address of that 'int', because the
   address is likely not aligned in accordance with the type,

 but

   it's ok in specific circumstances where this sort of misalignment doesn't matter,

 and

   using the address to perform a block oriented operation (like memcpy or memcmp)
   is one such specific circumstance.
 
> Is the usage of memcmp appreciated here?  Is it even correct in general?!

 We haven't introduced it so I don't know for sure. It's now only issued for
 BLKmode and that looks reasonable because we know the operands are at least byte
 aligned.

 It used to happen for integral modes as well and that was causing trouble (was
 inefficient at best and blowing up for bit-aligned operands).  This is one of the
 former changes we made.

> Maybe the problem is "DECL_BIT_FIELD set on STRICT_ALIGNMENT targets"
>  - I don't think the C frontend does this for packed structures.

 I'm not sure what you mean here.

> we might have to recurse here and look for DECL_FIELD_BIT_OFFSET
> (or rather, TREE_ADDRESSABLE on the FIELD_DECLs).
 
 Humm, maybe. Here's how our attempt came along (roughly):

 Whether it's ok to take the address depends on how this address is
 going to be used. In the general case we don't know, but we do know
 when we are doing so from the gimplifier and can provide a useful hint
 about it.

 This is what BLOCK_ADDRESS_P is about in the patch: to state that this ADDR_EXPR
 is formed to be used as an argument for of a block operation like memcpy, hence
 that byte alignment of the operand is good enough.

 The suggested is_gimple_addressable_1 predicate is intended as a conservative
 approximation of this property. It bails out (claims not addressable) fairly
 easily, a compromise to be precise enough in practice, while remaining simple
 and stopping the recursion more rapidly.

 I first thought of using get_inner_reference, and then that DECL_BIT_FIELD is a
 fairly precise hint already.

> >  we get down to expand_expr_real_1 with 'exp' like
...
> >  and it works just fine.
> 
> I see.  I guess the fact that we cannot emulate this in C means that
> occasional bugs in tree optimizers will sneak in.  Does the Ada testsuite
> provide good coverage of those corner-cases?

 It's certainly not exhaustive. We're adding the cases as we get the patches in.

 Olivier


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