[AArch64,mid-end,__RTL] Allow backends to set state when skipping passes.

When compiling __RTL functions with a start pass, `skip_pass` needs to
set global state when skipping a pass that would have marked something
for future passes to see.

Existing examples are setting `reload_completed` and
`epilogue_completed` when skipping the reload and pro_and_epilogue
passes respectively.

This general problem occurs again in the AArch64 backend, where the
cfun->machine->frame.laid_out variable is set on each function when
`aarch64_layout_frame` is called (which can happen in both ira and
reload.
Currently any `return` statement will trigger an assert in the AArch64
backend due to checking if address signing is enabled when the above
variable is not set.

To account for the need to have a backend specific settings we add a
target hook that each backend can implement as required for their own
global state.

Here we also account for the motivating example by implementing this
hook for the AArch64 backend.

regtested on x86_64 and aarch64.

gcc/ChangeLog:

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

	* config/aarch64/aarch64.c (aarch64_skip_pass): New.
	(TARGET_BACKEND_SKIP_PASS): New.
	* doc/tm.texi: Document BACKEND_SKIP_PASS hook.
	* doc/tm.texi.in: Document BACKEND_SKIP_PASS hook.
	* passes.c (skip_pass): Call targetm.backend_skip_pass if it's
	defined.
	* target.def (backend_skip_pass): Introduce new hook.

gcc/testsuite/ChangeLog:

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

	* gcc.dg/rtl/aarch64/return-statement.c: New test.



###############     Attachment also inlined for ease of reply    ###############
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 599d07a729e7438080f8b5240ee95037a49fb983..fbe900dec8d5ef3daf7be691cdaf4350c13d5024 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -75,6 +75,7 @@
 #include "intl.h"
 #include "expmed.h"
 #include "function-abi.h"
+#include "tree-pass.h"
 
 /* This file should be included last.  */
 #include "target-def.h"
@@ -21371,6 +21372,14 @@ aarch64_stack_protect_guard (void)
   return NULL_TREE;
 }
 
+/* Implement TARGET_BACKEND_SKIP_PASS.  We need to set
+   cfun->machine->frame.laid_out when skipping the ira pass.  */
+void aarch64_skip_pass (opt_pass *pass)
+{
+  if (strcmp (pass->name, "ira") == 0)
+    cfun->machine->frame.laid_out = true;
+}
+
 /* Implement TARGET_ASM_FILE_END for AArch64.  This adds the AArch64 GNU NOTE
    section at the end if needed.  */
 #define GNU_PROPERTY_AARCH64_FEATURE_1_AND	0xc0000000
@@ -21947,6 +21956,9 @@ aarch64_libgcc_floating_mode_supported_p
 #undef TARGET_STRICT_ARGUMENT_NAMING
 #define TARGET_STRICT_ARGUMENT_NAMING hook_bool_CUMULATIVE_ARGS_true
 
+#undef TARGET_BACKEND_SKIP_PASS
+#define TARGET_BACKEND_SKIP_PASS aarch64_skip_pass
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 
 #include "gt-aarch64.h"
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index cd9aed9874f4e6b2b0e2f8956ed6155975e643a8..d02fd747b479246cfdf7c7e6af03a1a2bd43e1c4 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -12152,3 +12152,7 @@ This target hook can be used to generate a target-specific code
 @deftypefn {Target Hook} void TARGET_RUN_TARGET_SELFTESTS (void)
 If selftests are enabled, run any selftests for this target.
 @end deftypefn
+
+@deftypefn {Target Hook} void TARGET_BACKEND_SKIP_PASS (opt_pass *@var{pass})
+Given pass currently skipping, run any actions to set up state.  When compiling code from the RTL frontend, some passes are skipped.  If a given pass would set backend state to mark that a given action has been done, then the backend should implement this hook to set the same state when that pass is skipped.  As an example, the AArch64 backend sets the backend specific cfun->machine->frame.laid_out structure member when the ira pass is skipped.
+@end deftypefn
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index 2739e9ceec5ad7253ff9135da8dbe3bf6010e8d7..7cf446a88abe1eb3bfdcf1c685fc19905d46fca0 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -8194,3 +8194,5 @@ maintainer is familiar with.
 @hook TARGET_SPECULATION_SAFE_VALUE
 
 @hook TARGET_RUN_TARGET_SELFTESTS
