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

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

[Segher Boessenkool]

Btw, if you just add

void *
retsp (void)
{
  register void *sp __asm ("sp");
  asm ("" : "+g" (sp));  // <-- this line
  return sp;
}

everything works fine.


Segher

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

[Jakub Jelinek]

On Wed, Nov 14, 2018 at 06:22:51AM -0600, Segher Boessenkool wrote:
> Btw, if you just add
> 
> void *
> retsp (void)
> {
>   register void *sp __asm ("sp");
>   asm ("" : "+g" (sp));  // <-- this line
>   return sp;
> }
> 
> everything works fine.

Even in what you are proposing, i.e. handle the var as any other var
in SSA form and only copy into the hard register right before asm and out of
it after it?
Because 
{
  void *sp;
  asm ("" : "+g" (sp));
  return sp;
}
would store into the register default definition of the SSA_NAME (the var
has no initializer).

	Jakub

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

[Alexander Monakov]

On Wed, 14 Nov 2018, Jakub Jelinek wrote:

> On Wed, Nov 14, 2018 at 06:22:51AM -0600, Segher Boessenkool wrote:
> > Btw, if you just add
> > 
> > void *
> > retsp (void)
> > {
> >   register void *sp __asm ("sp");
> >   asm ("" : "+g" (sp));  // <-- this line
> >   return sp;
> > }
> > 
> > everything works fine.
> 
> Even in what you are proposing, i.e. handle the var as any other var
> in SSA form and only copy into the hard register right before asm and out of
> it after it?
> Because 
> {
>   void *sp;
>   asm ("" : "+g" (sp));
>   return sp;
> }
> would store into the register default definition of the SSA_NAME (the var
> has no initializer).

I think with "=g" rather than "+g" this example is ok.

Alexander

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

[Segher Boessenkool]

On Wed, Nov 14, 2018 at 03:33:43PM +0300, Alexander Monakov wrote:
> On Wed, 14 Nov 2018, Jakub Jelinek wrote:
> 
> > On Wed, Nov 14, 2018 at 06:22:51AM -0600, Segher Boessenkool wrote:
> > > Btw, if you just add
> > > 
> > > void *
> > > retsp (void)
> > > {
> > >   register void *sp __asm ("sp");
> > >   asm ("" : "+g" (sp));  // <-- this line
> > >   return sp;
> > > }
> > > 
> > > everything works fine.
> > 
> > Even in what you are proposing, i.e. handle the var as any other var
> > in SSA form and only copy into the hard register right before asm and out of
> > it after it?
> > Because 
> > {
> >   void *sp;
> >   asm ("" : "+g" (sp));
> >   return sp;
> > }
> > would store into the register default definition of the SSA_NAME (the var
> > has no initializer).
> 
> I think with "=g" rather than "+g" this example is ok.

No, it needs the register var as an input.  That is the whole *point*.
It could output elsewhere, like with

void *
retsp (void)
{
  register void *sp __asm ("sp");
  void *p;
  asm ("" : "=g" (p) : "0" (sp));
  return p;
}

(which also works reliably with current GCC).

Or like

void *
retsp (void)
{
  register void *sp __asm ("sp");
  void *p;
  asm ("mov %0,%1" : "=r" (p) : "r" (sp));
  return p;
}

if you don't want to tie the asm input and output (but use an extra
machine instruction, alas).


Segher

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

[Alexander Monakov]

On Wed, 14 Nov 2018, Segher Boessenkool wrote:
> > I think with "=g" rather than "+g" this example is ok.
> 
> No, it needs the register var as an input.  That is the whole *point*.

Hm. I think I see what you meant, but "+g" is not correct either: the
asm, by intent, depends *on the current value in the 'sp' hardreg*, not
*on the current value of some automatic variable that is supposed to be
passed on the 'sp' hardreg to the asm* (which is what expressed by the
input constraint).

