[mid-end,__RTL] Clean df state despite invalid __RTL startwith passes
diff mbox series

Message ID HE1PR0802MB22515F7EF978B32D75646776E0710@HE1PR0802MB2251.eurprd08.prod.outlook.com
State New
Headers show
Series
  • [mid-end,__RTL] Clean df state despite invalid __RTL startwith passes
Related show

Commit Message

Matthew Malcomson Nov. 14, 2019, 6:37 p.m. UTC
Hi there,

When compiling an __RTL function that has an invalid "startwith" pass we
currently don't run the dfinish cleanup pass. This means we ICE on the next
function.

This change ensures that all state is cleaned up for the next function
to run correctly.

As an example, before this change the following code would ICE when compiling
the function `foo2` because the "peephole2" pass is not run at optimisation
level -O0.

When compiled with
./aarch64-none-linux-gnu-gcc -O0 -S missed-pass-error.c -o test.s

```
int __RTL (startwith ("peephole2")) badfoo ()
{
(function "badfoo"
  (insn-chain
    (block 2
      (edge-from entry (flags "FALLTHRU"))
      (cnote 3 [bb 2] NOTE_INSN_BASIC_BLOCK)
      (cinsn 101 (set (reg:DI x19) (reg:DI x0)))
      (cinsn 10 (use (reg/i:SI x19)))
      (edge-to exit (flags "FALLTHRU"))
    ) ;; block 2
  ) ;; insn-chain
) ;; function "foo2"
}

int __RTL (startwith ("final")) foo2 ()
{
(function "foo2"
  (insn-chain
    (block 2
      (edge-from entry (flags "FALLTHRU"))
      (cnote 3 [bb 2] NOTE_INSN_BASIC_BLOCK)
      (cinsn 101 (set (reg:DI x19) (reg:DI x0)))
      (cinsn 10 (use (reg/i:SI x19)))
      (edge-to exit (flags "FALLTHRU"))
    ) ;; block 2
  ) ;; insn-chain
) ;; function "foo2"
}
```

Now it silently ignores the __RTL function and successfully compiles foo2.

regtest done on aarch64
regtest done on x86_64

OK for trunk?

gcc/ChangeLog:

2019-11-14  Matthew Malcomson  <matthew.malcomson@arm.com>

	* passes.c (should_skip_pass_p): Always run "dfinish".

gcc/testsuite/ChangeLog:

2019-11-14  Matthew Malcomson  <matthew.malcomson@arm.com>

	* gcc.dg/rtl/aarch64/missed-pass-error.c: New test.



###############     Attachment also inlined for ease of reply    ###############
diff --git a/gcc/passes.c b/gcc/passes.c
index d86af115ecb16fcab6bfce070f1f3e4f1d90ce71..258f85ab4f8a1519b978b75dfa67536d2eacd106 100644
--- a/gcc/passes.c
+++ b/gcc/passes.c
@@ -2375,7 +2375,8 @@ should_skip_pass_p (opt_pass *pass)
     return false;
 
   /* Don't skip df init; later RTL passes need it.  */
-  if (strstr (pass->name, "dfinit") != NULL)
+  if (strstr (pass->name, "dfinit") != NULL
+      || strstr (pass->name, "dfinish") != NULL)
     return false;
 
   if (!quiet_flag)
diff --git a/gcc/testsuite/gcc.dg/rtl/aarch64/missed-pass-error.c b/gcc/testsuite/gcc.dg/rtl/aarch64/missed-pass-error.c
new file mode 100644
index 0000000000000000000000000000000000000000..2f02ca9d0c40b372d86b24009540e157ed1a8c59
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/rtl/aarch64/missed-pass-error.c
@@ -0,0 +1,45 @@
+/* { dg-do compile { target aarch64-*-* } } */
+/* { dg-additional-options "-O0" } */
+
+/*
+   When compiling __RTL functions the startwith string can be either incorrect
+   (i.e. not matching a pass) or be unused (i.e. can refer to a pass that is
+   not run at the current optimisation level).
+
+   Here we ensure that the state clean up is still run, so that functions other
+   than the faulty one can still be compiled.
+ */
+
+int __RTL (startwith ("peephole2")) badfoo ()
+{
+(function "badfoo"
+  (insn-chain
+    (block 2
+      (edge-from entry (flags "FALLTHRU"))
+      (cnote 3 [bb 2] NOTE_INSN_BASIC_BLOCK)
+      (cinsn 101 (set (reg:DI x19) (reg:DI x0)))
+      (cinsn 10 (use (reg/i:SI x19)))
+      (edge-to exit (flags "FALLTHRU"))
+    ) ;; block 2
+  ) ;; insn-chain
+) ;; function "foo2"
+}
+
+/* Compile a valid __RTL function to test state from the "dfinit" pass has been
+   cleaned with the "dfinish" pass.  */
+
+int __RTL (startwith ("final")) foo2 ()
+{
+(function "foo2"
+  (insn-chain
+    (block 2
+      (edge-from entry (flags "FALLTHRU"))
+      (cnote 3 [bb 2] NOTE_INSN_BASIC_BLOCK)
+      (cinsn 101 (set (reg:DI x19) (reg:DI x0)))
+      (cinsn 10 (use (reg/i:SI x19)))
+      (edge-to exit (flags "FALLTHRU"))
+    ) ;; block 2
+  ) ;; insn-chain
+) ;; function "foo2"
+}
+

