Re: rtl_verify_flow_info fix
On Tue, Sep 06, 2011 at 11:05:16AM +0200, Tom de Vries wrote: > bootstrapped and regtested on x86_64, build and regtested on arm. > > OK for trunk? Yes. > 2011-09-06 Tom de Vries > > * recog.c (asm_labels_ok): New function. > (check_asm_operands): Use asm_labels_ok. Jakub
Re: rtl_verify_flow_info fix
On 09/05/2011 02:46 PM, Jakub Jelinek wrote:
> On Mon, Sep 05, 2011 at 02:31:32PM +0200, Tom de Vries wrote:
>> --- gcc/recog.c (revision 178145)
>> +++ gcc/recog.c (working copy)
>> @@ -118,6 +118,46 @@ init_recog (void)
>> }
>>
>>
>> +/* Return true if labels in asm operands BODY are LABEL_REFs. */
>> +
>> +static bool
>> +asm_labels_ok (rtx body)
>> +{
>> + rtx first, asmop = NULL;
>> + int i;
>
> asmop = extract_asm_operands (body);
>
> if (asmop == NULL)
> return true;
> ?
>
> I'd say you don't need to call asm_noperands after it.
Yes, that's better.
>
> Jakub
bootstrapped and regtested on x86_64, build and regtested on arm.
OK for trunk?
Thanks,
- Tom
2011-09-06 Tom de Vries
* recog.c (asm_labels_ok): New function.
(check_asm_operands): Use asm_labels_ok.
Index: gcc/recog.c
===
--- gcc/recog.c (revision 178145)
+++ gcc/recog.c (working copy)
@@ -118,6 +118,25 @@ init_recog (void)
}
+/* Return true if labels in asm operands BODY are LABEL_REFs. */
+
+static bool
+asm_labels_ok (rtx body)
+{
+ rtx asmop;
+ int i;
+
+ asmop = extract_asm_operands (body);
+ if (asmop == NULL_RTX)
+return true;
+
+ for (i = 0; i < ASM_OPERANDS_LABEL_LENGTH (asmop); i++)
+if (GET_CODE (ASM_OPERANDS_LABEL (asmop, i)) != LABEL_REF)
+ return false;
+
+ return true;
+}
+
/* Check that X is an insn-body for an `asm' with operands
and that the operands mentioned in it are legitimate. */
@@ -129,6 +148,9 @@ check_asm_operands (rtx x)
const char **constraints;
int i;
+ if (!asm_labels_ok (x))
+return 0;
+
/* Post-reload, be more strict with things. */
if (reload_completed)
{
Re: rtl_verify_flow_info fix
On Mon, Sep 05, 2011 at 02:31:32PM +0200, Tom de Vries wrote:
> --- gcc/recog.c (revision 178145)
> +++ gcc/recog.c (working copy)
> @@ -118,6 +118,46 @@ init_recog (void)
> }
>
>
> +/* Return true if labels in asm operands BODY are LABEL_REFs. */
> +
> +static bool
> +asm_labels_ok (rtx body)
> +{
> + rtx first, asmop = NULL;
> + int i;
asmop = extract_asm_operands (body);
if (asmop == NULL)
return true;
?
I'd say you don't need to call asm_noperands after it.
Jakub
Re: rtl_verify_flow_info fix
On 09/05/2011 10:53 AM, Jakub Jelinek wrote:
> 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.
Right, if I use something like "b %l0", I hit the error you describe.
>
> Jakub
I managed to prevent the transformation using attached untested patch.
Is this the proper way to fix this?
Thanks,
- Tom
Index: gcc/recog.c
===
--- gcc/recog.c (revision 178145)
+++ gcc/recog.c (working copy)
@@ -118,6 +118,46 @@ init_recog (void)
}
+/* Return true if labels in asm operands BODY are LABEL_REFs. */
+
+static bool
+asm_labels_ok (rtx body)
+{
+ rtx first, asmop = NULL;
+ int i;
+
+ if (asm_noperands (body) <= 0)
+return true;
+
+ switch (GET_CODE (body))
+{
+case ASM_OPERANDS:
+ asmop = body;
+ break;
+
+case SET:
+ asmop = SET_SRC (body);
+ break;
+
+case PARALLEL:
+ first = XVECEXP (body, 0, 0);
+ if (GET_CODE (first) != SET)
+ return true;
+ asmop = SET_SRC (first);
+ break;
+default:
+ gcc_unreachable ();
+ break;
+}
+
+ gcc_assert (GET_CODE (asmop) == ASM_OPERANDS);
+
+ for (i = 0; i < ASM_OPERANDS_LABEL_LENGTH (asmop); i++)
+if (GET_CODE (ASM_OPERANDS_LABEL (asmop, i)) != LABEL_REF)
+ return false;
+ return true;
+}
+
/* Check that X is an insn-body for an `asm' with operands
and that the operands mentioned in it are legitimate. */
@@ -129,6 +169,9 @@ check_asm_operands (rtx x)
const char **constraints;
int i;
+ if (!asm_labels_ok (x))
+return 0;
+
/* Post-reload, be more strict with things. */
if (reload_completed)
{
Re: rtl_verify_flow_info fix
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
rtl_verify_flow_info fix
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
* cfgrtl.c (rtl_verify_flow_info): Use any_uncondjump_p instead of
!condjump_p.
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))
