diff mbox

rtl_verify_flow_info fix

Message ID 4E64885E.7040406@codesourcery.com
State New
Headers show

Commit Message

Tom de Vries Sept. 5, 2011, 8:29 a.m. UTC
Hi Eric,

During testing the approved-for-commit middle-end patch for bug 43864 on ARM, I
ran into a gcc.dg/torture/pr46068.c ICE.

An asm jump is not recognized as an unconditional jump, so its followed by a
fall-through block rather than a barrier.
...
(jump_insn 10 9 19 4 (asm_operands/v ("") ("") 0 []
         []
         [
            (label_ref:SI 16)
        ] pr46068.c:16) pr46068.c:6 -1
     (nil)
 -> 16)
...

ce3 then turns the asm jump into an asm return, still without a barrier after it:
...
(jump_insn 10 9 19 4 (asm_operands/v ("") ("") 0 []
         []
         [
            (return)
        ] pr46068.c:16) pr46068.c:6 -1
     (nil)
 -> return)
...

This triggers the cfgrtl.c assert:
...
      if (JUMP_P (x)
          && returnjump_p (x) && ! condjump_p (x)
          && ! (next_nonnote_insn (x) && BARRIER_P (next_nonnote_insn (x))))
            fatal_insn ("return not followed by barrier", x);
...

The patch fixes the assert by replacing ! condjump_p with uncondjump_p.

Bootstrapped and reg-tested on arm and x86_64.

OK for trunk?

Thanks,
- Tom

2011-09-05  Tom de Vries  <tom@codesourcery.com>

	* cfgrtl.c (rtl_verify_flow_info): Use any_uncondjump_p instead of
	!condjump_p.

Comments

Jakub Jelinek Sept. 5, 2011, 8:53 a.m. UTC | #1
On Mon, Sep 05, 2011 at 10:29:18AM +0200, Tom de Vries wrote:
> Hi Eric,
> 
> During testing the approved-for-commit middle-end patch for bug 43864 on ARM, I
> ran into a gcc.dg/torture/pr46068.c ICE.
> 
> An asm jump is not recognized as an unconditional jump, so its followed by a
> fall-through block rather than a barrier.
> ...
> (jump_insn 10 9 19 4 (asm_operands/v ("") ("") 0 []
>          []
>          [
>             (label_ref:SI 16)
>         ] pr46068.c:16) pr46068.c:6 -1
>      (nil)
>  -> 16)
> ...
> 
> ce3 then turns the asm jump into an asm return, still without a barrier after it:
> ...
> (jump_insn 10 9 19 4 (asm_operands/v ("") ("") 0 []
>          []
>          [
>             (return)
>         ] pr46068.c:16) pr46068.c:6 -1
>      (nil)
>  -> return)
> ...

I'd say the above transformation shouldn't be valid, asm goto is given some
possible labels it can jump to, but what will be emitted when the operand is
RETURN?  I think final.c does:
            else if (letter == 'l')
              output_asm_label (operands[opnum]);
here and when operands[opnum] isn't a label or deleted label, that function
will
  output_operand_lossage ("'%%l' operand isn't a label");
You don't get it only because this testcase uses empty inline asm string
and thus doesn't use any of the labels.

	Jakub
diff mbox

Patch

Index: gcc/cfgrtl.c
===================================================================
--- gcc/cfgrtl.c (revision 178056)
+++ gcc/cfgrtl.c (working copy)
@@ -2169,7 +2169,7 @@  rtl_verify_flow_info (void)
 	}
 
       if (JUMP_P (x)
-	  && returnjump_p (x) && ! condjump_p (x)
+	  && returnjump_p (x) && any_uncondjump_p (x)
 	  && ! (next_nonnote_insn (x) && BARRIER_P (next_nonnote_insn (x))))
 	    fatal_insn ("return not followed by barrier", x);
       if (curr_bb && x == BB_END (curr_bb))