Patchwork rtl_verify_flow_info fix

login
register
mail settings
Submitter Tom de Vries
Date Sept. 5, 2011, 12:31 p.m.
Message ID <4E64C124.4090507@codesourcery.com>
Download mbox | patch
Permalink /patch/113343/
State New
Headers show

Comments

Tom de Vries - Sept. 5, 2011, 12:31 p.m.
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
Jakub Jelinek - Sept. 5, 2011, 12:46 p.m.
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

Patch

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)
     {