public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Michael Matz <matz@suse.de>
To: Richard Sandiford <richard.sandiford@arm.com>
Cc: Qing Zhao <qing.zhao@oracle.com>,
	 gcc-patches Qing Zhao via <gcc-patches@gcc.gnu.org>,
	 kees cook <keescook@chromium.org>
Subject: Re: HELP!! How to inhibit optimizations applied to .DEFERRED_INIT argument?
Date: Thu, 1 Jul 2021 16:06:33 +0000 (UTC)	[thread overview]
Message-ID: <alpine.LSU.2.20.2107011550280.25557@wotan.suse.de> (raw)
In-Reply-To: <mpto8bmvygl.fsf@arm.com>

Hello,

On Thu, 1 Jul 2021, Richard Sandiford wrote:

> Well, it does feel like this is pressing the reset button on a thread
> whose message count is already in the high double figures.  There's a
> risk that restarting the discussion now is going to repeat a lot of the
> same ground, probably at similar length.  (But I realise that the sheer
> length of the thread is probably also the main reason for not having
> followed it closely. :-))

Yeah, I thought of not sending the mail because of that.  But it itched 
too itchy ;-)

> (2) continue to treat uses of uninitialised automatic variables as
>     (semantically) undefined behaviour
> 
> When this was discussed on the clang list, (2) was a key requirement
> and was instrumental in getting the feature accepted.
> ... 
> since the behaviour is at least deterministic.  But from a debugging QoI
> perspective, this should not happen.  We should generate the same code
> as for:
> 
>    int foo = <fill-pattern>;
>    if (flag)
>      foo = 1;
>    x = foo;
> 
> However, because of (2), we should still generate a 
> -Wmaybe-uninitialized warning for the “x = foo” assignment.
> 
> This is not a combination we support at the moment, so something needs 
> to give.  The question of course is what.

Nothing needs to give.  You add the initialization with <fill-pattern>, 
_without an uninitialized use_, i.e. just:

   foo = .deferred_init(pattern)

or the like.  Then you let the optimizers do their thing.  If 'foo' is 
provably initialized (or rather overwritten in this setting) on all paths 
from above init to each use, that init will be removed.  IOW: if the init 
remains then not all paths are provably initializing.  I.e. the warning 
code can be triggered based on the simple existence of the deferred_init 
initializer.

Basically the .deferred_init acts as a "here the scope starts" marker.  
That's indeed what you want, not the "here the scope ends" clobber.  If a 
use reaches the scope-start marker you have an (possibly) uninitialized 
use and warn.

(I'm obviously missing something because the above idea seems natural, so 
was probably already discussed and rejected, but why?)

> > Garbage in, garbage out.  It makes not too much sense to argue that
> > the generated PHI node on this loop (generated because of the
> > exposed-upward use of foo) is wrong, or right, or should better be
> > something else.  The input program was broken, so anything makes sense.
> 
> Yeah, without the new option it's GIGO.  But I think it's still possible
> to rank different forms of garbage output in terms of how self-consistent
> they are.
> 
> IMO it makes no sense to say that “foo” is upwards-exposed to somewhere
> where “foo” doesn't exist.  Using “foo_N(D)” doesn't have that problem,
> so I think it's possible to say that it's “better” garbage output. :-)
> 
> And the point is that with the new option, this is no longer garbage
> input as far as SSA is concerned: carrying the old value of “foo”
> across iterations would not be implementing the option correctly.
> How SSA handles this becomes a correctness issue.

I disagree.  The notion of "foo doesn't exist" simply doesn't exist (ugh!) 
in SSA; SSA is not the issue here.  The notion of ranges of where foo does 
or doesn't exist are scopes, that's orthogonal; so you want to encode that 
info somehow in a way compatible with SSA (!).  Doing that 
with explicit instructions is sensible (like marking scope endings with 
the clobber insn was sensible).  But those instructions simply need to not 
work against other invariants established and required by SSA.  Don't 
introduce new undefined uses and you're fine.

> > I think it's a chicken egg problem: you can't add undefined uses, for 
> > which you need to know if there was one, but the facility is supposed to 
> > help detecting if there is one to start with.
> 
> The idea is that .DEFERRED_INIT would naturally get optimised away by
> DCE/DSE if the variable is provably initialised before it's used.

Well, that's super.  So, why would you want or need the uninitialized use 
in the argument of .DEFERRED_INIT?


Ciao,
Michael.

  reply	other threads:[~2021-07-01 16:06 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-30  0:14 Qing Zhao
2021-06-30  7:46 ` Richard Biener
2021-06-30 14:04   ` Qing Zhao
2021-06-30 14:39     ` Richard Biener
2021-06-30 15:11       ` Qing Zhao
2021-06-30 15:43       ` Qing Zhao
2021-06-30 17:20         ` Andrew Pinski
2021-06-30 17:36           ` Richard Biener
2021-06-30 18:07             ` Qing Zhao
2021-06-30 18:59               ` Richard Biener
2021-06-30 19:10                 ` Qing Zhao
2021-06-30 19:14                 ` Qing Zhao
2021-06-30 19:20                   ` Andrew Pinski
2021-06-30 19:54                     ` Qing Zhao
2021-07-01  6:48                   ` Richard Biener
2021-07-01 13:45                     ` Qing Zhao
2021-07-01 14:04                       ` Richard Biener
2021-07-01 15:27                         ` Qing Zhao
2021-07-01 14:09                       ` Richard Sandiford
2021-07-01 14:40                         ` Michael Matz
2021-07-01 15:35                           ` Richard Sandiford
2021-07-01 16:06                             ` Michael Matz [this message]
2021-07-01 16:23                               ` Richard Sandiford
2021-07-01 16:35                                 ` Qing Zhao
2021-07-01 15:42                           ` Qing Zhao
2021-07-01 18:14                           ` Richard Biener
2021-07-01 15:32                         ` Qing Zhao
2021-06-30 19:04               ` Qing Zhao
2021-06-30 17:45           ` Qing Zhao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LSU.2.20.2107011550280.25557@wotan.suse.de \
    --to=matz@suse.de \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=keescook@chromium.org \
    --cc=qing.zhao@oracle.com \
    --cc=richard.sandiford@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).