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 <[email protected]>
>
> 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
>
>
--
Richard Biener <[email protected]>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
