* [patch] Fix PR rtl-optimization/100411
@ 2021-05-05 11:00 Eric Botcazou
2021-05-05 11:10 ` Jakub Jelinek
0 siblings, 1 reply; 6+ messages in thread
From: Eric Botcazou @ 2021-05-05 11:00 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 936 bytes --]
Hi,
this is the bootstrap failure of GCC 11 on MinGW64 configured with --enable-
tune=nocona. The bottom line is that SEH does not support CFI for epilogues
but the x86 back-end nevertheless attaches it to instructions, so we have to
filter it out and this is done by detecting the end of the prologue by means
of the NOTE_INSN_PROLOGUE_END note.
But the compiler manages to generate a second epilogue before this note in the
RTL stream and this fools the above logic. The root cause is cross-jumping,
which inserts a jump before the end of the prologue (in fact just before the
note); the rest (CFG cleanup, BB reordering) is downhill from there.
Tested on x86-64/Linux and x86-64/Windows, OK for mainline and 11 branch?
2021-05-05 Eric Botcazou <ebotcazou@adacore.com>
PR rtl-optimization/100411
* cfgcleanup.c (try_crossjump_to_edge): Also skip end of prologue
and beginning of function markers.
--
Eric Botcazou
[-- Attachment #2: pr100411.diff --]
[-- Type: text/x-patch, Size: 845 bytes --]
diff --git a/gcc/cfgcleanup.c b/gcc/cfgcleanup.c
index f05cb6136c7..64279cc8c20 100644
--- a/gcc/cfgcleanup.c
+++ b/gcc/cfgcleanup.c
@@ -2148,6 +2148,20 @@ try_crossjump_to_edge (int mode, edge e1, edge e2,
while (DEBUG_INSN_P (newpos1))
newpos1 = NEXT_INSN (newpos1);
+ /* And end of prologue marker. */
+ if (NOTE_P (newpos1) && NOTE_KIND (newpos1) == NOTE_INSN_PROLOGUE_END)
+ newpos1 = NEXT_INSN (newpos1);
+
+ while (DEBUG_INSN_P (newpos1))
+ newpos1 = NEXT_INSN (newpos1);
+
+ /* And also beginning of function marker. */
+ if (NOTE_P (newpos1) && NOTE_KIND (newpos1) == NOTE_INSN_FUNCTION_BEG)
+ newpos1 = NEXT_INSN (newpos1);
+
+ while (DEBUG_INSN_P (newpos1))
+ newpos1 = NEXT_INSN (newpos1);
+
redirect_from = split_block (src1, PREV_INSN (newpos1))->src;
to_remove = single_succ (redirect_from);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] Fix PR rtl-optimization/100411
2021-05-05 11:00 [patch] Fix PR rtl-optimization/100411 Eric Botcazou
@ 2021-05-05 11:10 ` Jakub Jelinek
2021-05-05 11:21 ` Eric Botcazou
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2021-05-05 11:10 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches
On Wed, May 05, 2021 at 01:00:35PM +0200, Eric Botcazou wrote:
> 2021-05-05 Eric Botcazou <ebotcazou@adacore.com>
>
> PR rtl-optimization/100411
> * cfgcleanup.c (try_crossjump_to_edge): Also skip end of prologue
> and beginning of function markers.
>
> --
> Eric Botcazou
> diff --git a/gcc/cfgcleanup.c b/gcc/cfgcleanup.c
> index f05cb6136c7..64279cc8c20 100644
> --- a/gcc/cfgcleanup.c
> +++ b/gcc/cfgcleanup.c
> @@ -2148,6 +2148,20 @@ try_crossjump_to_edge (int mode, edge e1, edge e2,
> while (DEBUG_INSN_P (newpos1))
> newpos1 = NEXT_INSN (newpos1);
>
> + /* And end of prologue marker. */
> + if (NOTE_P (newpos1) && NOTE_KIND (newpos1) == NOTE_INSN_PROLOGUE_END)
> + newpos1 = NEXT_INSN (newpos1);
> +
> + while (DEBUG_INSN_P (newpos1))
> + newpos1 = NEXT_INSN (newpos1);
> +
> + /* And also beginning of function marker. */
> + if (NOTE_P (newpos1) && NOTE_KIND (newpos1) == NOTE_INSN_FUNCTION_BEG)
> + newpos1 = NEXT_INSN (newpos1);
> +
> + while (DEBUG_INSN_P (newpos1))
> + newpos1 = NEXT_INSN (newpos1);
Do those notes always have to appear in that order?
I mean, can't we have just one while loop that skips over all debug insns,
NOTE_INSN_BASIC_BLOCK_P, NOTE_INSN_PROLOGUE_END and NOTE_INSN_FUNCTION_BEG
and stops on anything else, or, if we want to skip at most one of some or
all of those note kinds, do some tracking if we've already skipped that kind of
note?
> +
> redirect_from = split_block (src1, PREV_INSN (newpos1))->src;
> to_remove = single_succ (redirect_from);
>
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] Fix PR rtl-optimization/100411
2021-05-05 11:10 ` Jakub Jelinek
@ 2021-05-05 11:21 ` Eric Botcazou
2021-05-05 11:37 ` Jakub Jelinek
0 siblings, 1 reply; 6+ messages in thread
From: Eric Botcazou @ 2021-05-05 11:21 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 365 bytes --]
> I mean, can't we have just one while loop that skips over all debug insns,
> NOTE_INSN_BASIC_BLOCK_P, NOTE_INSN_PROLOGUE_END and NOTE_INSN_FUNCTION_BEG
> and stops on anything else, or, if we want to skip at most one of some or
> all of those note kinds, do some tracking if we've already skipped that kind
> of note?
Revised version attached.
--
Eric Botcazou
[-- Attachment #2: pr100411.diff --]
[-- Type: text/x-patch, Size: 997 bytes --]
diff --git a/gcc/cfgcleanup.c b/gcc/cfgcleanup.c
index f05cb6136c7..f49a34dcb0f 100644
--- a/gcc/cfgcleanup.c
+++ b/gcc/cfgcleanup.c
@@ -2134,18 +2134,15 @@ try_crossjump_to_edge (int mode, edge e1, edge e2,
update_br_prob_note (redirect_edges_to);
/* Edit SRC1 to go to REDIRECT_TO at NEWPOS1. */
-
- /* Skip possible basic block header. */
if (LABEL_P (newpos1))
newpos1 = NEXT_INSN (newpos1);
- while (DEBUG_INSN_P (newpos1))
- newpos1 = NEXT_INSN (newpos1);
-
- if (NOTE_INSN_BASIC_BLOCK_P (newpos1))
- newpos1 = NEXT_INSN (newpos1);
-
- while (DEBUG_INSN_P (newpos1))
+ /* Skip debug insns, basic block header and prologue markers. */
+ while (DEBUG_INSN_P (newpos1)
+ || (NOTE_P (newpos1)
+ && (NOTE_KIND (newpos1) == NOTE_INSN_BASIC_BLOCK
+ || NOTE_KIND (newpos1) == NOTE_INSN_PROLOGUE_END
+ || NOTE_KIND (newpos1) == NOTE_INSN_FUNCTION_BEG)))
newpos1 = NEXT_INSN (newpos1);
redirect_from = split_block (src1, PREV_INSN (newpos1))->src;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] Fix PR rtl-optimization/100411
2021-05-05 11:21 ` Eric Botcazou
@ 2021-05-05 11:37 ` Jakub Jelinek
2021-05-05 12:19 ` Eric Botcazou
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2021-05-05 11:37 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches
On Wed, May 05, 2021 at 01:21:20PM +0200, Eric Botcazou wrote:
> > I mean, can't we have just one while loop that skips over all debug insns,
> > NOTE_INSN_BASIC_BLOCK_P, NOTE_INSN_PROLOGUE_END and NOTE_INSN_FUNCTION_BEG
> > and stops on anything else, or, if we want to skip at most one of some or
> > all of those note kinds, do some tracking if we've already skipped that kind
> > of note?
>
> Revised version attached.
It was meant as a question, I don't know what the right answer is.
At least for NOTE_INSN_BASIC_BLOCK skipping more than one might
be problematic, because that would mean we've skipped into a different basic
block and it wouldn't surprise me if split_block in that case crashed or
did something weird (if the first argument is not BLOCK_FOR_INSN of the
second argument when it is non-NULL).
For the other notes, I think they should normally appear just once and
shouldn't be a problem therefore.
> diff --git a/gcc/cfgcleanup.c b/gcc/cfgcleanup.c
> index f05cb6136c7..f49a34dcb0f 100644
> --- a/gcc/cfgcleanup.c
> +++ b/gcc/cfgcleanup.c
> @@ -2134,18 +2134,15 @@ try_crossjump_to_edge (int mode, edge e1, edge e2,
> update_br_prob_note (redirect_edges_to);
>
> /* Edit SRC1 to go to REDIRECT_TO at NEWPOS1. */
> -
> - /* Skip possible basic block header. */
> if (LABEL_P (newpos1))
> newpos1 = NEXT_INSN (newpos1);
>
> - while (DEBUG_INSN_P (newpos1))
> - newpos1 = NEXT_INSN (newpos1);
> -
> - if (NOTE_INSN_BASIC_BLOCK_P (newpos1))
> - newpos1 = NEXT_INSN (newpos1);
> -
> - while (DEBUG_INSN_P (newpos1))
> + /* Skip debug insns, basic block header and prologue markers. */
> + while (DEBUG_INSN_P (newpos1)
> + || (NOTE_P (newpos1)
> + && (NOTE_KIND (newpos1) == NOTE_INSN_BASIC_BLOCK
> + || NOTE_KIND (newpos1) == NOTE_INSN_PROLOGUE_END
> + || NOTE_KIND (newpos1) == NOTE_INSN_FUNCTION_BEG)))
> newpos1 = NEXT_INSN (newpos1);
>
> redirect_from = split_block (src1, PREV_INSN (newpos1))->src;
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] Fix PR rtl-optimization/100411
2021-05-05 11:37 ` Jakub Jelinek
@ 2021-05-05 12:19 ` Eric Botcazou
2021-05-05 12:33 ` Jakub Jelinek
0 siblings, 1 reply; 6+ messages in thread
From: Eric Botcazou @ 2021-05-05 12:19 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 535 bytes --]
> At least for NOTE_INSN_BASIC_BLOCK skipping more than one might
> be problematic, because that would mean we've skipped into a different basic
> block and it wouldn't surprise me if split_block in that case crashed or
> did something weird (if the first argument is not BLOCK_FOR_INSN of the
> second argument when it is non-NULL).
> For the other notes, I think they should normally appear just once and
> shouldn't be a problem therefore.
OK, version essentially equivalent to the original one, but with a loop.
--
Eric Botcazou
[-- Attachment #2: pr100411.diff --]
[-- Type: text/x-patch, Size: 645 bytes --]
diff --git a/gcc/cfgcleanup.c b/gcc/cfgcleanup.c
index f05cb6136c7..17edc4f37ad 100644
--- a/gcc/cfgcleanup.c
+++ b/gcc/cfgcleanup.c
@@ -2145,7 +2145,11 @@ try_crossjump_to_edge (int mode, edge e1, edge e2,
if (NOTE_INSN_BASIC_BLOCK_P (newpos1))
newpos1 = NEXT_INSN (newpos1);
- while (DEBUG_INSN_P (newpos1))
+ /* Skip also prologue and function markers. */
+ while (DEBUG_INSN_P (newpos1)
+ || (NOTE_P (newpos1)
+ && (NOTE_KIND (newpos1) == NOTE_INSN_PROLOGUE_END
+ || NOTE_KIND (newpos1) == NOTE_INSN_FUNCTION_BEG)))
newpos1 = NEXT_INSN (newpos1);
redirect_from = split_block (src1, PREV_INSN (newpos1))->src;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] Fix PR rtl-optimization/100411
2021-05-05 12:19 ` Eric Botcazou
@ 2021-05-05 12:33 ` Jakub Jelinek
0 siblings, 0 replies; 6+ messages in thread
From: Jakub Jelinek @ 2021-05-05 12:33 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches
On Wed, May 05, 2021 at 02:19:27PM +0200, Eric Botcazou wrote:
> > At least for NOTE_INSN_BASIC_BLOCK skipping more than one might
> > be problematic, because that would mean we've skipped into a different basic
> > block and it wouldn't surprise me if split_block in that case crashed or
> > did something weird (if the first argument is not BLOCK_FOR_INSN of the
> > second argument when it is non-NULL).
> > For the other notes, I think they should normally appear just once and
> > shouldn't be a problem therefore.
>
> OK, version essentially equivalent to the original one, but with a loop.
LGTM.
> diff --git a/gcc/cfgcleanup.c b/gcc/cfgcleanup.c
> index f05cb6136c7..17edc4f37ad 100644
> --- a/gcc/cfgcleanup.c
> +++ b/gcc/cfgcleanup.c
> @@ -2145,7 +2145,11 @@ try_crossjump_to_edge (int mode, edge e1, edge e2,
> if (NOTE_INSN_BASIC_BLOCK_P (newpos1))
> newpos1 = NEXT_INSN (newpos1);
>
> - while (DEBUG_INSN_P (newpos1))
> + /* Skip also prologue and function markers. */
> + while (DEBUG_INSN_P (newpos1)
> + || (NOTE_P (newpos1)
> + && (NOTE_KIND (newpos1) == NOTE_INSN_PROLOGUE_END
> + || NOTE_KIND (newpos1) == NOTE_INSN_FUNCTION_BEG)))
> newpos1 = NEXT_INSN (newpos1);
>
> redirect_from = split_block (src1, PREV_INSN (newpos1))->src;
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-05-05 12:33 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-05 11:00 [patch] Fix PR rtl-optimization/100411 Eric Botcazou
2021-05-05 11:10 ` Jakub Jelinek
2021-05-05 11:21 ` Eric Botcazou
2021-05-05 11:37 ` Jakub Jelinek
2021-05-05 12:19 ` Eric Botcazou
2021-05-05 12:33 ` Jakub Jelinek
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).