[RFC] inline asm, v2: Add new constraint for symbol definitions

On Wed, Nov 06, 2024 at 09:08:10AM +0100, Richard Biener wrote:
> It would probably be cleanest to have a separate print modifier for
> "symbol for assembler label definition" or so, but given this feature

See the patch I'll post next.

> targets existing uses those already know how to emit the definition
> without any operand or print modifier - something that should continue
> to work.  So maybe we should document that there isn't any print
> modifier and users need to arrange for mangling for themselves?
> 
> Otherwise I expected a symbol definition to be an asm output,
> not an input, but that's probably a bikeshedding minor detail.

I've considered that, but it would be much harder internally (output
operands generally need an lvalue, so this would need to be an exception
everywhere) and the operand argument is address of something (especially
so that one can just use function names in the operand and having another
exception to avoid function to function-pointer or array to pointer
promotions given specific constraints would be a nightmare), so that
certainly isn't modified.

Anyway, testing showed that @ isn't a good character, while the generic
code doesn't mention @ anywhere, some targets use @ in the output
constraints for the =@cc<something> syntax of testing flags;
while the @ I was proposing was an input constraint and in theory I could
just remove that
+	case '@':
+	  error ("%<@%> constraint used for output operand");
+	  return false;
hunk from the patch, if @ means something different in output
vs. input constraints, it would just confuse people.

So, here is the patch reworked to use : instead of @.
Mnemotechnically, : is better, the : character is what is used
in many assemblers after the symbol names for the symbol definitions.

2024-11-06  Jakub Jelinek  <jakub@redhat.com>

gcc/
	* genpreds.cc (mangle): Add ':' mangling.
	(add_constraint): Allow : constraint.
	* common.md (:): New define_constraint.
	* stmt.cc (parse_output_constraint): Diagnose "=:".
	(parse_input_constraint): Handle ":" and diagnose invalid
	uses.
	* doc/md.texi (Simple Constraints): Document ":" constraint.
gcc/c/
	* c-typeck.cc (build_asm_expr): Diagnose invalid ":" constraint
	uses.
gcc/cp/
	* semantics.cc (finish_asm_stmt): Diagnose invalid ":" constraint
	uses.
gcc/testsuite/
	* c-c++-common/toplevel-asm-4.c: New test.
	* c-c++-common/toplevel-asm-5.c: New test.


	Jakub

On Wed, 6 Nov 2024, Jakub Jelinek wrote:

> On Wed, Nov 06, 2024 at 09:08:10AM +0100, Richard Biener wrote:
> > It would probably be cleanest to have a separate print modifier for
> > "symbol for assembler label definition" or so, but given this feature
> 
> See the patch I'll post next.
> 
> > targets existing uses those already know how to emit the definition
> > without any operand or print modifier - something that should continue
> > to work.  So maybe we should document that there isn't any print
> > modifier and users need to arrange for mangling for themselves?
> > 
> > Otherwise I expected a symbol definition to be an asm output,
> > not an input, but that's probably a bikeshedding minor detail.
> 
> I've considered that, but it would be much harder internally (output
> operands generally need an lvalue, so this would need to be an exception
> everywhere) and the operand argument is address of something (especially
> so that one can just use function names in the operand and having another
> exception to avoid function to function-pointer or array to pointer
> promotions given specific constraints would be a nightmare), so that
> certainly isn't modified.

Ah, good enough reason then.

> Anyway, testing showed that @ isn't a good character, while the generic
> code doesn't mention @ anywhere, some targets use @ in the output
> constraints for the =@cc<something> syntax of testing flags;
> while the @ I was proposing was an input constraint and in theory I could
> just remove that
> +	case '@':
> +	  error ("%<@%> constraint used for output operand");
> +	  return false;
> hunk from the patch, if @ means something different in output
> vs. input constraints, it would just confuse people.
> 
> So, here is the patch reworked to use : instead of @.
> Mnemotechnically, : is better, the : character is what is used
> in many assemblers after the symbol names for the symbol definitions.

I probably couldn't care less ... sure, ':' works for me.  '.' would
be available as well, so would ".set" or ".def" (it doesn't need to be
a single letter?).

So fine with me, please leave frontend folks and others some time
to comment.

It warrants a changes.html entry and the documentation should probably
be amended with a more complete example also using the 'cc' print
modifier?

Thanks,
Richard.

