On Thu, 10 Jul 2014, Richard Biener wrote:
On Sun, Jun 29, 2014 at 12:33 AM, Marc Glisse <marc.glisse@inria.fr>
wrote:
we currently drop clobbers on variables whose address is not taken
anymore.
However, rewrite_stmt has code to replace them with an SSA_NAME with a
default definition (an uninitialized variable), and I believe
rewrite_update_stmt should do the same. This allows us to warn sometimes
(see testcase), but during the debugging I also noticed several
places where
it allowed CCP to simplify further PHIs, so this is also an
optimization.
In an earlier version of the patch, I was using
get_or_create_ssa_default_def (cfun, sym);
(I was reusing the same variable). This passed bootstrap+testsuite on
all
languages except for ada. Indeed, the compiler wanted to coalesce
several
SSA_NAMEs, including those new ones, in out-of-ssa, but couldn't.
There are
abnormal PHIs involved. Maybe it shouldn't have insisted on
coalescing an
undefined ssa_name, maybe something should have prevented us from
reaching
such a situation, but creating a new variable was the simplest
workaround.
Hmm. We indeed notice "late" that the new names are used in abnormal
PHIs. Note that usually rewriting a symbol into SSA form does not
create overlapping life-ranges - but of course you are possibly
introducing
those by the new use of the default definitions.
Apart from the out-of-SSA patch you proposed elsewhere a possibility
is to simply never mark undefined SSA names as
SSA_NAME_OCCURS_IN_ABNORMAL_PHI ... (or not mark those
as must-coalesce).
For "not mark those as must-coalesce", replacing the liveness patch with
the attached patch also passed the testsuite: I skip undefined variables
when handling must-coalesce, and let the regular coalescing code handle
them. I am not sure what happens during expansion though, and bootstrap
only hits this issue a couple times in ada so it doesn't prove much.
This patch doesn't conflict with the liveness patch, they are rather
complementary. I didn't test but I am quite confident that having both
patches would also pass bootstrap+testsuite.
Of course that all becomes unnecessary if we use default definitions of
new variables instead of always the same old variable, but I can
understand not wanting all those new artificial variables.
I would be ok with the patch at
https://gcc.gnu.org/ml/gcc-patches/2014-10/msg01787.html
(minus the line with unlink_stmt_vdef, which is indeed unnecessary)
(I looked at what gsi_replace does when replacing a clobber by a nop,
and it seems harmless, but I can manually inline the non-dead parts of
it if you want)