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
