Re: SRA: don't drop clobbers

[Martin Jambor <mjambor at suse dot cz>]

Hi,

On Mon, Nov 03, 2014 at 01:59:24PM +0100, Marc Glisse wrote:
> Hello,
> 
> now that the update_address_taken patch is in, let me re-post the
> SRA follow-up. With this patch, testcase pr60517.C (attached) has a
> use of an undefined variable at the time of the uninit pass. Sadly,
> while this warned with earlier versions of the other patch (when I
> was inserting default definitions of new variables), it does not
> anymore because we have TREE_NO_WARNING all over the place. Still, I
> believe this is a step in the right direction, and it passed
> bootstrap+testsuite (minus the new testcase).

I am not sure what you mean by "minus the new testcase" but, with the
gimple checking compiler at least, the testcase produces an ICE:

  pr60517.C: In function âdouble foo(A)â:
  pr60517.C:15:1: error: non-decl/MEM_REF LHS in clobber statement
   }
   ^
  SR.1_8
  SR.1_8 ={v} {CLOBBER};
  pr60517.C:15:1: internal compiler error: verify_gimple failed
  0xc7810d verify_gimple_in_cfg(function*, bool)
          /home/mjambor/gcc/mine/src/gcc/tree-cfg.c:5039
  0xb8256f execute_function_todo
          /home/mjambor/gcc/mine/src/gcc/passes.c:1758
  0xb83183 execute_todo
          /home/mjambor/gcc/mine/src/gcc/passes.c:1815

The problem is exactly what the verifier complains about, a clobber of
an SSA_NAME which I agree with the verifier is a really bad idea.

If you want SRA to produce information that at a given point a scalar
replacement (which will become an SSA_NAME) does not have any valid
data, the way to represent that is to load it with a value from a
default-definition SSA name.  Something along the lines of
get_repl_default_def_ssa_name which unfortunately you cannot directly
use without confusing the SSA renamer.

The easiest way, at least to test a proof of concept, is probably to
create another declaration of the same type (reusing bits from
create_access_replacement, especially setting the same
DECL_DEBUG_EXPR), get its default-definition using
get_or_create_ssa_default_def and then assign that to the replacement
instead of trying to clobber it.

(A more correct approach would be to design a way of telling the
renamer that at this point the scalar becomes undefined and that it
should start using default-defs until the next definition appears but
that is likely to be difficult, although that is just my guess, it
might be easy.)

> Would it be ok to commit the tree-sra.c change?

I don't think so, sorry about the bad news.

Martin