Consider what would happen in the scenario demonstrated in PR 89784:
suppose you have (e.g. after inlining 'retsp' in a loop):

  for (int i=0; i<2; i++)
    {
      register long sp asm ("%rsp");
      asm ("" : "+r" (sp));
      <code using sp>
    }

and then after unrolling

  register long sp asm ("%rsp");
  asm ("" : "+r" (sp));
  <code using sp>
  asm ("" : "+r" (sp));
  <code using sp>

where only the first asm has an uninitialized input, and the second asm
implies restoring hardreg %rsp to the value in variable sp.

So at a minimum you'd need to use two separate register variables:

  register long sp_in asm ("%rsp");
  register long sp asm ("%rsp");
  asm ("" : "=r" (sp) : "r" (sp_in));

Alexander

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

[Segher Boessenkool]

On Wed, Nov 14, 2018 at 06:50:38PM +0300, Alexander Monakov wrote:
> On Wed, 14 Nov 2018, Segher Boessenkool wrote:
> > > I think with "=g" rather than "+g" this example is ok.
> > 
> > No, it needs the register var as an input.  That is the whole *point*.
> 
> Hm. I think I see what you meant, but "+g" is not correct either: the
> asm, by intent, depends *on the current value in the 'sp' hardreg*, not
> *on the current value of some automatic variable that is supposed to be
> passed on the 'sp' hardreg to the asm* (which is what expressed by the
> input constraint).

Yeah, using local register vars to access global registers only works
by accident.  It does work currently though, and people apparently use
it, so we shouldn't break it :-/


Segher

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

[Alexander Monakov]

On Wed, 14 Nov 2018, Segher Boessenkool wrote:
> Yeah, using local register vars to access global registers only works
> by accident.  It does work currently though, and people apparently use
> it, so we shouldn't break it :-/

In the proposed approach (copying from/to pseudos just before/after the
asm) we can emulate historic behavior by making uninitialized pseudos
take values of the corresponding hardregs. If we decide that doing that
just for must-uninit pseudos is enough, it's a simple extension to the
existing init-regs pass.  Does this sound reasonable?

Alexander

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

[Michael Matz]

Hi,

On Wed, 14 Nov 2018, Alexander Monakov wrote:

> On Wed, 14 Nov 2018, Segher Boessenkool wrote:
> > Yeah, using local register vars to access global registers only works
> > by accident.  It does work currently though, and people apparently use
> > it, so we shouldn't break it :-/
> 
> In the proposed approach (copying from/to pseudos just before/after the
> asm) we can emulate historic behavior by making uninitialized pseudos
> take values of the corresponding hardregs. If we decide that doing that
> just for must-uninit pseudos is enough, it's a simple extension to the
> existing init-regs pass.  Does this sound reasonable?

Or we can just decide that nothing needs fixing.  In particular about this 
from Jakub and Segher:

> > What doesn't work as the reporter expect is assumption that local hard
> > register variables that live across function calls must have their 
> > values preserved; they can be modified by the callees.
> 
> It would be really nice if we could fix that :-)

I disagree that there's something to fix.  My mental model for local reg 
vars has always been that such vars are actually an alias for that 
register, not comparable to normal automatic variables (so, much like 
global reg vars, except that they don't reserve away the register from 
regalloc).  I.e. like volatile they can arbitrarily change their value.  
I don't know if other peoples mind model is similar, but it certainly is 
the model that is implemented in the GIMPLE pipeline (and if memory serves 
well of the RTL pipeline as well).

Copying outof/into pseudos around asms serves no purpose with that model, 
it rather makes regvars somewhat "safer" to use, without actually making 
them safer (there _is_ nothing safe about them).


Ciao,
Michael.

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

[Alexander Monakov]

On Thu, 15 Nov 2018, Michael Matz wrote:
> I disagree that there's something to fix.  My mental model for local reg 
> vars has always been that such vars are actually an alias for that 
> register, not comparable to normal automatic variables (so, much like 
> global reg vars, except that they don't reserve away the register from 
> regalloc).  I.e. like volatile they can arbitrarily change their value.  
> I don't know if other peoples mind model is similar, but it certainly is 
> the model that is implemented in the GIMPLE pipeline (and if memory serves 
> well of the RTL pipeline as well).