> 2024-11-06  Jakub Jelinek  <jakub@redhat.com>
> 
> gcc/
> 	* genpreds.cc (mangle): Add ':' mangling.
> 	(add_constraint): Allow : constraint.
> 	* common.md (:): New define_constraint.
> 	* stmt.cc (parse_output_constraint): Diagnose "=:".
> 	(parse_input_constraint): Handle ":" and diagnose invalid
> 	uses.
> 	* doc/md.texi (Simple Constraints): Document ":" constraint.
> gcc/c/
> 	* c-typeck.cc (build_asm_expr): Diagnose invalid ":" constraint
> 	uses.
> gcc/cp/
> 	* semantics.cc (finish_asm_stmt): Diagnose invalid ":" constraint
> 	uses.
> gcc/testsuite/
> 	* c-c++-common/toplevel-asm-4.c: New test.
> 	* c-c++-common/toplevel-asm-5.c: New test.
> 
> --- gcc/genpreds.cc.jj	2024-02-10 11:25:10.404468273 +0100
> +++ gcc/genpreds.cc	2024-11-05 14:57:14.193060528 +0100
> @@ -753,6 +753,7 @@ mangle (const char *name)
>        case '_': obstack_grow (rtl_obstack, "__", 2); break;
>        case '<':	obstack_grow (rtl_obstack, "_l", 2); break;
>        case '>':	obstack_grow (rtl_obstack, "_g", 2); break;
> +      case ':': obstack_grow (rtl_obstack, "_c", 2); break;
>        default: obstack_1grow (rtl_obstack, *name); break;
>        }
>  
> @@ -797,12 +798,13 @@ add_constraint (const char *name, const
>    for (p = name; *p; p++)
>      if (!ISALNUM (*p))
>        {
> -	if (*p == '<' || *p == '>' || *p == '_')
> +	if (*p == '<' || *p == '>' || *p == '_' || *p == ':')
>  	  need_mangled_name = true;
>  	else
>  	  {
>  	    error_at (loc, "constraint name '%s' must be composed of letters,"
> -		      " digits, underscores, and angle brackets", name);
> +		      " digits, underscores, colon and angle brackets",
> +		      name);
>  	    return;
>  	  }
>        }
> --- gcc/common.md.jj	2024-01-03 11:51:24.519828508 +0100
> +++ gcc/common.md	2024-11-05 14:51:29.098989927 +0100
> @@ -100,6 +100,11 @@ (define_constraint "s"
>         (match_test "!CONST_SCALAR_INT_P (op)")
>         (match_test "!flag_pic || LEGITIMATE_PIC_OPERAND_P (op)")))
>  
> +(define_constraint ":"
> +  "Defines a symbol."
> +  (and (match_test "CONSTANT_P (op)")
> +       (match_test "!CONST_SCALAR_INT_P (op)")))
> +
>  (define_constraint "n"
>    "Matches a non-symbolic integer constant."
>    (and (match_test "CONST_SCALAR_INT_P (op)")
> --- gcc/stmt.cc.jj	2024-10-25 10:00:29.523767070 +0200
> +++ gcc/stmt.cc	2024-11-05 18:31:11.518948252 +0100
> @@ -278,6 +278,10 @@ parse_output_constraint (const char **co
>  	  error ("matching constraint not valid in output operand");
>  	  return false;
>  
> +	case ':':
> +	  error ("%<:%> constraint used for output operand");
> +	  return false;
> +
>  	case '<':  case '>':
>  	  /* ??? Before flow, auto inc/dec insns are not supposed to exist,
>  	     excepting those that expand_call created.  So match memory
> @@ -325,6 +329,7 @@ parse_input_constraint (const char **con
>    size_t c_len = strlen (constraint);
>    size_t j;
>    bool saw_match = false;
> +  bool at_checked = false;
>  
>    /* Assume the constraint doesn't allow the use of either
>       a register or memory.  */
> @@ -362,6 +367,21 @@ parse_input_constraint (const char **con
>        case 'N':  case 'O':  case 'P':  case ',':
>  	break;
>  
> +      case ':':
> +	/* Verify that if : is used, it is just ":" or say ":,:" but not
> +	   mixed with other constraints or say ",:,," etc.  */
> +	if (!at_checked)
> +	  {
> +	    for (size_t k = 0; k < c_len; ++k)
> +	      if (constraint[k] != ((k & 1) ? ',' : ':') || (c_len & 1) == 0)
> +		{
> +		  error ("%<:%> constraint mixed with other constraints");
> +		  return false;
> +		} 
> +	    at_checked = true;
> +	  }
> +	break;
> +
>  	/* Whether or not a numeric constraint allows a register is
>  	   decided by the matching constraint, and so there is no need
>  	   to do anything special with them.  We must handle them in
> --- gcc/doc/md.texi.jj	2024-10-16 14:41:45.553757783 +0200
> +++ gcc/doc/md.texi	2024-11-05 18:46:30.795896301 +0100
> @@ -1504,6 +1504,13 @@ as the predicate in the @code{match_oper
>  the mode specified in the @code{match_operand} as the mode of the memory
>  reference for which the address would be valid.
>  
> +@cindex @samp{:} in constraint
> +@item @samp{:}
> +This constraint, allowed only in input operands, says the inline @code{asm}
> +pattern defines specific function or variable symbol.  The constraint
> +shouldn't be mixed with other constraints on the same operand and
> +the operand should be address of a function or non-automatic variable.
> +
>  @cindex other register constraints
>  @cindex extensible constraints
>  @item @var{other-letters}
> --- gcc/c/c-typeck.cc.jj	2024-11-05 14:44:35.904892730 +0100
> +++ gcc/c/c-typeck.cc	2024-11-05 18:25:34.837723892 +0100
> @@ -12246,6 +12246,20 @@ build_asm_expr (location_t loc, tree str
>  	      error_at (loc, "invalid constraint outside of a function");
>  	      input = error_mark_node;
>  	    }
> +	  if (constraint[0] == ':' && input != error_mark_node)
> +	    {
> +	      tree t = input;
> +	      STRIP_NOPS (t);
> +	      if (TREE_CODE (t) != ADDR_EXPR
> +		  || !(TREE_CODE (TREE_OPERAND (t, 0)) == FUNCTION_DECL
> +		       || (VAR_P (TREE_OPERAND (t, 0))
> +			   && is_global_var (TREE_OPERAND (t, 0)))))
> +		{
> +		  error_at (loc, "%<:%> constraint operand is not address "
> +				 "of a function or non-automatic variable");
> +		  input = error_mark_node;
> +		}
> +	    }
>  	}
>        else
>  	input = error_mark_node;
> --- gcc/cp/semantics.cc.jj	2024-11-05 14:44:35.911892630 +0100
> +++ gcc/cp/semantics.cc	2024-11-05 18:27:05.162442682 +0100
> @@ -2325,6 +2325,20 @@ finish_asm_stmt (location_t loc, int vol
>  		  error_at (loc, "invalid constraint outside of a function");
>  		  operand = error_mark_node;
>  		}
> +	      if (constraint[0] == ':' && operand != error_mark_node)
> +		{
> +		  tree t = operand;
> +		  STRIP_NOPS (t);
> +		  if (TREE_CODE (t) != ADDR_EXPR
> +		      || !(TREE_CODE (TREE_OPERAND (t, 0)) == FUNCTION_DECL
> +			   || (VAR_P (TREE_OPERAND (t, 0))
> +			       && is_global_var (TREE_OPERAND (t, 0)))))
> +		    {
> +		      error_at (loc, "%<:%> constraint operand is not address "
> +				"of a function or non-automatic variable");
> +		      operand = error_mark_node;
> +		    }
> +		}
>  	    }
>  	  else
>  	    operand = error_mark_node;
> --- gcc/testsuite/c-c++-common/toplevel-asm-4.c.jj	2024-11-05 18:13:20.062136936 +0100
> +++ gcc/testsuite/c-c++-common/toplevel-asm-4.c	2024-11-05 18:36:26.800476151 +0100
> @@ -0,0 +1,9 @@
> +/* PR c/41045 */
> +/* { dg-do compile } */
> +/* { dg-options "-O0" } */
> +/* { dg-additional-options "-fno-pie" { target pie } } */
> +
> +int v[42], w;
> +void foo (void);
> +
> +asm ("# %c0: %c1:" :: ":" (foo), ":" (v), ":" (&w));
> --- gcc/testsuite/c-c++-common/toplevel-asm-5.c.jj	2024-11-05 18:14:44.449941185 +0100
> +++ gcc/testsuite/c-c++-common/toplevel-asm-5.c	2024-11-05 18:36:57.873035404 +0100
> @@ -0,0 +1,16 @@
> +/* PR c/41045 */
> +/* { dg-do compile } */
> +/* { dg-options "-O0" } */
> +/* { dg-additional-options "-fno-pie" { target pie } } */
> +
> +extern int v[42];
> +
> +asm ("# %0" : "=:" (32));		/* { dg-error "lvalue required in 'asm' statement" } */
> +					/* { dg-error "':' constraint used for output operand" "" { target *-*-* } .-1 } */
> +asm ("# %0" : "=:" (v));		/* { dg-error "':' constraint used for output operand" } */
> +asm ("# %0" : : "i:" (v));		/* { dg-error "':' constraint mixed with other constraints" } */
> +asm ("# %0" : : ":i" (v));		/* { dg-error "':' constraint mixed with other constraints" } */
> +asm ("# %0" : : ",:" (v));		/* { dg-error "':' constraint mixed with other constraints" } */
> +asm ("# %0" : : ":,:" (v));
> +asm ("# %0" : : ":," (v));		/* { dg-error "':' constraint mixed with other constraints" } */
> +asm ("# %0" : : ":,,:" (v));		/* { dg-error "':' constraint mixed with other constraints" } */
> 
> 	Jakub
> 
>

On Wed, Nov 06, 2024 at 03:13:10PM +0100, Richard Biener wrote:
> I probably couldn't care less ... sure, ':' works for me.  '.' would
> be available as well, so would ".set" or ".def" (it doesn't need to be
> a single letter?).
> 
> So fine with me, please leave frontend folks and others some time
> to comment.

The patch also depends on the toplevel asm patch, so I need to wait for that
too.
> 
> It warrants a changes.html entry

Sure, plan to do that for various recent changes.

> and the documentation should probably
> be amended with a more complete example also using the 'cc' print
> modifier?

The cc modifier patch adds an example there.

	Jakub