Re: [patch] Minor adjustment to gimplify_addr_expr

[Richard Biener <richard dot guenther at gmail dot com>]

On Wed, Oct 14, 2015 at 5:59 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> Hi,
>
> this is the regression of ACATS c37213k at -O2 with an upcoming change in
> the front-end of the Ada compiler:
>
> eric@polaris:~/gnat/gnat-head/native> gcc/gnat1 -quiet c37213k.adb -I
> /home/eric/gnat/bugs/support -O2
> +===========================GNAT BUG DETECTED==============================+
> | Pro 7.4.0w (20151014-60) (x86_64-suse-linux) GCC error:                  |
> | tree check: expected class 'expression', have              |
> |     'exceptional' (ssa_name) in tree_operand_check, at tree.h:3431|
> | Error detected around c37213k.adb:95:37

Btw, would be really nice to have libbacktrace support for ada ...

> It's recompute_tree_invariant_for_addr_expr receiving an SSA_NAME instead of
> an ADDR_EXPR when called from gimplify_addr_expr.  The sequence is as follows:
> we start with this GIMPLE statement:
>
>   *R.43_60 = c37213k.B_1.B_6.proc6.B_4.B_5.value (); [static-chain: &FRAME.60]
> [return slot optimization]
>
> Then IPA clones the function and turns the statement into:
>
>   MEM[(struct c37213k__B_1__B_6__proc6__B_4__nrec *)R.43_46] =
> c37213k.B_1.B_6.proc6.B_4.B_5.value (); [static-chain: &FRAME.60] [return slot
> optimization]
>
> The 'value' function has been NRVed and contains:
>
>   .builtin_memcpy (&<retval>, _9, _10);
>
> and gets inlined so the above statement is rewritten into:
>
> .builtin_memcpy (&MEM[(struct c37213k__B_1__B_6__proc6__B_4__nrec *)R.43_46],
> _174, _175);
>
> so gimplify_addr_expr is invoked on:
>
>   &MEM[(struct c37213k__B_1__B_6__proc6__B_4__nrec *)R.43_46]
>
> and gets confused because it doesn't see that it's just R.43_46 (it would have
> seen it if the original INDIRECT_REF was still present in lieu of MEM_REF).
>
> Hence the attached fixlet.  Tested on x86_64-suse-linux, OK for the mainline?

While the patch looks technically ok I think you'll run into the same issue with
a non-zero offset MEM_REF as that will get you a POINTER_PLUS_EXPR
from build_fold_addr_expr.  We might be lucky not to ICE in
recompute_tree_invariant_for_addr_expr because we can access operand
zero of that of course.  I think recompute_tree_invariant_for_addr_expr
misses an assert that it receives an ADDR_EXPR and the gimplify.c
caller would need to handle POINTER_PLUS_EXPR specially.

Or change your patch to also handle non-zero offset MEM_REFs by
simply gimplifying to POINTER_PLUS_EXPR op0, op1.

I'd prefer the latter.

Thanks,
Richard.

>
> 2015-10-14  Eric Botcazou  <ebotcazou@adacore.com>
>
>         * gimplify.c (gimplify_addr_expr) <MEM_REF>: New case.
>
> --
> Eric Botcazou