Reading the documentation certainly does not make that impression to me.
In any case, can you elaborate a bit further please:

1. Regarding the comparison to 'volatile' qualifier.  Suppose you have an
automatic variable 'int v;' in a correct program.  The variable is only used
for some arithmetic, never passed to asms, does not have its address taken.

Thus, changing its
declaration to 'volatile int v;' would not make the program invalid.  Now,
can declaring it as 'register int v asm("%rbx");' (with a callee-saved
register) make the program invalid?  My reading of the documentation is
that it would provide a regalloc hint and have no ill effects.


2. Are testcases given in PR 87984 valid? Quoting the latest example:

int f(void)
{
    int o=0, i;
    for (i=0; i<3; i++) {
        register int a asm("eax");
        a = 1;
        asm("add %1, %0" : "+r"(o) : "r"(a));
        asm("xor %%eax, %%eax" ::: "eax");
    }
    return o;
}

This follows both your model and the documentation to the letter, and
yet will return 1 rather than 3.

I disagree that it is practical to implement your model on GIMPLE.

Thanks.
Alexander

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

[Michael Matz]

Hi,

On Thu, 15 Nov 2018, Alexander Monakov wrote:

> Reading the documentation certainly does not make that impression to me. 
> In any case, can you elaborate a bit further please:
> 
> 1. Regarding the comparison to 'volatile' qualifier.  Suppose you have an
> automatic variable 'int v;' in a correct program.  The variable is only used
> for some arithmetic, never passed to asms, does not have its address taken.

I should have been more precise, I meant volatile and mapped to some 
device memory.  I.e. a register variable is never automatic in that sense.  
Every read can return different values even without intervening changes in 
source code.

> 2. Are testcases given in PR 87984 valid? Quoting the latest example:
> 
> int f(void)
> {
>     int o=0, i;
>     for (i=0; i<3; i++) {
>         register int a asm("eax");
>         a = 1;
>         asm("add %1, %0" : "+r"(o) : "r"(a));
>         asm("xor %%eax, %%eax" ::: "eax");
>     }
>     return o;
> }

Which is indeed what happens here.

> This follows both your model

Not really, it ignores the fact that 'a' can change at any time, which is 
what happens.

> and the documentation to the letter, and yet will return 1 rather than 
> 3.
> 
> I disagree that it is practical to implement your model on GIMPLE.

I claim that that is what is implemented right now.


Ciao,
Michael.

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

[Alexander Monakov]

On Fri, 16 Nov 2018, Michael Matz wrote:
> > This follows both your model
> 
> Not really, it ignores the fact that 'a' can change at any time, which is 
> what happens.

Are you saying that local register variables, in your model, are essentially
unfit for passing operands to inline asm on specific registers (i.e. their
primary purpose)?

Alexander

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

[Michael Matz]

Hi,

On Fri, 16 Nov 2018, Alexander Monakov wrote:

> > Not really, it ignores the fact that 'a' can change at any time, which 
> > is what happens.
> 
> Are you saying that local register variables, in your model, are 
> essentially unfit for passing operands to inline asm on specific 
> registers (i.e. their primary purpose)?

It seems so, yes.  Obviously we _want_ them to be used for this purpose, 
so something is wrong with my mental model :)  I think I'm starting to 
lean towards your model, hmm.


Ciao,
Michael.

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

[Segher Boessenkool]

Hi!

