On Tue, 5 Nov 2024, Jakub Jelinek wrote:

> Hi!
> 
> The following patch on top of the PR41045 toplevel extended asm patch
> allows marking inline asms (both toplevel and function-local, admittedly
> it is less useful for the latter, so if you want, I can add restrictions)
> as defining symbols, either functions or variables.
> 
> As most remaining constraint letters are used at least on some targets,
> I'm using @ as the new constraint.  It is similar to "s" in that it
> wants CONSTANT_P && !CONST_SCALAR_INT_P, but
> 1) it specially requires an address of a function or variable declaration,
>    so for functions the expected use is
> void foo (void);
> ...
> "@" (foo)
> or
> "@" (&foo)
> and for variables (unless they are arrays)
> int var;
> ...
> "@" (&var)
> 2) it makes no sense to say that either something is defined or it is
>    used in a register or something similar, so the patch diagnoses if
>    one attempts to mix it with other constraints; "@,@,@" is allowed
>    just because one could be using 3 alternatives in some other operand
> 3) unlike "s", the constraint doesn't check LEGITIMATE_PIC_OPERAND_P for
>    -fpic, even in -fpic one should be able to use it the same way
> 4) the cgraph portion needs to be really added later
> 5) and last but not least, I'm afraid %c0 print modifier isn't very
>    good for printing it; it works fine without -fpic/-fpie, but 'c'
>    modifier is handled as
>                 if (CONSTANT_ADDRESS_P (operands[opnum]))
>                   output_addr_const (asm_out_file, operands[opnum]);
>                 else
>                   output_operand (operands[opnum], 'c');
>    and because at least on some arches like x86 CONSTANT_ADDRESS_P
>    is redefined to do backend specific PIC mess, it will just
>    output_operand and likely just be rejected (on x86 with an error
>    that the argument is not a comparison)
>    Guess for x86 one can use %p0 instead.
>    But I'm afraid we are mostly out of generic modifiers,
>    and targetm.asm_out.print_operand_punct_valid_p seems to use most
>    of the punctuation characters as well.
>    I think ` is unused, but wonder if we want to use up the last
>    remaining letter that way, perhaps make %`<letter>0?
>    Or extend the existing generic modifiers, keep %c0 behave as it
>    does right now and make %cc0 be a 2 letter modifier which is
>    PIC friendly and prints using output_addr_const anything that can
>    be printed that way?

It would probably be cleanest to have a separate print modifier for
"symbol for assembler label definition" or so, but given this feature
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.

Richard.

> 2024-11-05  Jakub Jelinek  <ja...@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, "_a", 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, at sign 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 <rguent...@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)

Reply via email to