* [PATCH, RTL] Fix PR 51106
@ 2012-01-19 14:13 Andrey Belevantsev
2012-01-19 14:40 ` Jakub Jelinek
0 siblings, 1 reply; 5+ messages in thread
From: Andrey Belevantsev @ 2012-01-19 14:13 UTC (permalink / raw)
To: GCC Patches; +Cc: Jakub Jelinek
[-- Attachment #1: Type: text/plain, Size: 618 bytes --]
Hello,
As discussed with Jakub in the PR, the problem is that a jump asm goto insn
is removed without purging the dead edges. Bootstrapped and tested on
x86-64. I had to use dg-prune-output to remove the error message and to
make the test pass, as we only checking for an ICE.
OK for trunk/4.6/4.5?
Andrey
gcc/
2012-01-19 Andrey Belevantsev <abel@ispras.ru>
PR target/51106
* function.c (instantiate_virtual_regs_in_insn): Use delete_insn_and_edges
when removing a wrong asm insn.
testsuite/
2012-01-19 Jakub Jelinek <jakub@redhat.com>
PR target/51106
* gcc.c-torture/compile/pr51106.c: New test.
[-- Attachment #2: pr51106.diff --]
[-- Type: text/x-patch, Size: 893 bytes --]
diff --git a/gcc/function.c b/gcc/function.c
index fcb79f5..94e51f4 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -1737,7 +1737,7 @@ instantiate_virtual_regs_in_insn (rtx insn)
if (!check_asm_operands (PATTERN (insn)))
{
error_for_asm (insn, "impossible constraint in %<asm%>");
- delete_insn (insn);
+ delete_insn_and_edges (insn);
}
}
else
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr51106.c b/gcc/testsuite/gcc.c-torture/compile/pr51106.c
new file mode 100644
index 0000000..2f1c51d
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr51106.c
@@ -0,0 +1,19 @@
+/* { dg-options "-w" } */
+/* { dg-prune-output "impossible constraint" } */
+int
+foo (int x)
+{
+ asm goto ("" : : "i" (x) : : lab);
+ return 1;
+lab:
+ return 0;
+}
+
+int
+bar (int x)
+{
+ asm goto ("" : : "i" (x) : : lab);
+ __builtin_unreachable ();
+lab:
+ return 0;
+}
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH, RTL] Fix PR 51106
2012-01-19 14:13 [PATCH, RTL] Fix PR 51106 Andrey Belevantsev
@ 2012-01-19 14:40 ` Jakub Jelinek
2012-04-02 14:56 ` Andrey Belevantsev
0 siblings, 1 reply; 5+ messages in thread
From: Jakub Jelinek @ 2012-01-19 14:40 UTC (permalink / raw)
To: Andrey Belevantsev; +Cc: GCC Patches
On Thu, Jan 19, 2012 at 06:13:58PM +0400, Andrey Belevantsev wrote:
> 2012-01-19 Andrey Belevantsev <abel@ispras.ru>
>
> PR target/51106
> * function.c (instantiate_virtual_regs_in_insn): Use
> delete_insn_and_edges when removing a wrong asm insn.
This is ok for trunk.
> 2012-01-19 Jakub Jelinek <jakub@redhat.com>
>
> PR target/51106
> * gcc.c-torture/compile/pr51106.c: New test.
I'd prefer to explicitly test for the warnings and errors.
So something like:
--- gcc/testsuite/gcc.dg/torture/pr51106-1.c.jj 2012-01-19 15:23:28.501117223 +0100
+++ gcc/testsuite/gcc.dg/torture/pr51106-1.c 2012-01-19 15:37:20.658296791 +0100
@@ -0,0 +1,14 @@
+/* PR target/51106 */
+/* { dg-do "compile" } */
+/* { dg-skip-if "RTL error" { "*-*-*" } { "-fno-fat-lto-objects" } { "" } } */
+
+int
+foo (int x)
+{
+ asm goto ("" : : "i" (x) : : lab); /* { dg-error "impossible constraint" } */
+ return 1;
+lab:
+ return 0;
+}
+
+/* { dg-warning "probably doesn.t match constraints" "" { target *-*-* } 8 } */
--- gcc/testsuite/gcc.dg/torture/pr51106-2.c.jj 2012-01-19 15:23:28.000000000 +0100
+++ gcc/testsuite/gcc.dg/torture/pr51106-2.c 2012-01-19 15:37:30.062243322 +0100
@@ -0,0 +1,14 @@
+/* PR target/51106 */
+/* { dg-do "compile" } */
+/* { dg-skip-if "RTL error" { "*-*-*" } { "-fno-fat-lto-objects" } { "" } } */
+
+int
+bar (int x)
+{
+ asm goto ("" : : "i" (x) : : lab); /* { dg-error "impossible constraint" } */
+ __builtin_unreachable ();
+lab:
+ return 0;
+}
+
+/* { dg-warning "probably doesn.t match constraints" "" { target *-*-* } 8 } */
Jakub
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH, RTL] Fix PR 51106
2012-01-19 14:40 ` Jakub Jelinek
@ 2012-04-02 14:56 ` Andrey Belevantsev
2012-04-03 9:36 ` Jakub Jelinek
0 siblings, 1 reply; 5+ messages in thread
From: Andrey Belevantsev @ 2012-04-02 14:56 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: GCC Patches
Hello,
On 19.01.2012 18:40, Jakub Jelinek wrote:
> On Thu, Jan 19, 2012 at 06:13:58PM +0400, Andrey Belevantsev wrote:
>> 2012-01-19 Andrey Belevantsev<abel@ispras.ru>
>>
>> PR target/51106
>> * function.c (instantiate_virtual_regs_in_insn): Use
>> delete_insn_and_edges when removing a wrong asm insn.
>
> This is ok for trunk.
>
After Richi's RTL generation related cleanups went it, the extra
cleanup_cfg call was added so we are no longer lucky to have the proper
fallthru edge in this case. The PR trail has the patch to purge_dead_edges
making it consider this situation, but I still prefer the below patch that
fixes only the invalid asm case. The reason is that I think it unlikely
that after initial RTL expansion (of which the instantiate virtual regs
pass seems to be the part) we will get the problematic situation. However,
I'm happy to test the PR trail patch, too.
Tested fine on x86-64, ok for trunk?
Andrey
2012-04-02 Andrey Belevantsev <abel@ispras.ru>
PR target/51106
PR middle-end/52650
* function.c (instantiate_virtual_regs_in_insn): Make sure to set
the proper fallthru bits when removing a wrong jump asm.
diff --git a/gcc/function.c b/gcc/function.c
index 3e903ef..a2638bb 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -1730,6 +1730,15 @@ instantiate_virtual_regs_in_insn (rtx insn)
if (!check_asm_operands (PATTERN (insn)))
{
error_for_asm (insn, "impossible constraint in %<asm%>");
+ if (JUMP_P (insn))
+ {
+ basic_block bb = BLOCK_FOR_INSN (insn);
+ edge e;
+
+ if (single_succ_p (bb)
+ && !((e = single_succ_edge (bb))->flags & EDGE_FALLTHRU))
+ e->flags |= EDGE_FALLTHRU;
+ }
delete_insn_and_edges (insn);
}
}
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH, RTL] Fix PR 51106
2012-04-02 14:56 ` Andrey Belevantsev
@ 2012-04-03 9:36 ` Jakub Jelinek
2012-04-04 7:59 ` Andrey Belevantsev
0 siblings, 1 reply; 5+ messages in thread
From: Jakub Jelinek @ 2012-04-03 9:36 UTC (permalink / raw)
To: Andrey Belevantsev; +Cc: GCC Patches
On Mon, Apr 02, 2012 at 06:56:25PM +0400, Andrey Belevantsev wrote:
> After Richi's RTL generation related cleanups went it, the extra
> cleanup_cfg call was added so we are no longer lucky to have the
> proper fallthru edge in this case. The PR trail has the patch to
> purge_dead_edges making it consider this situation, but I still
> prefer the below patch that fixes only the invalid asm case. The
> reason is that I think it unlikely that after initial RTL expansion
> (of which the instantiate virtual regs pass seems to be the part) we
> will get the problematic situation. However, I'm happy to test the
> PR trail patch, too.
I don't like blindly changing edge into FALLTHRU, generally the edge
could be abnormal, or EH, etc. and making that fallthru is not a good idea.
I'd prefer if wherever the fallthru edge is removed the other normal edge(s)
are adjusted.
Jakub
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH, RTL] Fix PR 51106
2012-04-03 9:36 ` Jakub Jelinek
@ 2012-04-04 7:59 ` Andrey Belevantsev
0 siblings, 0 replies; 5+ messages in thread
From: Andrey Belevantsev @ 2012-04-04 7:59 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: GCC Patches
On 03.04.2012 13:36, Jakub Jelinek wrote:
> On Mon, Apr 02, 2012 at 06:56:25PM +0400, Andrey Belevantsev wrote:
>> After Richi's RTL generation related cleanups went it, the extra
>> cleanup_cfg call was added so we are no longer lucky to have the
>> proper fallthru edge in this case. The PR trail has the patch to
>> purge_dead_edges making it consider this situation, but I still
>> prefer the below patch that fixes only the invalid asm case. The
>> reason is that I think it unlikely that after initial RTL expansion
>> (of which the instantiate virtual regs pass seems to be the part) we
>> will get the problematic situation. However, I'm happy to test the
>> PR trail patch, too.
>
> I don't like blindly changing edge into FALLTHRU, generally the edge
> could be abnormal, or EH, etc. and making that fallthru is not a good idea.
> I'd prefer if wherever the fallthru edge is removed the other normal edge(s)
> are adjusted.
Well, as I mentioned in
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51106#c18 the removal happens
in try_optimize_cfg around line 2617, that's the code that deals with
removing trivially empty block (in the PR case, the succ of the asm goto
block is trivially empty). After that we have an asm goto acting as an
unconditional jump with a single succ edge and no fallthru bit, which seems
perfectly fine. We get into trouble when we actually realize that the asm
is bogus. Thus I've tried to fix it up at this time. The options that we
briefly discussed on IRC with Richi are as follows:
- Fix up try_optimize_cfg in the case of asm gotos, but it is not clear to
me how do we do this -- we don't yet distinguish between good and bad asm
goto at this point;
- Fix up function.c as I did but make it more robust. Either handle more
cases with strange edges (EH seems also possible after introducing the
throw attribute for asms), or just remove the asm and insert the
unconditional jump pointing to the place where the asm was, retaining all
the flags;
- Fix up purge_dead_edges. as I did initially in
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51106#c16. When we find a
situation of no jump at the end of block and no fallthru edge, assert this
happens only with single_succ_p and actually make this edge fallthru.
(Probably also watch out whether we need to make it fake or whatever.) Or
as Richi tried, just accept the situation of having no successors here, as
in -- no fallthru edge on entry to purge_dead_edges and no jump means no
successors, period.
I think that just nobody deleted unconditional jumps with
delete_insn_and_edges previously, otherwise I don't understand why this did
not trigger.
Thoughts?
Andrey
>
> Jakub
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-04-04 7:59 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-19 14:13 [PATCH, RTL] Fix PR 51106 Andrey Belevantsev
2012-01-19 14:40 ` Jakub Jelinek
2012-04-02 14:56 ` Andrey Belevantsev
2012-04-03 9:36 ` Jakub Jelinek
2012-04-04 7:59 ` Andrey Belevantsev
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).