From: Jeff Law <jeffreyalaw@gmail.com>
To: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH 2/3] tree-cfg: do not duplicate returns_twice calls
Date: Wed, 13 Jul 2022 10:01:02 -0600 [thread overview]
Message-ID: <1e6ee886-7fc6-494a-9216-ba6b5855a88a@gmail.com> (raw)
In-Reply-To: <CAFiYyc2He7G-L2WF0zoHeSEd1fHaHVoi3oV0CbRffSpV5Yug2A@mail.gmail.com>
On 7/13/2022 1:13 AM, Richard Biener via Gcc-patches wrote:
> On Tue, Jul 12, 2022 at 10:10 PM Alexander Monakov <amonakov@ispras.ru> wrote:
>>
>> Apologies for the prolonged silence Richard, it is a bit of an obscure topic,
>> and I was unsure I'd be able to handle any complications in a timely manner.
>> I'm ready to revisit it now, please see below.
>>
>> On Mon, 17 Jan 2022, Richard Biener wrote:
>>
>>> On Fri, Jan 14, 2022 at 7:21 PM Alexander Monakov <amonakov@ispras.ru> wrote:
>>>> A returns_twice call may have associated abnormal edges that correspond
>>>> to the "second return" from the call. If the call is duplicated, the
>>>> copies of those edges also need to be abnormal, but e.g. tracer does not
>>>> enforce that. Just prohibit the (unlikely to be useful) duplication.
>>> The general CFG copying routines properly duplicate those edges, no?
>> No (in fact you say so in the next paragraph). In general I think they cannot,
>> abnormal edges are a special case, so it should be the responsibility of the
>> caller.
>>
>>> Tracer uses duplicate_block so it should also get copies of all successor
>>> edges of that block. It also only traces along normal edges. What it might
>>> miss is abnormal incoming edges - is that what you are referring to?
>> Yes (I think its entire point is to build a "trace" of duplicated blocks that
>> does not have incoming edges in the middle, abnormal or not).
>>
>>> That would be a thing we don't handle in duplicate_block on its own but
>>> that callers are expected to do (though I don't see copy_bbs doing that
>>> either). I wonder if we can trigger this issue for some testcase?
>> Oh yes (in fact my desire to find a testcase delayed this quite a bit).
>> When compiling the following testcase with -O2 -ftracer:
>>
>> __attribute__((returns_twice))
>> int rtwice_a(int), rtwice_b(int);
>>
>> int f(int *x)
>> {
>> volatile unsigned k, i = (*x);
>>
>> for (k = 1; (i = rtwice_a(i)) * k; k = 2);
>>
>> for (; (i = rtwice_b(i)) * k; k = 4);
>>
>> return k;
>> }
>>
>> tracer manages to eliminate the ABNORMAL_DISPATCHER block completely, so
>> the possibility of transferring control back to rtwice_a from rtwice_b
>> is no longer modeled in the IR. I could spend some time "upgrading" this
>> to an end-to-end miscompilation, but I hope you agree this is quite broken
>> already.
>>
>>> The thing to check would be incoming abnormal edges in
>>> can_duplicate_block_p, not (only) returns twice functions?
>> Unfortunately not, abnormal edges are also used for computed gotos, which are
>> less magic than returns_twice edges and should not block tracer I think.
> I think computed gotos should use regular edges, only non-local goto should
> use abnormals...
>
> I suppose asm goto also uses abnormal edges?
>
> Btw, I don't see how they in general are "less magic". Sure, we have an
> explicit receiver (the destination label), but we can only do edge inserts
> if we have a single computed goto edge into a block (we can "move" the
> label to the block created when splitting the edge).
I suspect treating them like abnormals probably came from the inability
to reliably split them way back when we introduced RTL GCSE and the like.
Jeff
next prev parent reply other threads:[~2022-07-13 16:01 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-13 14:25 [RFC PATCH] tree-ssa-sink: do not sink to in front of setjmp Alexander Monakov
2021-12-13 14:45 ` Richard Biener
2021-12-13 15:20 ` Alexander Monakov
2021-12-14 11:10 ` Алексей Нурмухаметов
2022-01-03 13:41 ` Richard Biener
2022-01-03 16:35 ` Alexander Monakov
2022-01-04 7:25 ` Richard Biener
2022-01-14 18:20 ` Alexander Monakov
2022-01-14 18:20 ` [PATCH 1/3] " Alexander Monakov
2022-01-17 7:47 ` Richard Biener
2023-11-08 9:04 ` Florian Weimer
2023-11-08 10:01 ` Richard Biener
2023-11-08 13:06 ` Alexander Monakov
2022-01-14 18:20 ` [PATCH 2/3] tree-cfg: do not duplicate returns_twice calls Alexander Monakov
2022-01-17 8:08 ` Richard Biener
2022-07-12 20:10 ` Alexander Monakov
2022-07-13 7:13 ` Richard Biener
2022-07-13 14:57 ` Alexander Monakov
2022-07-14 6:38 ` Richard Biener
2022-07-14 20:12 ` Alexander Monakov
2022-07-19 8:40 ` Richard Biener
2022-07-19 20:00 ` Alexander Monakov
2022-07-13 16:01 ` Jeff Law [this message]
2022-01-14 18:20 ` [PATCH 3/3] tree-cfg: check placement of " Alexander Monakov
2022-01-17 8:12 ` Richard Biener
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=1e6ee886-7fc6-494a-9216-ba6b5855a88a@gmail.com \
--to=jeffreyalaw@gmail.com \
--cc=gcc-patches@gcc.gnu.org \
/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).