[PATCH] diagnose unsupported uses of hardware register variables (PR 88000)

[Martin Sebor <msebor@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 908 bytes --]

In PR 88000 the reporter expects to be able to use an explicit
register local variable in a context where it isn't supported
i.e., for something other than an input or output of an asm
statement: namely to pass it as argument to a user-defined
function.  GCC emits unexpected object code in this case which
the reporter thought was a GCC bug.

Since explicit register global variables are supported in these
contexts, using the same kind of local variable seems like an easy
mistake to make.  To help users avoid it the attached patch adds
a warning that points it out.

Tested on x86_64-linux.  A number of i386 tests use explicit
register variables in calls to GCC library functions like
__builtin_fabsq.  I prune the warnings from those but if using
explicit register vars in those functions is meant to be supported
despite what the manual says the patch will need tweaking (as will
the manual).

Martin

[-- Attachment #2: gcc-88000.diff --]
[-- Type: text/x-patch, Size: 14464 bytes --]

PR c/88000 - Different local vars regs order may produce different code

gcc/c/ChangeLog:

	PR c/88000
	* c-typeck.c (convert_arguments):  Call warn_hw_reg_arg to diagnose
	unsupported uses of hardware register variables.

gcc/c-family/ChangeLog:

	PR c/88000
	* c.opt (-Wasm-register-var): New option.
	* c-common.h (warn_hw_reg_arg): Declare.
	* c-warn.c (warn_hw_reg_arg): Define.

gcc/cp/ChangeLog:

	PR c/88000
	* call.c (build_over_call): Call warn_hw_reg_arg.

gcc/testsuite/ChangeLog:

	PR c/88000
	* c-c++-common/Wasm-register.c: New test.
	* g++.dg/ext/Wasm-register.C: Same.
	* gcc.dg/torture/pr71762-3.c: Prune excess diagnostics.
	* gcc.target/i386/attr-returns_twice-1.c: Same.
	* gcc.target/i386/avx512dq-abs-copysign-1.c: Same.
	* gcc.target/i386/avx512dq-concatv2si-1.c: Same.
	* gcc.target/i386/avx512vl-abs-copysign-1.c: Same.
	* gcc.target/i386/avx512vl-abs-copysign-2.c: Same.
	* gcc.target/i386/avx512vl-concatv2si-1.c: Same.

gcc/ChangeLog:

	PR c/88000
	* doc/invoke.texi (-Wasm-register-var): Document.

Index: gcc/c/c-typeck.c
===================================================================
--- gcc/c/c-typeck.c	(revision 266086)
+++ gcc/c/c-typeck.c	(working copy)
@@ -3543,6 +3543,9 @@ convert_arguments (location_t loc, vec<location_t>
 	/* Convert `short' and `char' to full-size `int'.  */
 	parmval = default_conversion (val);
 
+      /* Diagnose uses of local variables declared asm register.  */
+      warn_hw_reg_arg (fundecl, parmnum + 1, val);
+
       (*values)[parmnum] = parmval;
       if (parmval == error_mark_node)
 	error_args = true;
Index: gcc/c-family/c.opt
===================================================================
--- gcc/c-family/c.opt	(revision 266086)
+++ gcc/c-family/c.opt	(working copy)
@@ -338,6 +338,10 @@ Warray-bounds=
 LangEnabledBy(C ObjC C++ LTO ObjC++,Wall,1,0)
 ; in common.opt
 
+Wasm-register-var
+C ObjC C++ ObjC++ Var(warn_asm_register_var) Warning Init(1)
+Warn for unsupported uses of variables declared asm register.
+
 Wassign-intercept
 ObjC ObjC++ Var(warn_assign_intercept) Warning
 Warn whenever an Objective-C assignment is being intercepted by the garbage collector.
Index: gcc/c-family/c-common.h
===================================================================
--- gcc/c-family/c-common.h	(revision 266086)
+++ gcc/c-family/c-common.h	(working copy)
@@ -1321,6 +1321,7 @@ extern bool diagnose_mismatched_attributes (tree,
 extern tree do_warn_duplicated_branches_r (tree *, int *, void *);
 extern void warn_for_multistatement_macros (location_t, location_t,
 					    location_t, enum rid);
+extern void warn_hw_reg_arg (tree, int, tree);
 
 /* In c-attribs.c.  */
 extern bool attribute_takes_identifier_p (const_tree);
Index: gcc/c-family/c-warn.c
===================================================================
--- gcc/c-family/c-warn.c	(revision 266086)
+++ gcc/c-family/c-warn.c	(working copy)
@@ -2609,3 +2609,39 @@ warn_for_multistatement_macros (location_t body_lo
     inform (guard_loc, "some parts of macro expansion are not guarded by "
 	    "this %qs clause", guard_tinfo_to_string (keyword));
 }
+
+
+/* Diagnose unsuported use of explicit hardware register variable ARG
+   as an argument ARGNO to function FNDECL.  */
+
+void
+warn_hw_reg_arg (tree fndecl, int argno, tree arg)
+{
+  if (!fndecl)
+    return;
+
+  /* Avoid diagnosing GCC intrinsics with no library fallbacks.  */
+  if (fndecl_built_in_p (fndecl)
+      && DECL_IS_BUILTIN (fndecl)
+      && !c_decl_implicit (fndecl)
+      && !DECL_ASSEMBLER_NAME_SET_P (fndecl))
+    return;
+
+  /* Also avoid diagnosing always_inline functions since those are
+     often used to implement vectorization intrinsics that make use
+     of hardware register variables.  */
+  if (lookup_attribute ("always_inline", DECL_ATTRIBUTES (fndecl)))
+    return;
+
+  /* Diagnose uses of local variables declared asm register.  */
+  STRIP_ANY_LOCATION_WRAPPER (arg);
+  if (VAR_P (arg)
+      && !TREE_STATIC (arg)
+      && DECL_HARD_REGISTER (arg)
+      && warning_at (input_location, OPT_Wasm_register_var,
+		     "unsupported use of hardware register %qD as "
+		     "argument %d of %qD",
+		     arg, argno, fndecl))
+    inform (DECL_SOURCE_LOCATION (arg),
+	    "%qD declared here", arg);
+}
Index: gcc/cp/call.c
===================================================================
--- gcc/cp/call.c	(revision 266086)
+++ gcc/cp/call.c	(working copy)
@@ -8111,6 +8111,9 @@ build_over_call (struct z_candidate *cand, int fla
 
       conv = convs[i];
 
+      /* Diagnose uses of local variables declared asm register.  */
+      warn_hw_reg_arg (cand->fn, i + 1, arg);
+
       /* If the argument is NULL and used to (implicitly) instantiate a
          template function (and bind one of the template arguments to
          the type of 'long int'), we don't want to warn about passing NULL
@@ -8207,6 +8210,9 @@ build_over_call (struct z_candidate *cand, int fla
   for (; arg_index < vec_safe_length (args); ++arg_index)
     {
       tree a = (*args)[arg_index];
+      /* Diagnose uses of local variables declared asm register.  */
+      warn_hw_reg_arg (cand->fn, arg_index + 1, a);
+
       if ((magic == 3 && arg_index == 2) || magic == 2)
 	{
 	  /* Do no conversions for certain magic varargs.  */
Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 266086)
+++ gcc/doc/invoke.texi	(working copy)
@@ -283,6 +283,7 @@ Objective-C and Objective-C++ Dialects}.
 -Walloc-zero  -Walloc-size-larger-than=@var{byte-size}
 -Walloca  -Walloca-larger-than=@var{byte-size} @gol
 -Wno-aggressive-loop-optimizations  -Warray-bounds  -Warray-bounds=@var{n} @gol
+-Wasm-register-var @gol
 -Wno-attributes -Wno-attribute-alias @gol
 -Wbool-compare  -Wbool-operation @gol
 -Wno-builtin-declaration-mismatch @gol
@@ -5868,6 +5869,13 @@ pointers. This warning level may give a larger num
 false positives and is deactivated by default.
 @end table
 
+@item -Wno-asm-register-var
+@opindex Wasm-register-var
+@opindex Wno-asm-register-var
+Warn about unsupported uses of hardware register variables, or those declared
+using the @code{asm register} extension (@pxref{Explicit Register Variables}).
+This warning is enabled by default.
+
 @item -Wattribute-alias=@var{n}
 @itemx -Wno-attribute-alias
 @opindex -Wattribute-alias
Index: gcc/testsuite/c-c++-common/Wasm-register.c
===================================================================
--- gcc/testsuite/c-c++-common/Wasm-register.c	(nonexistent)
+++ gcc/testsuite/c-c++-common/Wasm-register.c	(working copy)
@@ -0,0 +1,120 @@
+/* PR 88000 - Different local vars regs order may produce different code
+   Verify that passing asm register variables as arguments to functions
+   triggers a warning.
+   { dg-do compile }
+   { dg-options "-Wno-pedantic" } */
+
+#if __cplusplus
+/* Fake C++ function without a prototype.  */
+void fany (...);
+#else
+void fany ();
+#endif
+
+void fproto (char, int);
+void fellip (int, ...);
+
+register char gcr __asm__ ("r12");
+register long glr __asm__ ("r13");
+
+
+void test_global_no_proto (void)
+{
+  fany (gcr);
+  fany (0, gcr);
+  fany (glr);
+  fany (0, glr);
+}
+
+
+/* Verify that calls to a GCC intrinsic with no library fallback do
+   not trigger a warning.  */
+
+void test_gcc_builtin (void)
+{
+  register char cr __asm__ ("r11") = 1;
+  register int ir __asm__ ("r12") = 22;
+
+  __builtin_clz (cr);
+  __builtin_clz (ir);
+}
+
+
+/* Verify that calls to a GCC intrinsic with a library fallback do
+   trigger a warning.  */
+
+void test_lib_builtin (void)
+{
+  register char cr __asm__ ("r11") = 1;
+  register int ir __asm__ ("r12") = 22;
+
+  __builtin_abs (cr);     /* { dg-warning "unsupported use of hardware register .cr. as argument 1 of .\(int \)?__builtin_abs" } */
+
+  __builtin_abs (ir);     /* { dg-warning "unsupported use of hardware register .ir. as argument 1 " } */
+}
+
+
+void test_local_no_proto (void)
+{
+  register char cr __asm__ ("r11") = 1;
+  register int ir __asm__ ("r12") = 22;
+
+  fany (cr);          /* { dg-warning "unsupported use of hardware register .cr. as argument 1 of .\(void \)?fany" } */
+
+  fany (0, cr);       /* { dg-warning "unsupported use of hardware register .cr. as argument 2 " } */
+
+  fany (ir);          /* { dg-warning "unsupported use of hardware register .ir. as argument 1 " } */
+
+  fany (0, ir);       /* { dg-warning "unsupported use of hardware register .ir. as argument 2 " } */
+}
+
+
+void test_global_proto (void)
+{
+  fproto (gcr, 0);
+  fproto (0, gcr);
+  fproto (glr, 0);
+  fproto (0, glr);
+}
+
+
+void test_local_proto (void)
+{
+  register char cr __asm__ ("r11") = 1;
+  register int ir __asm__ ("r12") = 22;
+
+  fproto (cr, 0);     /* { dg-warning "unsupported use of hardware register .cr. as argument 1 of .\(void \)?fproto" } */
+
+  fproto (0, cr);     /* { dg-warning "unsupported use of hardware register .cr. as argument 2 " } */
+
+  fproto (ir, 0);     /* { dg-warning "unsupported use of hardware register .ir. as argument 1 " } */
+
+  fproto (0, ir);     /* { dg-warning "unsupported use of hardware register .ir. as argument 2 " } */
+}
+
+
+void test_global_ellip (void)
+{
+  fellip (gcr);
+  fellip (glr);
+  fellip (0, gcr);
+  fellip (0, glr);
+}
+
+
+void test_local_ellip (void)
+{
+  register char cr __asm__ ("r11") = 1;
+  register int ir __asm__ ("r12") = 22;
+
+  fellip (cr);        /* { dg-warning "unsupported use of hardware register .cr. as argument 1 of .\(void \)?fellip." } */
+
+  fellip (ir);        /* { dg-warning "unsupported use of hardware register .ir. as argument 1 " } */
+
+  fellip (0, cr);     /* { dg-warning "unsupported use of hardware register .cr. as argument 2 " } */
+
+  fellip (0, ir);     /* { dg-warning "unsupported use of hardware register .ir. as argument 2 " } */
+}
+
+
+/* { dg-prune-output "register used" } */
Index: gcc/testsuite/g++.dg/ext/Wasm-register.C
===================================================================
--- gcc/testsuite/g++.dg/ext/Wasm-register.C	(nonexistent)
+++ gcc/testsuite/g++.dg/ext/Wasm-register.C	(working copy)
@@ -0,0 +1,31 @@
+/* PR c/88000 - Different local vars regs order may produce different code
+   Verify that passing asm register variables as arguments to a function
+   template triggers a warning.
+   { dg-do compile }
+   { dg-options "-Wno-pedantic" } */
+
+template <class T>
+void f (T);
+
+register char gcr __asm__ ("r12");
+register long glr __asm__ ("r13");
+
+
+void test_global (void)
+{
+  f (gcr);
+  f (glr);
+}
+
+
+void test_local (void)
+{
+  register char cr __asm__ ("r11") = 1;
+  register int ir __asm__ ("r12") = 22;
+
+  f (cr);          /* { dg-warning "unsupported use of hardware register .cr. as argument 1 of .void f\\\(T\\\) \\\[with T = char]" } */
+
+  f (ir);          /* { dg-warning "unsupported use of hardware register .ir. as argument 1 of .void f\\\(T\\\) \\\[with T = int]" } */
+}
+
+/* { dg-prune-output "register used" } */
Index: gcc/testsuite/gcc.dg/torture/pr71762-3.c
===================================================================
--- gcc/testsuite/gcc.dg/torture/pr71762-3.c	(revision 266086)
+++ gcc/testsuite/gcc.dg/torture/pr71762-3.c	(working copy)
@@ -20,3 +20,5 @@ int main()
     __builtin_abort ();
   return 0;
 }
+
+/* { dg-prune-output "unsupported use of hardware register" } */
Index: gcc/testsuite/gcc.target/i386/attr-returns_twice-1.c
===================================================================
--- gcc/testsuite/gcc.target/i386/attr-returns_twice-1.c	(revision 266086)
+++ gcc/testsuite/gcc.target/i386/attr-returns_twice-1.c	(working copy)
@@ -21,3 +21,5 @@ main (void)
 
   return 0;
 }
+
+/* { dg-prune-output "unsupported use of hardware register" } */
Index: gcc/testsuite/gcc.target/i386/avx512dq-abs-copysign-1.c
===================================================================
--- gcc/testsuite/gcc.target/i386/avx512dq-abs-copysign-1.c	(revision 266086)
+++ gcc/testsuite/gcc.target/i386/avx512dq-abs-copysign-1.c	(working copy)
@@ -69,3 +69,4 @@ f6 (double x)
 /* { dg-final { scan-assembler "vandpd\[^\n\r\]*xmm18" } } */
 /* { dg-final { scan-assembler "vorpd\[^\n\r\]*xmm18" } } */
 /* { dg-final { scan-assembler "vxorpd\[^\n\r\]*xmm18" } } */
+/* { dg-prune-output "unsupported use of hardware register" } */
Index: gcc/testsuite/gcc.target/i386/avx512dq-concatv2si-1.c
===================================================================
--- gcc/testsuite/gcc.target/i386/avx512dq-concatv2si-1.c	(revision 266086)
+++ gcc/testsuite/gcc.target/i386/avx512dq-concatv2si-1.c	(working copy)
@@ -41,3 +41,4 @@ f3 (int x, int *y)
 }
 
 /* { dg-final { scan-assembler-times "vpinsrd\[^\n\r]*\\\$1\[^\n\r]*%xmm16\[^\n\r]*%xmm3" 2 } } */