+
+@hook TARGET_BACKEND_SKIP_PASS
diff --git a/gcc/passes.c b/gcc/passes.c
index d86af115ecb16fcab6bfce070f1f3e4f1d90ce71..16b3eb106f52da88d4a5717cdc4b66efd4f2c9d4 100644
--- a/gcc/passes.c
+++ b/gcc/passes.c
@@ -2416,6 +2416,9 @@ skip_pass (opt_pass *pass)
       rtl_register_cfg_hooks ();
       cfun->curr_properties &= ~PROP_cfglayout;
     }
+
+  if (targetm.backend_skip_pass)
+    targetm.backend_skip_pass(pass);
 }
 
 /* Execute PASS. */
diff --git a/gcc/target.def b/gcc/target.def
index 8e83c2c7a7136511c07a5bc9e18876c91a38b955..f5313fa1ebe50875559e68792991c6ea37e15bcb 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -6765,6 +6765,12 @@ DEFHOOK
  void, (void),
  NULL)
 
+DEFHOOK
+(backend_skip_pass,
+ "Given pass currently skipping, run any actions to set up state.  When compiling code from the RTL frontend, some passes are skipped.  If a given pass would set backend state to mark that a given action has been done, then the backend should implement this hook to set the same state when that pass is skipped.  As an example, the AArch64 backend sets the backend specific cfun->machine->frame.laid_out structure member when the ira pass is skipped.",
+ void, (opt_pass *pass), NULL)
+
+
 /* Close the 'struct gcc_target' definition.  */
 HOOK_VECTOR_END (C90_EMPTY_HACK)
 
diff --git a/gcc/testsuite/gcc.dg/rtl/aarch64/return-statement.c b/gcc/testsuite/gcc.dg/rtl/aarch64/return-statement.c
new file mode 100644
index 0000000000000000000000000000000000000000..4fae39b4c3d657063f07f6c4a23e2eb7e1d46068
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/rtl/aarch64/return-statement.c
@@ -0,0 +1,41 @@
+/* { dg-do compile { target aarch64-*-* } } */
+
+/* Testing that the aarch64 `backend_skip_pass` implementation sets the
+   relevant value so that `return` does not cause an assertion failure later
+   on.  */
+int __RTL (startwith ("cprop_hardreg")) foo2 (int a)
+{
+(function "foo2"
+  (param "a"
+    (DECL_RTL (reg/v:SI <1> [ a ]))
+    (DECL_RTL_INCOMING (reg:SI x0 [ a ])))
+  (insn-chain
+    (cnote 1 NOTE_INSN_DELETED)
+    (block 2
+      (edge-from entry (flags "FALLTHRU"))
+      (cnote 4 [bb 2] NOTE_INSN_BASIC_BLOCK)
+      (cnote 17 NOTE_INSN_PROLOGUE_END)
+      (cnote 2 NOTE_INSN_DELETED)
+      (cnote 3 NOTE_INSN_FUNCTION_BEG)
+      (cnote 6 NOTE_INSN_DELETED)
+      (cinsn:TI 11 (set (reg/i:SI x0)
+                    (plus:SI (reg:SI x0 [95])
+                        (const_int 1))) "some-test-file.c":7)
+      (cinsn 12 (use (reg/i:SI x0)) "some-test-file.c":7)
+      (cnote 24 NOTE_INSN_EPILOGUE_BEG)
+      (cinsn 19 (use (reg:DI x30)) "some-test-file.c":7)
+      (cjump_insn 20 (return) "some-test-file.c":7)
+      (edge-to exit)
+    ) ;; block 2
+    (cbarrier 21)
+    (cnote 15 NOTE_INSN_DELETED)
+    (cnote 16 NOTE_INSN_DELETED)
+  ) ;; insn-chain
+  (crtl
+    (return_rtx 
+      (reg/i:SI x0)
+    ) ;; return_rtx
+  ) ;; crtl
+) ;; function "foo2"
+}
+

On Wed, 20 Nov 2019, Matthew Malcomson wrote:

> When compiling __RTL functions with a start pass, `skip_pass` needs to
> set global state when skipping a pass that would have marked something
> for future passes to see.
> 
> Existing examples are setting `reload_completed` and
> `epilogue_completed` when skipping the reload and pro_and_epilogue
> passes respectively.
> 
> This general problem occurs again in the AArch64 backend, where the
> cfun->machine->frame.laid_out variable is set on each function when
> `aarch64_layout_frame` is called (which can happen in both ira and
> reload.
> Currently any `return` statement will trigger an assert in the AArch64
> backend due to checking if address signing is enabled when the above
> variable is not set.
> 
> To account for the need to have a backend specific settings we add a
> target hook that each backend can implement as required for their own
> global state.
> 
> Here we also account for the motivating example by implementing this
> hook for the AArch64 backend.

Hmm, so this is only sensible when the RTL IL contains all state
besides the flag stating the state is present.  Which makes me
wonder how the aarch64 backend actually needs this flag and
why it is not derivable from existing global state
(reload_completed || lra_in_progress maybe?).

That is, this hook shouldn't be necessary, and if it is then
backends might be tempted to do actual work on the IL as if
the IL was in the state of the pass being skipped - which can be
not the case at all if the IL is for a much later pass.

So -- not OK without further explanation.

Thanks,
Richard.

> regtested on x86_64 and aarch64.
> 
> gcc/ChangeLog:
> 
> 2019-11-20  Matthew Malcomson  <matthew.malcomson@arm.com>
> 
> 	* config/aarch64/aarch64.c (aarch64_skip_pass): New.
> 	(TARGET_BACKEND_SKIP_PASS): New.
> 	* doc/tm.texi: Document BACKEND_SKIP_PASS hook.
> 	* doc/tm.texi.in: Document BACKEND_SKIP_PASS hook.
> 	* passes.c (skip_pass): Call targetm.backend_skip_pass if it's
> 	defined.
> 	* target.def (backend_skip_pass): Introduce new hook.
> 
> gcc/testsuite/ChangeLog:
> 
> 2019-11-20  Matthew Malcomson  <matthew.malcomson@arm.com>
> 
> 	* gcc.dg/rtl/aarch64/return-statement.c: New test.
> 
> 
> 
> ###############     Attachment also inlined for ease of reply    ###############
> 
> 
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 599d07a729e7438080f8b5240ee95037a49fb983..fbe900dec8d5ef3daf7be691cdaf4350c13d5024 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -75,6 +75,7 @@
>  #include "intl.h"
>  #include "expmed.h"
>  #include "function-abi.h"
> +#include "tree-pass.h"
>  
>  /* This file should be included last.  */
>  #include "target-def.h"
> @@ -21371,6 +21372,14 @@ aarch64_stack_protect_guard (void)
>    return NULL_TREE;
>  }
>  
> +/* Implement TARGET_BACKEND_SKIP_PASS.  We need to set
> +   cfun->machine->frame.laid_out when skipping the ira pass.  */
> +void aarch64_skip_pass (opt_pass *pass)
> +{
> +  if (strcmp (pass->name, "ira") == 0)
> +    cfun->machine->frame.laid_out = true;
> +}
> +
>  /* Implement TARGET_ASM_FILE_END for AArch64.  This adds the AArch64 GNU NOTE
>     section at the end if needed.  */
>  #define GNU_PROPERTY_AARCH64_FEATURE_1_AND	0xc0000000
> @@ -21947,6 +21956,9 @@ aarch64_libgcc_floating_mode_supported_p
>  #undef TARGET_STRICT_ARGUMENT_NAMING
>  #define TARGET_STRICT_ARGUMENT_NAMING hook_bool_CUMULATIVE_ARGS_true
>  
> +#undef TARGET_BACKEND_SKIP_PASS
> +#define TARGET_BACKEND_SKIP_PASS aarch64_skip_pass
> +
>  struct gcc_target targetm = TARGET_INITIALIZER;
>  
>  #include "gt-aarch64.h"
> diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
> index cd9aed9874f4e6b2b0e2f8956ed6155975e643a8..d02fd747b479246cfdf7c7e6af03a1a2bd43e1c4 100644
> --- a/gcc/doc/tm.texi
> +++ b/gcc/doc/tm.texi
> @@ -12152,3 +12152,7 @@ This target hook can be used to generate a target-specific code
>  @deftypefn {Target Hook} void TARGET_RUN_TARGET_SELFTESTS (void)
>  If selftests are enabled, run any selftests for this target.
>  @end deftypefn
> +
> +@deftypefn {Target Hook} void TARGET_BACKEND_SKIP_PASS (opt_pass *@var{pass})
> +Given pass currently skipping, run any actions to set up state.  When compiling code from the RTL frontend, some passes are skipped.  If a given pass would set backend state to mark that a given action has been done, then the backend should implement this hook to set the same state when that pass is skipped.  As an example, the AArch64 backend sets the backend specific cfun->machine->frame.laid_out structure member when the ira pass is skipped.
> +@end deftypefn
> diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
> index 2739e9ceec5ad7253ff9135da8dbe3bf6010e8d7..7cf446a88abe1eb3bfdcf1c685fc19905d46fca0 100644
> --- a/gcc/doc/tm.texi.in
> +++ b/gcc/doc/tm.texi.in
> @@ -8194,3 +8194,5 @@ maintainer is familiar with.
>  @hook TARGET_SPECULATION_SAFE_VALUE
>  
>  @hook TARGET_RUN_TARGET_SELFTESTS
> +
> +@hook TARGET_BACKEND_SKIP_PASS
> diff --git a/gcc/passes.c b/gcc/passes.c
> index d86af115ecb16fcab6bfce070f1f3e4f1d90ce71..16b3eb106f52da88d4a5717cdc4b66efd4f2c9d4 100644
> --- a/gcc/passes.c
> +++ b/gcc/passes.c
> @@ -2416,6 +2416,9 @@ skip_pass (opt_pass *pass)
>        rtl_register_cfg_hooks ();
>        cfun->curr_properties &= ~PROP_cfglayout;
>      }
> +
> +  if (targetm.backend_skip_pass)
> +    targetm.backend_skip_pass(pass);
>  }
>  
>  /* Execute PASS. */
> diff --git a/gcc/target.def b/gcc/target.def
> index 8e83c2c7a7136511c07a5bc9e18876c91a38b955..f5313fa1ebe50875559e68792991c6ea37e15bcb 100644
> --- a/gcc/target.def
> +++ b/gcc/target.def
> @@ -6765,6 +6765,12 @@ DEFHOOK
>   void, (void),
>   NULL)
>  
> +DEFHOOK
> +(backend_skip_pass,
> + "Given pass currently skipping, run any actions to set up state.  When compiling code from the RTL frontend, some passes are skipped.  If a given pass would set backend state to mark that a given action has been done, then the backend should implement this hook to set the same state when that pass is skipped.  As an example, the AArch64 backend sets the backend specific cfun->machine->frame.laid_out structure member when the ira pass is skipped.",
> + void, (opt_pass *pass), NULL)
> +
> +
>  /* Close the 'struct gcc_target' definition.  */
>  HOOK_VECTOR_END (C90_EMPTY_HACK)
>  
> diff --git a/gcc/testsuite/gcc.dg/rtl/aarch64/return-statement.c b/gcc/testsuite/gcc.dg/rtl/aarch64/return-statement.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..4fae39b4c3d657063f07f6c4a23e2eb7e1d46068
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/rtl/aarch64/return-statement.c
> @@ -0,0 +1,41 @@
> +/* { dg-do compile { target aarch64-*-* } } */
> +
> +/* Testing that the aarch64 `backend_skip_pass` implementation sets the
> +   relevant value so that `return` does not cause an assertion failure later
> +   on.  */
> +int __RTL (startwith ("cprop_hardreg")) foo2 (int a)
> +{
> +(function "foo2"
> +  (param "a"
> +    (DECL_RTL (reg/v:SI <1> [ a ]))
> +    (DECL_RTL_INCOMING (reg:SI x0 [ a ])))
> +  (insn-chain
> +    (cnote 1 NOTE_INSN_DELETED)
> +    (block 2
> +      (edge-from entry (flags "FALLTHRU"))
> +      (cnote 4 [bb 2] NOTE_INSN_BASIC_BLOCK)
> +      (cnote 17 NOTE_INSN_PROLOGUE_END)
> +      (cnote 2 NOTE_INSN_DELETED)
> +      (cnote 3 NOTE_INSN_FUNCTION_BEG)
> +      (cnote 6 NOTE_INSN_DELETED)
> +      (cinsn:TI 11 (set (reg/i:SI x0)
> +                    (plus:SI (reg:SI x0 [95])
> +                        (const_int 1))) "some-test-file.c":7)
> +      (cinsn 12 (use (reg/i:SI x0)) "some-test-file.c":7)
> +      (cnote 24 NOTE_INSN_EPILOGUE_BEG)
> +      (cinsn 19 (use (reg:DI x30)) "some-test-file.c":7)
> +      (cjump_insn 20 (return) "some-test-file.c":7)
> +      (edge-to exit)
> +    ) ;; block 2
> +    (cbarrier 21)
> +    (cnote 15 NOTE_INSN_DELETED)
> +    (cnote 16 NOTE_INSN_DELETED)
> +  ) ;; insn-chain
> +  (crtl
> +    (return_rtx 
> +      (reg/i:SI x0)
> +    ) ;; return_rtx
> +  ) ;; crtl
> +) ;; function "foo2"
> +}
> +
> 
>