On Thu, Nov 15, 2018 at 03:54:03PM +0000, Michael Matz wrote:
> On Wed, 14 Nov 2018, Alexander Monakov wrote:
> > On Wed, 14 Nov 2018, Segher Boessenkool wrote:
> > > Yeah, using local register vars to access global registers only works
> > > by accident.  It does work currently though, and people apparently use
> > > it, so we shouldn't break it :-/
> > 
> > In the proposed approach (copying from/to pseudos just before/after the
> > asm) we can emulate historic behavior by making uninitialized pseudos
> > take values of the corresponding hardregs. If we decide that doing that
> > just for must-uninit pseudos is enough, it's a simple extension to the
> > existing init-regs pass.  Does this sound reasonable?
> 
> Or we can just decide that nothing needs fixing.  In particular about this 
> from Jakub and Segher:
> 
> > > What doesn't work as the reporter expect is assumption that local hard
> > > register variables that live across function calls must have their 
> > > values preserved; they can be modified by the callees.
> > 
> > It would be really nice if we could fix that :-)
> 
> I disagree that there's something to fix.

What I would like fixed is the surprising behaviour that people run into
time after time again.

> My mental model for local reg 
> vars has always been that such vars are actually an alias for that 
> register, not comparable to normal automatic variables (so, much like 
> global reg vars, except that they don't reserve away the register from 
> regalloc).

