Re: [Patch] [mid-end][__RTL] Clean df state despite invalid __RTL startwith passes

[Richard Biener <rguenther at suse dot de>]

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"
> +}
> +
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)