Comments

Richard Biener Nov. 15, 2019, 7:41 a.m. UTC | #1
On Thu, 14 Nov 2019, Matthew Malcomson wrote:

> Hi there,
> 
> When compiling an __RTL function that has an invalid "startwith" pass we
> currently don't run the dfinish cleanup pass. This means we ICE on the next
> function.
> 
> This change ensures that all state is cleaned up for the next function
> to run correctly.
> 
> As an example, before this change the following code would ICE when compiling
> the function `foo2` because the "peephole2" pass is not run at optimisation
> level -O0.
> 
> When compiled with
> ./aarch64-none-linux-gnu-gcc -O0 -S missed-pass-error.c -o test.s
> 
> ```
> int __RTL (startwith ("peephole2")) badfoo ()
> {
> (function "badfoo"
>   (insn-chain
>     (block 2
>       (edge-from entry (flags "FALLTHRU"))
>       (cnote 3 [bb 2] NOTE_INSN_BASIC_BLOCK)
>       (cinsn 101 (set (reg:DI x19) (reg:DI x0)))
>       (cinsn 10 (use (reg/i:SI x19)))
>       (edge-to exit (flags "FALLTHRU"))
>     ) ;; block 2
>   ) ;; insn-chain
> ) ;; function "foo2"
> }
> 
> int __RTL (startwith ("final")) foo2 ()
> {
> (function "foo2"
>   (insn-chain
>     (block 2
>       (edge-from entry (flags "FALLTHRU"))
>       (cnote 3 [bb 2] NOTE_INSN_BASIC_BLOCK)
>       (cinsn 101 (set (reg:DI x19) (reg:DI x0)))
>       (cinsn 10 (use (reg/i:SI x19)))
>       (edge-to exit (flags "FALLTHRU"))
>     ) ;; block 2
>   ) ;; insn-chain
> ) ;; function "foo2"
> }
> ```
> 
> Now it silently ignores the __RTL function and successfully compiles foo2.
> 
> regtest done on aarch64
> regtest done on x86_64
> 
> OK for trunk?

OK.

Richard.

