This is the mail archive of the mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: update address taken: don't drop clobbers

On 10/18/14 15:59, Marc Glisse wrote:
On Thu, 10 Jul 2014, Richard Biener wrote:

On Sun, Jun 29, 2014 at 12:33 AM, Marc Glisse <>

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
places where
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.
There are
abnormal PHIs involved. Maybe it shouldn't have insisted on
coalescing an
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
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
(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 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.

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, they're independent.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]