That was the case long ago, yes.  And mostly it still is, but this is not
guaranteed behaviour.  Things are expanded as being in that reg, but many
optimisations can destroy this later.  And function calls (some of which
are not explicit in the user's program!) will clobber things.  And and and.
It is very hard to use this feature reliably.

> I.e. like volatile they can arbitrarily change their value.  
> I don't know if other peoples mind model is similar, but it certainly is 
> the model that is implemented in the GIMPLE pipeline (and if memory serves 
> well of the RTL pipeline as well).
> 
> Copying outof/into pseudos around asms serves no purpose with that model, 
> it rather makes regvars somewhat "safer" to use, without actually making 
> them safer (there _is_ nothing safe about them).

The only supported case is for inputs and outputs to extended asm.  Maybe
we should just come up with a syntax so you can specify hard regs for that
directly (without needing a separate regclass for every reg).


Segher

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

[Michael Matz]

Hi,

On Fri, 16 Nov 2018, Segher Boessenkool wrote:

> > I.e. like volatile they can arbitrarily change their value.  I don't 
> > know if other peoples mind model is similar, but it certainly is the 
> > model that is implemented in the GIMPLE pipeline (and if memory serves 
> > well of the RTL pipeline as well).
> > 
> > Copying outof/into pseudos around asms serves no purpose with that 
> > model, it rather makes regvars somewhat "safer" to use, without 
> > actually making them safer (there _is_ nothing safe about them).
> 
> The only supported case is for inputs and outputs to extended asm.  
> Maybe we should just come up with a syntax so you can specify hard regs 
> for that directly (without needing a separate regclass for every reg).

I would like that, yes.  (Of course backward compat would have us support 
the historic usage for some time, but still ...)


Ciao,
Michael.

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

[Segher Boessenkool]

On Mon, Nov 19, 2018 at 12:59:29PM +0000, Michael Matz wrote:
> Hi,
> 
> On Fri, 16 Nov 2018, Segher Boessenkool wrote:
> 
> > > I.e. like volatile they can arbitrarily change their value.  I don't 
> > > know if other peoples mind model is similar, but it certainly is the 
> > > model that is implemented in the GIMPLE pipeline (and if memory serves 
> > > well of the RTL pipeline as well).
> > > 
> > > Copying outof/into pseudos around asms serves no purpose with that 
> > > model, it rather makes regvars somewhat "safer" to use, without 
> > > actually making them safer (there _is_ nothing safe about them).
> > 
> > The only supported case is for inputs and outputs to extended asm.  
> > Maybe we should just come up with a syntax so you can specify hard regs 
> > for that directly (without needing a separate regclass for every reg).
> 
> I would like that, yes.  (Of course backward compat would have us support 
> the historic usage for some time, but still ...)

Right, or for ten years at least (there are header files that use
register asm!)

I was thinking something like

  asm("smth %0,%1" : "=*r0"(x) : "*r1"(y));

where then "*" means the rest of the constraint is just a register name.

I don't know how easy this really is to make work, but do you think this
is a sensible syntax?


Segher

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

[Segher Boessenkool]

On Wed, Nov 14, 2018 at 01:27:26PM +0100, Jakub Jelinek wrote:
> On Wed, Nov 14, 2018 at 06:22:51AM -0600, Segher Boessenkool wrote:
> > Btw, if you just add
> > 
> > void *
> > retsp (void)
> > {
> >   register void *sp __asm ("sp");
> >   asm ("" : "+g" (sp));  // <-- this line
> >   return sp;
> > }
> > 
> > everything works fine.
> 
> Even in what you are proposing, i.e. handle the var as any other var
> in SSA form and only copy into the hard register right before asm and out of
> it after it?

Yes, *only* in that: with current trunk sp lives in the "sp" hard register
at the "return sp", which cannot work reliably (what value is returned?
It is unspecified).

> Because 
> {
>   void *sp;
>   asm ("" : "+g" (sp));
>   return sp;
> }
> would store into the register default definition of the SSA_NAME (the var
> has no initializer).

I'm more concerned about what it looks like in RTL, but sure :-)  What
*should* it do before RTL?  Not much at all I think, just keep track that
this var is a register asm and that's that?


Segher

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

[Martin Sebor]

[-- 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" } */

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

[Alexander Monakov]

On Tue, 13 Nov 2018, Martin Sebor wrote:

> 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.

I appreciate warnings for misuse of extensions in general, but in
this particular case I think GCC's behavior is misdesigned, so
instead of enshrining a bad engineering choice in a warning and
in the manual, I'd rather see GCC implement the extension sanely.

Merely changing a normal auto variable to a register asm variable
should not invalidate the program. As the manual says, it should
amount to providing a hint to the register allocator, at most.

Have a look at PR 87984, where code is miscompiled despite following
the documentation to the letter. This is because we lower accesses to
register variables when transitioning from GIMPLE to RTL incorrectly.
Fixing that should make the warning unnecessary. I hope I can work on
that before stage 4.

I think LLVM is doing the right thing there, and so should we.

Thanks.
Alexander

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

[Martin Sebor]

On 11/14/2018 02:39 AM, Alexander Monakov wrote:
> On Tue, 13 Nov 2018, Martin Sebor wrote:
>
>> 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.
>
> I appreciate warnings for misuse of extensions in general, but in
> this particular case I think GCC's behavior is misdesigned, so
> instead of enshrining a bad engineering choice in a warning and
> in the manual, I'd rather see GCC implement the extension sanely.
>
> Merely changing a normal auto variable to a register asm variable
> should not invalidate the program. As the manual says, it should
> amount to providing a hint to the register allocator, at most.
>
> Have a look at PR 87984, where code is miscompiled despite following
> the documentation to the letter. This is because we lower accesses to
> register variables when transitioning from GIMPLE to RTL incorrectly.
> Fixing that should make the warning unnecessary. I hope I can work on
> that before stage 4.
>
> I think LLVM is doing the right thing there, and so should we.

That would indeed be preferable but it's not something I expect
to have the cycles to work on.  I put the patch together only
because it seemed like an easy way to help keep users from
shooting themselves in the foot (to borrow the phrase from
the comment in PR 88000).

If there's consensus that the general use case of passing hard
registers to functions should be supported then I suggest to
acknowledge PR 88000 as a bug.  We can discuss whether it's
possible and worthwhile to warn on the unsupported subset of
the use cases.  In any case, I think the least we can for now
is to clarify in the manual what that supported subset is,
since as I (and others) read it, passing hard registers to
functions is not.

Martin

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

[Jakub Jelinek]

On Tue, Nov 13, 2018 at 09:11:41PM -0700, Martin Sebor wrote:
> --- 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);
> +}

This makes no sense to me.  There is nothing unsupported in passing
a local hard register variable to a function, that is well defined,
and as your testcase changes show, you broke quite some completely valid
testcases with that.

