[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