Re: [nvptx, PR 81662, committed] Error out on nvptx for fpatchable-function-entry

[Thomas Schwinge]

Hi Tom!

On Thu, 3 Aug 2017 13:23:16 +0200, Tom de Vries  wrote:
> [ was: Re: [testsuite, PR81662, committed] Skip 
> fpatchable-function-entry tests for nvptx ]
> 
> On 08/03/2017 09:17 AM, Tom de Vries wrote:
> > fpatchable-function-entry requires nop, which nvptx does not have.

Generally, should we perhaps use something like the following to at least
make it obvious in the generated PTX code that the compiler tried to
generate a "nop"?

--- gcc/config/nvptx/nvptx.md
+++ gcc/config/nvptx/nvptx.md
@@ -989,10 +989,11 @@
 
 ;; Miscellaneous
 
+;; PTX doesn't define a real "nop" instruction.
 (define_insn "nop"
   [(const_int 0)]
   ""
-  "")
+  "/* nop */")
 
 (define_insn "return"
   [(return)]

I have not tested this, have not verified whether we need to set the
"predicable" attribute to "false" here (existing problem maybe?), have
not thought about any other implications.  But given that "comments in
PTX are treated as whitespace", that should be fine?


> > I've disabled the corresponding test for nvptx.

Conceptually ACK.  Not useful on PTX.


> This patch errors out on nvptx for fpatchable-function-entry.

> --- a/gcc/config/nvptx/nvptx.c
> +++ b/gcc/config/nvptx/nvptx.c
> @@ -180,6 +180,10 @@ nvptx_option_override (void)
>if (!global_options_set.x_flag_no_common)
>  flag_no_common = 1;
>  
> +  /* The patch area requires nops, which we don't have.  */
> +  if (function_entry_patch_area_size > 0)
> +sorry ("not generating patch area, nops not supported");
> +
>/* Assumes that it will see only hard registers.  */
>flag_var_tracking = 0;

I noticed that this doesn't trigger if using "-fpatchable-function-entry"
during offloading compilation (but it does trigger for
"-foffload=-fpatchable-function-entry").  Is this a) intentional or by
accident, and/or b) desired?


> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/nvptx/patchable_function_entry-default.c
> @@ -0,0 +1,15 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fpatchable-function-entry=3,1" } */
> +
> +extern int a;
> +
> +int f3 (void);
> +
> +int
> +__attribute__((noinline))
> +f3 (void)
> +{
> +  return 5*a;
> +}
> +
> +/* { dg-excess-errors "sorry, unimplemented: not generating patch area, nops 
> not supported" } */

Given that the first argument of "dg-excess-errors" is just a comment,
shouldn't we instead explicitly scan for this specific diagnostic, while
also still watching for other excess errors?

--- gcc/testsuite/gcc.target/nvptx/patchable_function_entry-default.c
+++ gcc/testsuite/gcc.target/nvptx/patchable_function_entry-default.c
@@ -1,5 +1,6 @@
 /* { dg-do compile } */
 /* { dg-options "-O2 -fpatchable-function-entry=3,1" } */
+/* { dg-message "sorry, unimplemented: not generating patch area, nops not 
supported" "" { target *-*-* } 0 } */
 
 extern int a;
 
@@ -11,5 +12,3 @@ f3 (void)
 {
   return 5*a;
 }
-
-/* { dg-excess-errors "sorry, unimplemented: not generating patch area, nops 
not supported" } */


Grüße
 Thomas

[nvptx, PR 81662, committed] Error out on nvptx for fpatchable-function-entry

[Tom de Vries]

[ was: Re: [testsuite, PR81662, committed] Skip 
fpatchable-function-entry tests for nvptx ]


On 08/03/2017 09:17 AM, Tom de Vries wrote:

Hi,

fpatchable-function-entry requires nop, which nvptx does not have.

I've disabled the corresponding test for nvptx.


This patch errors out on nvptx for fpatchable-function-entry.

Committed.

Thanks,
- Tom
Error out on nvptx for fpatchable-function-entry

2017-08-03  Tom de Vries  

	PR target/81662
	* config/nvptx/nvptx.c (nvptx_option_override): Emit sorry if
	function_entry_patch_area_size > 0.

	* gcc.target/nvptx/patchable_function_entry-default.c: New test.

---
 gcc/config/nvptx/nvptx.c  |  4 
 .../gcc.target/nvptx/patchable_function_entry-default.c   | 15 +++
 2 files changed, 19 insertions(+)

diff --git a/gcc/config/nvptx/nvptx.c b/gcc/config/nvptx/nvptx.c
index 208b115..9211d1a 100644
--- a/gcc/config/nvptx/nvptx.c
+++ b/gcc/config/nvptx/nvptx.c
@@ -180,6 +180,10 @@ nvptx_option_override (void)
   if (!global_options_set.x_flag_no_common)
 flag_no_common = 1;
 
+  /* The patch area requires nops, which we don't have.  */
+  if (function_entry_patch_area_size > 0)
+sorry ("not generating patch area, nops not supported");
+
   /* Assumes that it will see only hard registers.  */
   flag_var_tracking = 0;
 
diff --git a/gcc/testsuite/gcc.target/nvptx/patchable_function_entry-default.c b/gcc/testsuite/gcc.target/nvptx/patchable_function_entry-default.c
new file mode 100644
index 000..4254456
--- /dev/null
+++ b/gcc/testsuite/gcc.target/nvptx/patchable_function_entry-default.c
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fpatchable-function-entry=3,1" } */
+
+extern int a;
+
+int f3 (void);
+
+int
+__attribute__((noinline))
+f3 (void)
+{
+  return 5*a;
+}
+
+/* { dg-excess-errors "sorry, unimplemented: not generating patch area, nops not supported" } */