What doesn't work as the reporter expect is assumption that local hard
register variables that live across function calls must have their values
preserved; they can be modified by the callees.

	Jakub

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

[Segher Boessenkool]

On Wed, Nov 14, 2018 at 10:47:57AM +0100, Jakub Jelinek wrote:
> This makes no sense to me.  There is nothing unsupported in passing
> a local hard register variable to a function, that is well defined,
> and as your testcase changes show, you broke quite some completely valid
> testcases with that.

What could perhaps be useful is a warning for a local register var that
is not used in any asm?

> What doesn't work as the reporter expect is assumption that local hard
> register variables that live across function calls must have their values
> preserved; they can be modified by the callees.

It would be really nice if we could fix that :-)


Segher

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

[Jakub Jelinek]

On Wed, Nov 14, 2018 at 05:35:06AM -0600, Segher Boessenkool wrote:
> On Wed, Nov 14, 2018 at 10:47:57AM +0100, Jakub Jelinek wrote:
> > This makes no sense to me.  There is nothing unsupported in passing
> > a local hard register variable to a function, that is well defined,
> > and as your testcase changes show, you broke quite some completely valid
> > testcases with that.
> 
> What could perhaps be useful is a warning for a local register var that
> is not used in any asm?
> 
> > What doesn't work as the reporter expect is assumption that local hard
> > register variables that live across function calls must have their values
> > preserved; they can be modified by the callees.
> 
> It would be really nice if we could fix that :-)

You mean for all local hard register variables living across function calls
save them into temporary and restore them around the calls?
One issue is that those variables aren't in SSA form, so liveness analysis
is harder.  And, would we want to have an exception for the stack pointer?
I mean there is no need for register void *sp __asm ("rsp"); to be
saved/restored that way, it shouldn't change value across function calls.
Plus, as has been mentioned, function calls aren't the only issue here,
e.g. division/modulo etc. that require specific hard registers might clobber
them too.

	Jakub

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

[Segher Boessenkool]

On Wed, Nov 14, 2018 at 12:40:01PM +0100, Jakub Jelinek wrote:
> On Wed, Nov 14, 2018 at 05:35:06AM -0600, Segher Boessenkool wrote:
> > On Wed, Nov 14, 2018 at 10:47:57AM +0100, Jakub Jelinek wrote:
> > > This makes no sense to me.  There is nothing unsupported in passing
> > > a local hard register variable to a function, that is well defined,
> > > and as your testcase changes show, you broke quite some completely valid
> > > testcases with that.
> > 
> > What could perhaps be useful is a warning for a local register var that
> > is not used in any asm?
> > 
> > > What doesn't work as the reporter expect is assumption that local hard
> > > register variables that live across function calls must have their values
> > > preserved; they can be modified by the callees.
> > 
> > It would be really nice if we could fix that :-)
> 
> You mean for all local hard register variables living across function calls
> save them into temporary and restore them around the calls?
> One issue is that those variables aren't in SSA form, so liveness analysis
> is harder.  And, would we want to have an exception for the stack pointer?
> I mean there is no need for register void *sp __asm ("rsp"); to be
> saved/restored that way, it shouldn't change value across function calls.
> Plus, as has been mentioned, function calls aren't the only issue here,
> e.g. division/modulo etc. that require specific hard registers might clobber
> them too.

I was thinking put them in pseudos always, just copy them to the hard reg
right before the asm statements that use them (and the other way around,
for outputs).


Segher

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

[Jakub Jelinek]

On Wed, Nov 14, 2018 at 05:50:39AM -0600, Segher Boessenkool wrote:
> > You mean for all local hard register variables living across function calls
> > save them into temporary and restore them around the calls?
> > One issue is that those variables aren't in SSA form, so liveness analysis
> > is harder.  And, would we want to have an exception for the stack pointer?
> > I mean there is no need for register void *sp __asm ("rsp"); to be
> > saved/restored that way, it shouldn't change value across function calls.
> > Plus, as has been mentioned, function calls aren't the only issue here,
> > e.g. division/modulo etc. that require specific hard registers might clobber
> > them too.
> 
> I was thinking put them in pseudos always, just copy them to the hard reg
> right before the asm statements that use them (and the other way around,
> for outputs).

