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.