[Bug debug/108656] [12/13 Regression] '-fcompare-debug' failure (length) w/ -O2 -fno-ipa-pure-const -fno-tree-dce --param early-inlining-insns=0 since r12-5236
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108656 --- Comment #13 from rguenther at suse dot de --- On Mon, 6 Feb 2023, jakub at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108656 > > --- Comment #12 from Jakub Jelinek --- > (In reply to rguent...@suse.de from comment #11) > > On Mon, 6 Feb 2023, jakub at gcc dot gnu.org wrote: > > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108656 > > > > > > --- Comment #10 from Jakub Jelinek --- > > > Anyway, if we decided that it is ok to have just the incoming ab edges, > > > we'd > > > need to change any code that can DCE or inline calls to update abnormal > > > edges > > > not just for the case where the last stmt used to be > > > can_make_abnormal_goto and > > > no longer is, but also for the case when the first stmt was returns_twice > > > call > > > and no longer is, in that case we'd need to drop that abnormal edge > > > pointing to > > > the start of the bb from the .ABNORMAL_DISPATCHER block. > > > > Doesn't cleanup CFG do this via removal of the unreachable > > .ABNORMAL_DISPATCHER? > > If all non-pure calls are removed (all can_make_abnormal_goto to be precise), > sure. Or if we reset and recompute cfun->calls_setjmp and then do cfg > cleanup. > The problem on the above testcase is that it can never happen or takes many > passes. > The returns_twice function is gone during einline pass, but there are other > calls that can make abnormal goto (well, have side-effects and nothing noticed > we don't have any returns_twice calls anymore). So we have an ab edge > pointing > to a bb that doesn't start with returns_twice function, that alone looks wrong > to me. During IPA we then inline further functions and diverge on when we > trigger the removal on outgoing abnormal edges because of debug stmts. I've fixed a lot of issues like this, even implemented a verifier, but that's still too trigger happy. So yes, there are some issues left and the only hard assert I've put in is the DCE one that makes sure abnormal dispatchers are not discovered "late".
[Bug debug/108656] [12/13 Regression] '-fcompare-debug' failure (length) w/ -O2 -fno-ipa-pure-const -fno-tree-dce --param early-inlining-insns=0 since r12-5236
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108656 --- Comment #12 from Jakub Jelinek --- (In reply to rguent...@suse.de from comment #11) > On Mon, 6 Feb 2023, jakub at gcc dot gnu.org wrote: > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108656 > > > > --- Comment #10 from Jakub Jelinek --- > > Anyway, if we decided that it is ok to have just the incoming ab edges, we'd > > need to change any code that can DCE or inline calls to update abnormal > > edges > > not just for the case where the last stmt used to be can_make_abnormal_goto > > and > > no longer is, but also for the case when the first stmt was returns_twice > > call > > and no longer is, in that case we'd need to drop that abnormal edge > > pointing to > > the start of the bb from the .ABNORMAL_DISPATCHER block. > > Doesn't cleanup CFG do this via removal of the unreachable > .ABNORMAL_DISPATCHER? If all non-pure calls are removed (all can_make_abnormal_goto to be precise), sure. Or if we reset and recompute cfun->calls_setjmp and then do cfg cleanup. The problem on the above testcase is that it can never happen or takes many passes. The returns_twice function is gone during einline pass, but there are other calls that can make abnormal goto (well, have side-effects and nothing noticed we don't have any returns_twice calls anymore). So we have an ab edge pointing to a bb that doesn't start with returns_twice function, that alone looks wrong to me. During IPA we then inline further functions and diverge on when we trigger the removal on outgoing abnormal edges because of debug stmts.
[Bug debug/108656] [12/13 Regression] '-fcompare-debug' failure (length) w/ -O2 -fno-ipa-pure-const -fno-tree-dce --param early-inlining-insns=0 since r12-5236
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108656 --- Comment #11 from rguenther at suse dot de --- On Mon, 6 Feb 2023, jakub at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108656 > > --- Comment #10 from Jakub Jelinek --- > Anyway, if we decided that it is ok to have just the incoming ab edges, we'd > need to change any code that can DCE or inline calls to update abnormal edges > not just for the case where the last stmt used to be can_make_abnormal_goto > and > no longer is, but also for the case when the first stmt was returns_twice call > and no longer is, in that case we'd need to drop that abnormal edge pointing > to > the start of the bb from the .ABNORMAL_DISPATCHER block. Doesn't cleanup CFG do this via removal of the unreachable .ABNORMAL_DISPATCHER?
[Bug debug/108656] [12/13 Regression] '-fcompare-debug' failure (length) w/ -O2 -fno-ipa-pure-const -fno-tree-dce --param early-inlining-insns=0 since r12-5236
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108656 --- Comment #10 from Jakub Jelinek --- Anyway, if we decided that it is ok to have just the incoming ab edges, we'd need to change any code that can DCE or inline calls to update abnormal edges not just for the case where the last stmt used to be can_make_abnormal_goto and no longer is, but also for the case when the first stmt was returns_twice call and no longer is, in that case we'd need to drop that abnormal edge pointing to the start of the bb from the .ABNORMAL_DISPATCHER block.
[Bug debug/108656] [12/13 Regression] '-fcompare-debug' failure (length) w/ -O2 -fno-ipa-pure-const -fno-tree-dce --param early-inlining-insns=0 since r12-5236
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108656 --- Comment #9 from Jakub Jelinek --- In the setjmp/longjmp case obviously you need some other function that will do the longjmp, on the other side setjmp as pure makes no sense because setjmp has to remember pc/sp etc. in some jump buffer, so it isn't pure. Ditto getcontext etc. But returns_twice isn't just setjmp, it is also fork/vfork or anything else that can arrange multiple returns through other means.
[Bug debug/108656] [12/13 Regression] '-fcompare-debug' failure (length) w/ -O2 -fno-ipa-pure-const -fno-tree-dce --param early-inlining-insns=0 since r12-5236
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108656
--- Comment #8 from Richard Biener ---
(In reply to Jakub Jelinek from comment #7)
> (In reply to Richard Biener from comment #6)
> > (In reply to Jakub Jelinek from comment #5)
> > > Created attachment 54412 [details]
> > > gcc13-pr108656.patch
> > >
> > > So shall we fix it like this then?
> >
> > But isn't this the wrong "side"? returns_twice means it is the abnormal
> > control _receiver_, it doesn't perform an abnormal goto itself.
> >
> > stmt_starts_bb_p is correct here, so where does it go wrong?
>
> I think it needs both. The thing is, when it returns the second time, it
> does that again by returning from itself, not through returning from some
> unrelated function.
> Say, if we have pure + returns_twice call and no other call in a function,
> the abnormal edge to the pure + returns_twice call would be optimized away,
> even when the call remains, because there would be no edge from some call to
> the .ABNORMAL_DISPATCHER block.
> __attribute__((pure, returns_twice)) int foo (void);
>
> int
> bar (void)
> {
> for (int i = 0; i < 64; ++i)
> {
> int x = foo ();
> if (x == 26)
> return -42;
> }
> return 42;
> }
> doesn't even have because of this any abnormal edges created.
> Or, if there is some other non-pure call somewhere else, we model through
> the abnormal
> edges that that other call can pass control back to the start of the
> returns_twice call to make it return again.
Ah, but then the issue is that we assume that 'foo' doesn't longjmp,
independent on whether it is returns_twice or not? Can a setjmp ()
function perform a longjmp () to its own context? Would it even appear
as returning twice then? Would calling setjmp in a loop like above
and then jumping to another iteration via longjmp even be valid?
That said, for your example I would think not having any abnormal edges
is correct - there's no frame that could transfer control back to the
returns_twice receiver, no?
[Bug debug/108656] [12/13 Regression] '-fcompare-debug' failure (length) w/ -O2 -fno-ipa-pure-const -fno-tree-dce --param early-inlining-insns=0 since r12-5236
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108656
--- Comment #7 from Jakub Jelinek ---
(In reply to Richard Biener from comment #6)
> (In reply to Jakub Jelinek from comment #5)
> > Created attachment 54412 [details]
> > gcc13-pr108656.patch
> >
> > So shall we fix it like this then?
>
> But isn't this the wrong "side"? returns_twice means it is the abnormal
> control _receiver_, it doesn't perform an abnormal goto itself.
>
> stmt_starts_bb_p is correct here, so where does it go wrong?
I think it needs both. The thing is, when it returns the second time, it does
that again by returning from itself, not through returning from some unrelated
function.
Say, if we have pure + returns_twice call and no other call in a function, the
abnormal edge to the pure + returns_twice call would be optimized away, even
when the call remains, because there would be no edge from some call to the
.ABNORMAL_DISPATCHER block.
__attribute__((pure, returns_twice)) int foo (void);
int
bar (void)
{
for (int i = 0; i < 64; ++i)
{
int x = foo ();
if (x == 26)
return -42;
}
return 42;
}
doesn't even have because of this any abnormal edges created.
Or, if there is some other non-pure call somewhere else, we model through the
abnormal
edges that that other call can pass control back to the start of the
returns_twice call to make it return again.
[Bug debug/108656] [12/13 Regression] '-fcompare-debug' failure (length) w/ -O2 -fno-ipa-pure-const -fno-tree-dce --param early-inlining-insns=0 since r12-5236
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108656 --- Comment #6 from Richard Biener --- (In reply to Jakub Jelinek from comment #5) > Created attachment 54412 [details] > gcc13-pr108656.patch > > So shall we fix it like this then? But isn't this the wrong "side"? returns_twice means it is the abnormal control _receiver_, it doesn't perform an abnormal goto itself. stmt_starts_bb_p is correct here, so where does it go wrong?
[Bug debug/108656] [12/13 Regression] '-fcompare-debug' failure (length) w/ -O2 -fno-ipa-pure-const -fno-tree-dce --param early-inlining-insns=0 since r12-5236
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108656 --- Comment #5 from Jakub Jelinek --- Created attachment 54412 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54412=edit gcc13-pr108656.patch So shall we fix it like this then?
[Bug debug/108656] [12/13 Regression] '-fcompare-debug' failure (length) w/ -O2 -fno-ipa-pure-const -fno-tree-dce --param early-inlining-insns=0 since r12-5236
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108656 --- Comment #4 from Richard Biener --- In general we avoid disallowing attribute combinations that at least in theory make sense. pure/const are about memory side-effects while returns_twice is about control flow, so in this regard I don't see how they should conflict. That we exclude !gimple_has_side_effects from call_can_make_abnormal_goto is somewhat of a chicken-and-egg thing - "side effect" is something not explicitely encoded in the IL but a returns_twice results in explicit encoding via abnormal edges. But then we cannot use that for CFG construction.
[Bug debug/108656] [12/13 Regression] '-fcompare-debug' failure (length) w/ -O2 -fno-ipa-pure-const -fno-tree-dce --param early-inlining-insns=0 since r12-5236
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108656 Jakub Jelinek changed: What|Removed |Added CC||hubicka at gcc dot gnu.org, ||rguenth at gcc dot gnu.org --- Comment #3 from Jakub Jelinek --- I think the problem is in the bogus combination of attributes, pure (or const) and returns_twice. For returns_twice we force the call to be the first statement in a bb (stmt_starts_bb_p returns true for it) with an AB edge to it and expect it to be the last one as well with an AB edge out of it. For normal returns_twice calls that is the case, the fact that there is returns_twice results in cfun->calls_setjmp and in that case call_can_make_abnormal_goto will be true (so among others the call is marked as ctrl altering and eventually stmt_ends_bb_p). But if returns_twice is mixed with const, pure or leaf attributes, this all falls apart. The exact reason for the -fcompare-debug failure is that we have the weird ;; basic block 4, loop depth 0, maybe hot ;;prev block 3, next block 5, flags: (NEW) ;;pred: 3 (FALLTHRU) ;;5 (ABNORMAL,DFS_BACK) # _5(ab) = PHI <_12(3), _6(ab)(5)> _2 = bar (); _14 = corge (_2, _5(ab)); goto ; [INV] ;;succ: 6 (FALLTHRU) ;;5 (ABNORMAL) pre-einline (note, corge isn't pure/const/leaf and so unlike bar is assumed to maybe doing abnormal goto), einline turns it into: ;; basic block 3, loop depth 0, maybe hot ;;prev block 2, next block 4, flags: (NEW, REACHABLE) ;;pred: 2 (FALLTHRU) ;;4 (ABNORMAL,DFS_BACK) # _5(ab) = PHI <_12(2), _6(ab)(4)> # DEBUG BEGIN_STMT baz (); # DEBUG BEGIN_STMT _19 = 0; _2 = _19; _14 = corge (_2, _5(ab)); goto ; [INV] ;;succ: 5 (FALLTHRU) ;;4 (ABNORMAL) (with the debug stmts missing for -g0), no returns_twice nor anything else actually needing abnormal stuff in the cfg, but bar wasn't the last stmt in bb, so no purging was done. Later on, expand_calL_inline is called on the bb which has just ;; basic block 3, loop depth 0, maybe hot ;;prev block 2, next block 4, flags: (NEW, REACHABLE) ;;pred: 2 (FALLTHRU) ;;4 (ABNORMAL,DFS_BACK) # _5(ab) = PHI <_12(2), _6(ab)(4)> # DEBUG BEGIN_STMT baz (); # DEBUG BEGIN_STMT goto ; [INV] ;;succ: 5 (FALLTHRU) ;;4 (ABNORMAL) and return_block is after splitting in one case just baz (); call and in another case baz () call + debug stmt. /* If the GIMPLE_CALL was in the last statement of BB, it may have been the source of abnormal edges. In this case, schedule the removal of dead abnormal edges. */ gsi = gsi_start_bb (return_block); gsi_next (); purge_dead_abnormal_edges = gsi_end_p (gsi); So, purge_dead_abnormal_edges is set for -g0 but not -g and as after inlining baz there is nothing that would need an abnormal edge, we drop .ABNORMAL_DISPATCHER in the former case and not the latter. I wonder if we shouldn't reject mixing const/pure/leaf attributes with returns_twice, like we already warn and ignore returns_twice attribute when mixed with noreturn. Or, I think we'd need to make sure returns_twice results in call_can_make_abnormal_goto true, and perhaps stmt_can_terminate_bb_p true too. Though, still I wonder what will happen if we DCE the returns_twice calls, wonder if we at least drop the abnormal successor edges then.
[Bug debug/108656] [12/13 Regression] '-fcompare-debug' failure (length) w/ -O2 -fno-ipa-pure-const -fno-tree-dce --param early-inlining-insns=0 since r12-5236
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108656 Jakub Jelinek changed: What|Removed |Added Priority|P3 |P2 Last reconfirmed||2023-02-03 CC||jakub at gcc dot gnu.org Summary|[12/13 Regression] |[12/13 Regression] |'-fcompare-debug' failure |'-fcompare-debug' failure |(length) w/ -O2 |(length) w/ -O2 |-fno-ipa-pure-const |-fno-ipa-pure-const |-fno-tree-dce --param |-fno-tree-dce --param |early-inlining-insns=0 |early-inlining-insns=0 ||since r12-5236 Target Milestone|--- |12.3 Status|UNCONFIRMED |NEW Ever confirmed|0 |1 --- Comment #2 from Jakub Jelinek --- Started with r12-5236-g5aa91072e24c1e16a5e .
[Bug debug/108656] [12/13 Regression] '-fcompare-debug' failure (length) w/ -O2 -fno-ipa-pure-const -fno-tree-dce --param early-inlining-insns=0
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108656 --- Comment #1 from Arseny Solokha --- The snapshot date is of course wrong. It should read "gcc 13.0.1 20230119 snapshot (g:2e32a12c04c72f692a7bd119fd3e4e5b74392c9d)".