Wouldn't that break all the code that does:
void *
retsp (void)
{
  register void *sp __asm ("sp");
  return sp;
}
(or any other (usually fixed) reg, where the user code expects to just
read the value of that register)?
Storing uninitialized value into it would break the program miserably.

	Jakub

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

[Segher Boessenkool]

On Wed, Nov 14, 2018 at 01:00:43PM +0100, Jakub Jelinek wrote:
> On Wed, Nov 14, 2018 at 05:50:39AM -0600, Segher Boessenkool wrote:
> > > You mean for all local hard register variables living across function calls
> > > save them into temporary and restore them around the calls?
> > > One issue is that those variables aren't in SSA form, so liveness analysis
> > > is harder.  And, would we want to have an exception for the stack pointer?
> > > I mean there is no need for register void *sp __asm ("rsp"); to be
> > > saved/restored that way, it shouldn't change value across function calls.
> > > Plus, as has been mentioned, function calls aren't the only issue here,
> > > e.g. division/modulo etc. that require specific hard registers might clobber
> > > them too.
> > 
> > I was thinking put them in pseudos always, just copy them to the hard reg
> > right before the asm statements that use them (and the other way around,
> > for outputs).
> 
> Wouldn't that break all the code that does:
> void *
> retsp (void)
> {
>   register void *sp __asm ("sp");
>   return sp;
> }
> (or any other (usually fixed) reg, where the user code expects to just
> read the value of that register)?

It doesn't "break" anything because it currently isn't guaranteed to work
either.  There is __builtin_frame_address(0) for this of course (well,
almost the same semantics, and it is actually well-defined and actually
works).  Is there user code that tries to do this with a register var?

> Storing uninitialized value into it would break the program miserably.

There is no asm statement here.


Segher

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

[Jakub Jelinek]

On Wed, Nov 14, 2018 at 06:11:37AM -0600, Segher Boessenkool wrote:
> It doesn't "break" anything because it currently isn't guaranteed to work
> either.  There is __builtin_frame_address(0) for this of course (well,
> almost the same semantics, and it is actually well-defined and actually
> works).  Is there user code that tries to do this with a register var?

I've seen such code many times over the years, don't have time to perform
code searches for such kind of constructs now though.

	Jakub

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

[Martin Sebor]

On 11/14/2018 02:47 AM, Jakub Jelinek wrote:
> On Tue, Nov 13, 2018 at 09:11:41PM -0700, Martin Sebor wrote:
>> --- 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);
>> +}
>
> This makes no sense to me.  There is nothing unsupported in passing
> a local hard register variable to a function, that is well defined,

PR 88000 was resolved as invalid quoting the following sentence
from the manual as the rationale:

   The only supported use for this feature is to specify registers
   for input and output operands when calling Extended asm.

If these variables are meant to be supported in other contexts
(such as in calls to built-ins or even user-defined functions)
the manual needs to be clarified.

> and as your testcase changes show, you broke quite some completely valid
> testcases with that.

Let's not be melodramatic.  Nothing was "broken" by a proposed
warning.  But I did say that "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)."

> What doesn't work as the reporter expect is assumption that local hard
> register variables that live across function calls must have their values
> preserved; they can be modified by the callees.

I'm not sure I understand how what you described as supported
is different from what the test case in 88000 does: i.e., pass
the value of a register variable to a function and expect its
value to be unchanged.  If the value happens to be preserved
in some cases/for some registers but not for others, unless
the two sets can be reliably distinguished it seems like a good
candidate for a warning, to help users fall into the same trap.
Users who know what they're doing can easily suppress the warning
and others will fix their code.  Is there a way to do distinguish
the two sets of cases?

Martin