+/* { dg-prune-output "unsupported use of hardware register" } */
Index: gcc/testsuite/gcc.target/i386/avx512vl-abs-copysign-1.c
===================================================================
--- gcc/testsuite/gcc.target/i386/avx512vl-abs-copysign-1.c	(revision 266086)
+++ gcc/testsuite/gcc.target/i386/avx512vl-abs-copysign-1.c	(working copy)
@@ -69,3 +69,4 @@ f6 (double x)
 /* { dg-final { scan-assembler "vpandq\[^\n\r\]*xmm18" } } */
 /* { dg-final { scan-assembler "vporq\[^\n\r\]*xmm18" } } */
 /* { dg-final { scan-assembler "vpxorq\[^\n\r\]*xmm18" } } */
+/* { dg-prune-output "unsupported use of hardware register" } */
Index: gcc/testsuite/gcc.target/i386/avx512vl-abs-copysign-2.c
===================================================================
--- gcc/testsuite/gcc.target/i386/avx512vl-abs-copysign-2.c	(revision 266086)
+++ gcc/testsuite/gcc.target/i386/avx512vl-abs-copysign-2.c	(working copy)
@@ -47,3 +47,4 @@ f4 (void)
 /* { dg-final { scan-assembler "vpandq\[^\n\r\]*xmm16" } } */
 /* { dg-final { scan-assembler "vporq\[^\n\r\]*xmm16" } } */
 /* { dg-final { scan-assembler "vpxorq\[^\n\r\]*xmm16" } } */
+/* { dg-prune-output "unsupported use of hardware register" } */
Index: gcc/testsuite/gcc.target/i386/avx512vl-concatv2si-1.c
===================================================================
--- gcc/testsuite/gcc.target/i386/avx512vl-concatv2si-1.c	(revision 266086)
+++ gcc/testsuite/gcc.target/i386/avx512vl-concatv2si-1.c	(working copy)
@@ -41,3 +41,4 @@ f3 (int x, int *y)
 }
 
 /* { dg-final { scan-assembler-not "vpinsrd\[^\n\r]*\\\$1\[^\n\r]*%xmm16\[^\n\r]*%xmm3" } } */
+/* { dg-prune-output "unsupported use of hardware register" } */