This is the mail archive of the
mailing list for the GCC project.
Re: update address taken: don't drop clobbers
- From: Jeff Law <law at redhat dot com>
- To: Marc Glisse <marc dot glisse at inria dot fr>, Richard Biener <richard dot guenther at gmail dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Fri, 24 Oct 2014 14:18:03 -0600
- Subject: Re: update address taken: don't drop clobbers
- Authentication-results: sourceware.org; auth=none
- References: <alpine dot DEB dot 2 dot 02 dot 1406282350110 dot 31815 at stedding dot saclay dot inria dot fr> <CAFiYyc0fRhV09A3C2WT8yQ1ndp9dcyWntCVSPHzhwHb3tgNZLg at mail dot gmail dot com> <alpine dot DEB dot 2 dot 11 dot 1410182108070 dot 8744 at laptop-mg dot saclay dot inria dot fr>
On 10/18/14 15:59, Marc Glisse wrote:
So I'm still trying to get comfortable with this patch. I guess my
concerns about having one of the undefined value SSA_NAMEs appearing in
two conflicting coalesce lists are alleviated by the twiddle to
coalesce_partitions where we essentially ignore them.
On Thu, 10 Jul 2014, Richard Biener wrote:
On Sun, Jun 29, 2014 at 12:33 AM, Marc Glisse <firstname.lastname@example.org>
we currently drop clobbers on variables whose address is not taken
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
it allowed CCP to simplify further PHIs, so this is also an
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
languages except for ada. Indeed, the compiler wanted to coalesce
SSA_NAMEs, including those new ones, in out-of-ssa, but couldn't.
abnormal PHIs involved. Maybe it shouldn't have insisted on
undefined ssa_name, maybe something should have prevented us from
such a situation, but creating a new variable was the simplest
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
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
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
(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)
So in the end, they don't end up a part of any partition? What happens
when we expand them? I guess they get a new pseudo since they're a
distinct partition? If we had a sensible story for expansion, then I
could probably get on board with this patch.
And I'm still going to look at the other as well -- as you mention,