> gcc/ChangeLog:
> 
> 2019-11-14  Matthew Malcomson  <matthew.malcomson@arm.com>
> 
> 	* passes.c (should_skip_pass_p): Always run "dfinish".
> 
> gcc/testsuite/ChangeLog:
> 
> 2019-11-14  Matthew Malcomson  <matthew.malcomson@arm.com>
> 
> 	* gcc.dg/rtl/aarch64/missed-pass-error.c: New test.
> 
> 
> 
> ###############     Attachment also inlined for ease of reply    ###############
> 
> 
> diff --git a/gcc/passes.c b/gcc/passes.c
> index d86af115ecb16fcab6bfce070f1f3e4f1d90ce71..258f85ab4f8a1519b978b75dfa67536d2eacd106 100644
> --- a/gcc/passes.c
> +++ b/gcc/passes.c
> @@ -2375,7 +2375,8 @@ should_skip_pass_p (opt_pass *pass)
>      return false;
>  
>    /* Don't skip df init; later RTL passes need it.  */
> -  if (strstr (pass->name, "dfinit") != NULL)
> +  if (strstr (pass->name, "dfinit") != NULL
> +      || strstr (pass->name, "dfinish") != NULL)
>      return false;
>  
>    if (!quiet_flag)
> diff --git a/gcc/testsuite/gcc.dg/rtl/aarch64/missed-pass-error.c b/gcc/testsuite/gcc.dg/rtl/aarch64/missed-pass-error.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..2f02ca9d0c40b372d86b24009540e157ed1a8c59
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/rtl/aarch64/missed-pass-error.c
> @@ -0,0 +1,45 @@
> +/* { dg-do compile { target aarch64-*-* } } */
> +/* { dg-additional-options "-O0" } */
> +
> +/*
> +   When compiling __RTL functions the startwith string can be either incorrect
> +   (i.e. not matching a pass) or be unused (i.e. can refer to a pass that is
> +   not run at the current optimisation level).
> +
> +   Here we ensure that the state clean up is still run, so that functions other
> +   than the faulty one can still be compiled.
> + */
> +
> +int __RTL (startwith ("peephole2")) badfoo ()
> +{
> +(function "badfoo"
> +  (insn-chain
> +    (block 2
> +      (edge-from entry (flags "FALLTHRU"))
> +      (cnote 3 [bb 2] NOTE_INSN_BASIC_BLOCK)
> +      (cinsn 101 (set (reg:DI x19) (reg:DI x0)))
> +      (cinsn 10 (use (reg/i:SI x19)))
> +      (edge-to exit (flags "FALLTHRU"))
> +    ) ;; block 2
> +  ) ;; insn-chain
> +) ;; function "foo2"
> +}
> +
> +/* Compile a valid __RTL function to test state from the "dfinit" pass has been
> +   cleaned with the "dfinish" pass.  */
> +
> +int __RTL (startwith ("final")) foo2 ()
> +{
> +(function "foo2"
> +  (insn-chain
> +    (block 2
> +      (edge-from entry (flags "FALLTHRU"))
> +      (cnote 3 [bb 2] NOTE_INSN_BASIC_BLOCK)
> +      (cinsn 101 (set (reg:DI x19) (reg:DI x0)))
> +      (cinsn 10 (use (reg/i:SI x19)))
> +      (edge-to exit (flags "FALLTHRU"))
> +    ) ;; block 2
> +  ) ;; insn-chain
> +) ;; function "foo2"
> +}
> +
> 
>

Patch
diff mbox series

diff --git a/gcc/passes.c b/gcc/passes.c
index d86af115ecb16fcab6bfce070f1f3e4f1d90ce71..258f85ab4f8a1519b978b75dfa67536d2eacd106 100644
--- a/gcc/passes.c
+++ b/gcc/passes.c
@@ -2375,7 +2375,8 @@  should_skip_pass_p (opt_pass *pass)
     return false;
 
   /* Don't skip df init; later RTL passes need it.  */
-  if (strstr (pass->name, "dfinit") != NULL)
+  if (strstr (pass->name, "dfinit") != NULL
+      || strstr (pass->name, "dfinish") != NULL)
     return false;
 
   if (!quiet_flag)
diff --git a/gcc/testsuite/gcc.dg/rtl/aarch64/missed-pass-error.c b/gcc/testsuite/gcc.dg/rtl/aarch64/missed-pass-error.c
new file mode 100644
index 0000000000000000000000000000000000000000..2f02ca9d0c40b372d86b24009540e157ed1a8c59
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/rtl/aarch64/missed-pass-error.c
@@ -0,0 +1,45 @@ 
+/* { dg-do compile { target aarch64-*-* } } */
+/* { dg-additional-options "-O0" } */
+
+/*
+   When compiling __RTL functions the startwith string can be either incorrect
+   (i.e. not matching a pass) or be unused (i.e. can refer to a pass that is
+   not run at the current optimisation level).
+
+   Here we ensure that the state clean up is still run, so that functions other
+   than the faulty one can still be compiled.
+ */
+
+int __RTL (startwith ("peephole2")) badfoo ()
+{
+(function "badfoo"
+  (insn-chain
+    (block 2
+      (edge-from entry (flags "FALLTHRU"))
+      (cnote 3 [bb 2] NOTE_INSN_BASIC_BLOCK)
+      (cinsn 101 (set (reg:DI x19) (reg:DI x0)))
+      (cinsn 10 (use (reg/i:SI x19)))
+      (edge-to exit (flags "FALLTHRU"))
+    ) ;; block 2
+  ) ;; insn-chain
+) ;; function "foo2"
+}
+
+/* Compile a valid __RTL function to test state from the "dfinit" pass has been
+   cleaned with the "dfinish" pass.  */
+
+int __RTL (startwith ("final")) foo2 ()
+{
+(function "foo2"
+  (insn-chain
+    (block 2
+      (edge-from entry (flags "FALLTHRU"))
+      (cnote 3 [bb 2] NOTE_INSN_BASIC_BLOCK)
+      (cinsn 101 (set (reg:DI x19) (reg:DI x0)))
+      (cinsn 10 (use (reg/i:SI x19)))
+      (edge-to exit (flags "FALLTHRU"))
+    ) ;; block 2
+  ) ;; insn-chain
+) ;; function "foo2"
+}
+