Re: [PATCH] Fix PR middle-end/17746

[Eric Botcazou <ebotcazou at libertysurf dot fr>]

> Then the above is not valid!  handled_component_p and get_inner_reference
> must always agree on what's handled.  The bug is in handled_component_p.

Very interesting.  You added the supplementary check for VIEW_CONVERT_EXPR 
without updating handled_component_p.  For the FSF tree:
http://gcc.gnu.org/ml/gcc-patches/2003-05/msg00396.html

For GNAT 5.x:

2002-10-16  Richard Kenner  <kenner@vlsi1.ultra.nyu.edu>

	* expr.c (get_inner_reference): Don't go through a VIEW_CONVERT_EXPR
	whose purpose is to step up the alignment.

I can't imagine this is an oversight.  If it is, all the GNAT 5.x compilers 
are affected by the "bug".


Here's my take on this.  My initial reaction when investigating the problem 
was similar to yours: why on Earth is there this obvious gap between 
get_inner_reference and handled_component_p?  I found your patch and 
immediately added the missing bits to handled_component_p.  Then things 
started to break because is_gimple_addressable now returns false for this 
kind of VIEW_CONVERT_EXPR.

I came to the conclusion that returning true for them in handled_component_p 
is a good solution (and that you probably came to it too).  They are handled 
like the other VIEW_CONVERT_EXPRs except that, when they are expanded, an 
additional step is performed:

expand_expr:

  case VIEW_CONVERT_EXPR:
      op0 = expand_expr (TREE_OPERAND (exp, 0), NULL_RTX, mode, modifier);

    [...]

     /* At this point, OP0 is in the correct mode.  If the output type is such
	 that the operand is known to be aligned, indicate that it is.
	 Otherwise, we need only be concerned about alignment for non-BLKmode
	 results.  */
      if (MEM_P (op0))
	{
	  op0 = copy_rtx (op0);

	  if (TYPE_ALIGN_OK (type))
	    set_mem_align (op0, MAX (MEM_ALIGN (op0), TYPE_ALIGN (type)));

that is, the super-alignment is recorded.


This has worked fine up to 3.4.x because ADDR_EXPR was handled by recursive 
calls to expand_expr.  For 4.0, ADDR_EXPR is handled by the specialized 
expand_expr_addr_expr, which doesn't invoke expand_expr anymore.  My patch 
simply lets expand_expr_addr_expr be aware of these VIEW_CONVERT_EXPRs.

>     The proposed fix is to teach expand_expr_addr_expr_1 to look itself
>     through the VIEW_CONVERT_EXPR nodes that "step up" the alignment, but
>     only if they carry the TYPE_ALIGN_OK flag.
>
> In that case get_inner_reference should *also* look at TYPE_ALIGN_OK,
> as should handled_component_p.

get_inner_reference already looks at TYPE_ALIGN_OK and actually rejects 
VIEW_CONVERT_EXPR because of it.

-- 
Eric Botcazou