* [PATCH] Hard register asm constraint
@ 2024-05-24 9:13 Stefan Schulze Frielinghaus
2024-06-10 5:19 ` Stefan Schulze Frielinghaus
2024-06-27 7:45 ` Georg-Johann Lay
0 siblings, 2 replies; 14+ messages in thread
From: Stefan Schulze Frielinghaus @ 2024-05-24 9:13 UTC (permalink / raw)
To: gcc-patches; +Cc: Stefan Schulze Frielinghaus
This implements hard register constraints for inline asm. A hard register
constraint is of the form {regname} where regname is any valid register. This
basically renders register asm superfluous. For example, the snippet
int test (int x, int y)
{
register int r4 asm ("r4") = x;
register int r5 asm ("r5") = y;
unsigned int copy = y;
asm ("foo %0,%1,%2" : "+d" (r4) : "d" (r5), "d" (copy));
return r4;
}
could be rewritten into
int test (int x, int y)
{
asm ("foo %0,%1,%2" : "+{r4}" (x) : "{r5}" (y), "d" (y));
return x;
}
As a side-effect this also solves the problem of call-clobbered registers.
That being said, I was wondering whether we could utilize this feature in order
to get rid of local register asm automatically? For example, converting
// Result will be in r2 on s390
extern int bar (void);
void test (void)
{
register int x asm ("r2") = 42;
bar ();
asm ("foo %0\n" :: "r" (x));
}
into
void test (void)
{
int x = 42;
bar ();
asm ("foo %0\n" :: "{r2}" (x));
}
in order to get rid of the limitation of call-clobbered registers which may
lead to subtle bugs---especially if you think of non-obvious calls e.g.
introduced by sanitizer/tracer/whatever. Since such a transformation has the
potential to break existing code do you see any edge cases where this might be
problematic or even show stoppers? Currently, even
int test (void)
{
register int x asm ("r2") = 42;
register int y asm ("r2") = 24;
asm ("foo %0,%1\n" :: "r" (x), "r" (y));
}
is allowed which seems error prone to me. Thus, if 100% backwards
compatibility would be required, then automatically converting every register
asm to the new mechanism isn't viable. Still quite a lot could be transformed.
Any thoughts?
Currently I allow multiple alternatives as demonstrated by
gcc/testsuite/gcc.target/s390/asm-hard-reg-2.c. However, since a hard register
constraint is pretty specific I could also think of erroring out in case of
alternatives. Are there any real use cases out there for multiple
alternatives where one would like to use hard register constraints?
With the current implementation we have a "user visible change" in the sense
that for
void test (void)
{
register int x asm ("r2") = 42;
register int y asm ("r2") = 24;
asm ("foo %0,%1\n" : "=r" (x), "=r" (y));
}
we do not get the error
"invalid hard register usage between output operands"
anymore but rather
"multiple outputs to hard register: %r2"
This is due to the error handling in gimplify_asm_expr (). Speaking of errors,
I also error out earlier as before which means that e.g. in pr87600-2.c only
the first error is reported and processing is stopped afterwards which means
the subsequent tests fail.
I've been skimming through all targets and it looks to me as if none is using
curly brackets for their constraints. Of course, I may have missed something.
Cheers,
Stefan
PS: Current state for Clang: https://reviews.llvm.org/D105142
---
gcc/cfgexpand.cc | 42 -------
gcc/genpreds.cc | 4 +-
gcc/gimplify.cc | 115 +++++++++++++++++-
gcc/lra-constraints.cc | 17 +++
gcc/recog.cc | 14 ++-
gcc/stmt.cc | 102 +++++++++++++++-
gcc/stmt.h | 10 +-
.../gcc.target/s390/asm-hard-reg-1.c | 103 ++++++++++++++++
.../gcc.target/s390/asm-hard-reg-2.c | 29 +++++
.../gcc.target/s390/asm-hard-reg-3.c | 24 ++++
gcc/testsuite/lib/scanasm.exp | 4 +
11 files changed, 407 insertions(+), 57 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/s390/asm-hard-reg-1.c
create mode 100644 gcc/testsuite/gcc.target/s390/asm-hard-reg-2.c
create mode 100644 gcc/testsuite/gcc.target/s390/asm-hard-reg-3.c
diff --git a/gcc/cfgexpand.cc b/gcc/cfgexpand.cc
index 557cb28733b..47f71a2e803 100644
--- a/gcc/cfgexpand.cc
+++ b/gcc/cfgexpand.cc
@@ -2955,44 +2955,6 @@ expand_asm_loc (tree string, int vol, location_t locus)
emit_insn (body);
}
-/* Return the number of times character C occurs in string S. */
-static int
-n_occurrences (int c, const char *s)
-{
- int n = 0;
- while (*s)
- n += (*s++ == c);
- return n;
-}
-
-/* A subroutine of expand_asm_operands. Check that all operands have
- the same number of alternatives. Return true if so. */
-
-static bool
-check_operand_nalternatives (const vec<const char *> &constraints)
-{
- unsigned len = constraints.length();
- if (len > 0)
- {
- int nalternatives = n_occurrences (',', constraints[0]);
-
- if (nalternatives + 1 > MAX_RECOG_ALTERNATIVES)
- {
- error ("too many alternatives in %<asm%>");
- return false;
- }
-
- for (unsigned i = 1; i < len; ++i)
- if (n_occurrences (',', constraints[i]) != nalternatives)
- {
- error ("operand constraints for %<asm%> differ "
- "in number of alternatives");
- return false;
- }
- }
- return true;
-}
-
/* Check for overlap between registers marked in CLOBBERED_REGS and
anything inappropriate in T. Emit error and return the register
variable definition for error, NULL_TREE for ok. */
@@ -3158,10 +3120,6 @@ expand_asm_stmt (gasm *stmt)
= TREE_STRING_POINTER (TREE_VALUE (TREE_PURPOSE (t)));
}
- /* ??? Diagnose during gimplification? */
- if (! check_operand_nalternatives (constraints))
- return;
-
/* Count the number of meaningful clobbered registers, ignoring what
we would ignore later. */
auto_vec<rtx> clobber_rvec;
diff --git a/gcc/genpreds.cc b/gcc/genpreds.cc
index 55d149e8a40..f0d59cb0846 100644
--- a/gcc/genpreds.cc
+++ b/gcc/genpreds.cc
@@ -1148,7 +1148,7 @@ write_insn_constraint_len (void)
unsigned int i;
puts ("static inline size_t\n"
- "insn_constraint_len (char fc, const char *str ATTRIBUTE_UNUSED)\n"
+ "insn_constraint_len (char fc, const char *str)\n"
"{\n"
" switch (fc)\n"
" {");
@@ -1181,6 +1181,8 @@ write_insn_constraint_len (void)
puts (" default: break;\n"
" }\n"
+ " if (str[0] == '{')\n"
+ " return ((const char *)rawmemchr (str + 1, '}') - str) + 1;\n"
" return 1;\n"
"}\n");
}
diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc
index b0ed58ed0f9..b4b16e75023 100644
--- a/gcc/gimplify.cc
+++ b/gcc/gimplify.cc
@@ -70,6 +70,9 @@ along with GCC; see the file COPYING3. If not see
#include "omp-offload.h"
#include "context.h"
#include "tree-nested.h"
+#include "insn-config.h"
+#include "recog.h"
+#include "output.h"
/* Identifier for a basic condition, mapping it to other basic conditions of
its Boolean expression. Basic conditions given the same uid (in the same
@@ -6952,6 +6955,42 @@ gimplify_addr_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
return ret;
}
+/* Return the number of times character C occurs in string S. */
+
+static int
+num_occurrences (int c, const char *s)
+{
+ int n = 0;
+ while (*s)
+ n += (*s++ == c);
+ return n;
+}
+
+/* A subroutine of gimplify_asm_expr. Check that all operands have
+ the same number of alternatives. Return -1 if this is violated. Otherwise
+ return the number of alternatives. */
+
+static int
+num_alternatives (const_tree link)
+{
+ if (link == nullptr)
+ return 0;
+
+ const char *constraint = TREE_STRING_POINTER (TREE_VALUE (TREE_PURPOSE (link)));
+ int num = num_occurrences (',', constraint);
+
+ if (num + 1 > MAX_RECOG_ALTERNATIVES)
+ return -1;
+
+ for (link = TREE_CHAIN (link); link; link = TREE_CHAIN (link))
+ {
+ constraint = TREE_STRING_POINTER (TREE_VALUE (TREE_PURPOSE (link)));
+ if (num_occurrences (',', constraint) != num)
+ return -1;
+ }
+ return num + 1;
+}
+
/* Gimplify the operands of an ASM_EXPR. Input operands should be a gimple
value; output operands should be a gimple lvalue. */
@@ -6982,6 +7021,35 @@ gimplify_asm_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
clobbers = NULL;
labels = NULL;
+ int num_alternatives_out = num_alternatives (ASM_OUTPUTS (expr));
+ int num_alternatives_in = num_alternatives (ASM_INPUTS (expr));
+ if (num_alternatives_out == -1 || num_alternatives_in == -1
+ || (num_alternatives_out > 0 && num_alternatives_in > 0
+ && num_alternatives_out != num_alternatives_in))
+ {
+ error ("operand constraints for %<asm%> differ "
+ "in number of alternatives");
+ return GS_ERROR;
+ }
+ int num_alternatives = MAX (num_alternatives_out, num_alternatives_in);
+
+ /* Regarding hard register constraints ensure that each hard register is used
+ at most once over all inputs/outputs and each alternative. Keep track in
+ hardregs[0] which hard register is used via an asm register over all
+ inputs/outputs. hardregs[i] for i >= 2 describes which hard registers are
+ used for alternative i-2 over all inputs/outputs. hardregs[1] is a
+ reduction of all alternatives, i.e., hardregs[1] |= hardregs[i] for i >= 2
+ and describes whether a hard register is used in any alternative. This is
+ just a shortcut instead of recomputing the union over all alternatives;
+ possibly multiple times. */
+ auto_vec<HARD_REG_SET> hardregs (num_alternatives + 2);
+ for (int i = 0; i < num_alternatives + 2; ++i)
+ {
+ HARD_REG_SET hregset;
+ CLEAR_HARD_REG_SET (hregset);
+ hardregs.quick_push (hregset);
+ }
+
ret = GS_ALL_DONE;
link_next = NULL_TREE;
for (i = 0, link = ASM_OUTPUTS (expr); link; ++i, link = link_next)
@@ -6998,8 +7066,8 @@ gimplify_asm_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
if (constraint_len == 0)
continue;
- ok = parse_output_constraint (&constraint, i, 0, 0,
- &allows_mem, &allows_reg, &is_inout);
+ ok = parse_output_constraint (&constraint, i, 0, 0, &allows_mem,
+ &allows_reg, &is_inout, &hardregs);
if (!ok)
{
ret = GS_ERROR;
@@ -7062,6 +7130,24 @@ gimplify_asm_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
TREE_VALUE (link) = tem;
tret = GS_OK;
}
+ if (VAR_P (op) && DECL_HARD_REGISTER (op))
+ {
+ tree id = DECL_ASSEMBLER_NAME (op);
+ const char *asmspec = IDENTIFIER_POINTER (id) + 1;
+ int hardreg = decode_reg_name (asmspec);
+ if (hardreg >= 0)
+ {
+ if (TEST_HARD_REG_BIT (hardregs[0], hardreg)
+ || TEST_HARD_REG_BIT (hardregs[1], hardreg))
+ {
+ error ("multiple outputs to hard register: %s",
+ reg_names[hardreg]);
+ return GS_ERROR;
+ }
+ else
+ SET_HARD_REG_BIT (hardregs[0], hardreg);
+ }
+ }
}
vec_safe_push (outputs, link);
@@ -7161,13 +7247,16 @@ gimplify_asm_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
}
}
+ for (unsigned int i = 0; i < hardregs.length (); ++i)
+ CLEAR_HARD_REG_SET (hardregs[i]);
+
link_next = NULL_TREE;
for (link = ASM_INPUTS (expr); link; ++i, link = link_next)
{
link_next = TREE_CHAIN (link);
constraint = TREE_STRING_POINTER (TREE_VALUE (TREE_PURPOSE (link)));
- parse_input_constraint (&constraint, 0, 0, noutputs, 0,
- oconstraints, &allows_mem, &allows_reg);
+ parse_input_constraint (&constraint, 0, 0, noutputs, 0, oconstraints,
+ &allows_mem, &allows_reg, &hardregs);
/* If we can't make copies, we can only accept memory. */
tree intype = TREE_TYPE (TREE_VALUE (link));
@@ -7241,6 +7330,24 @@ gimplify_asm_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
is_gimple_asm_val, fb_rvalue);
if (tret == GS_ERROR)
ret = tret;
+ tree inputv = TREE_VALUE (link);
+ if (VAR_P (inputv) && DECL_HARD_REGISTER (inputv))
+ {
+ tree id = DECL_ASSEMBLER_NAME (inputv);
+ const char *asmspec = IDENTIFIER_POINTER (id) + 1;
+ int hardreg = decode_reg_name (asmspec);
+ if (hardreg >= 0)
+ {
+ if (TEST_HARD_REG_BIT (hardregs[1], hardreg))
+ {
+ error ("multiple inputs to hard register: %s",
+ reg_names[hardreg]);
+ return GS_ERROR;
+ }
+ else
+ SET_HARD_REG_BIT (hardregs[0], hardreg);
+ }
+ }
}
TREE_CHAIN (link) = NULL_TREE;
diff --git a/gcc/lra-constraints.cc b/gcc/lra-constraints.cc
index e945a4da451..d81753eefaa 100644
--- a/gcc/lra-constraints.cc
+++ b/gcc/lra-constraints.cc
@@ -114,6 +114,7 @@
#include "target.h"
#include "rtl.h"
#include "tree.h"
+#include "stmt.h"
#include "predict.h"
#include "df.h"
#include "memmodel.h"
@@ -2165,6 +2166,7 @@ process_alt_operands (int only_alternative)
bool costly_p;
enum reg_class cl;
const HARD_REG_SET *cl_filter;
+ HARD_REG_SET hregset;
/* Calculate some data common for all alternatives to speed up the
function. */
@@ -2536,6 +2538,21 @@ process_alt_operands (int only_alternative)
cl_filter = nullptr;
goto reg;
+ case '{':
+ {
+ /* Currently this form of constraint is only allowed in
+ asm statements which are verified during gimplify,
+ i.e., regno >= 0 holds for those. genoutput fails on
+ it. For future proofness assert it. */
+ int regno = parse_constraint_regname (p);
+ gcc_assert (regno >= 0);
+ cl = REGNO_REG_CLASS (regno);
+ CLEAR_HARD_REG_SET (hregset);
+ SET_HARD_REG_BIT (hregset, regno);
+ cl_filter = &hregset;
+ goto reg;
+ }
+
default:
cn = lookup_constraint (p);
switch (get_constraint_type (cn))
diff --git a/gcc/recog.cc b/gcc/recog.cc
index a6799e3f5e6..8a474cfb8a7 100644
--- a/gcc/recog.cc
+++ b/gcc/recog.cc
@@ -25,6 +25,7 @@ along with GCC; see the file COPYING3. If not see
#include "target.h"
#include "rtl.h"
#include "tree.h"
+#include "stmt.h"
#include "cfghooks.h"
#include "df.h"
#include "memmodel.h"
@@ -2296,10 +2297,11 @@ asm_operand_ok (rtx op, const char *constraint, const char **constraints)
switch (get_constraint_type (cn))
{
case CT_REGISTER:
- if (!result
- && reg_class_for_constraint (cn) != NO_REGS
- && GET_MODE (op) != BLKmode
- && register_operand (op, VOIDmode))
+ if ((!result
+ && reg_class_for_constraint (cn) != NO_REGS
+ && GET_MODE (op) != BLKmode
+ && register_operand (op, VOIDmode))
+ || constraint[0] == '{')
result = 1;
break;
@@ -3231,6 +3233,10 @@ constrain_operands (int strict, alternative_mask alternatives)
win = true;
break;
+ case '{':
+ win = true;
+ break;
+
default:
{
enum constraint_num cn = lookup_constraint (p);
diff --git a/gcc/stmt.cc b/gcc/stmt.cc
index ae1527f0a19..1f20b09f90e 100644
--- a/gcc/stmt.cc
+++ b/gcc/stmt.cc
@@ -39,6 +39,7 @@ along with GCC; see the file COPYING3. If not see
#include "emit-rtl.h"
#include "pretty-print.h"
#include "diagnostic-core.h"
+#include "output.h"
#include "fold-const.h"
#include "varasm.h"
@@ -174,6 +175,32 @@ expand_label (tree label)
maybe_set_first_label_num (label_r);
}
\f
+/* Parse a hard register constraint and return its number or -1 in case of an
+ error. BEGIN should point to a string of the form "{regname}". For the
+ sake of simplicity assume that a register name is not longer than 31
+ characters, if not error out. */
+
+int
+parse_constraint_regname (const char *begin)
+{
+ if (*begin != '{')
+ return -1;
+ ++begin;
+ const char *end = begin;
+ while (*end != '}' && *end != '\0')
+ ++end;
+ if (*end != '}' || end == begin)
+ return -1;
+ ptrdiff_t len = end - begin;
+ if (len >= 31)
+ return -1;
+ char regname[32];
+ memcpy (regname, begin, len);
+ regname[len] = '\0';
+ int regno = decode_reg_name (regname);
+ return regno;
+}
+
/* Parse the output constraint pointed to by *CONSTRAINT_P. It is the
OPERAND_NUMth output operand, indexed from zero. There are NINPUTS
inputs and NOUTPUTS outputs to this extended-asm. Upon return,
@@ -190,7 +217,8 @@ expand_label (tree label)
bool
parse_output_constraint (const char **constraint_p, int operand_num,
int ninputs, int noutputs, bool *allows_mem,
- bool *allows_reg, bool *is_inout)
+ bool *allows_reg, bool *is_inout,
+ vec<HARD_REG_SET> *hardregs)
{
const char *constraint = *constraint_p;
const char *p;
@@ -244,6 +272,8 @@ parse_output_constraint (const char **constraint_p, int operand_num,
constraint = *constraint_p;
}
+ unsigned int alternative = 2;
+
/* Loop through the constraint string. */
for (p = constraint + 1; *p; )
{
@@ -268,7 +298,11 @@ parse_output_constraint (const char **constraint_p, int operand_num,
case 'E': case 'F': case 'G': case 'H':
case 's': case 'i': case 'n':
case 'I': case 'J': case 'K': case 'L': case 'M':
- case 'N': case 'O': case 'P': case ',':
+ case 'N': case 'O': case 'P':
+ break;
+
+ case ',':
+ ++alternative;
break;
case '0': case '1': case '2': case '3': case '4':
@@ -289,6 +323,33 @@ parse_output_constraint (const char **constraint_p, int operand_num,
*allows_mem = true;
break;
+ case '{':
+ {
+ int regno = parse_constraint_regname (p);
+ if (regno < 0)
+ {
+ error ("invalid output constraint: %s", p);
+ return false;
+ }
+ if (hardregs)
+ {
+ if (TEST_HARD_REG_BIT ((*hardregs)[0], regno)
+ || TEST_HARD_REG_BIT ((*hardregs)[alternative], regno))
+ {
+ error ("multiple outputs to hard register: %s",
+ reg_names[regno]);
+ return false;
+ }
+ else
+ {
+ SET_HARD_REG_BIT ((*hardregs)[1], regno);
+ SET_HARD_REG_BIT ((*hardregs)[alternative], regno);
+ }
+ }
+ *allows_reg = true;
+ break;
+ }
+
default:
if (!ISALPHA (*p))
break;
@@ -317,7 +378,8 @@ bool
parse_input_constraint (const char **constraint_p, int input_num,
int ninputs, int noutputs, int ninout,
const char * const * constraints,
- bool *allows_mem, bool *allows_reg)
+ bool *allows_mem, bool *allows_reg,
+ vec<HARD_REG_SET> *hardregs)
{
const char *constraint = *constraint_p;
const char *orig_constraint = constraint;
@@ -332,6 +394,8 @@ parse_input_constraint (const char **constraint_p, int input_num,
/* Make sure constraint has neither `=', `+', nor '&'. */
+ unsigned int alternative = 2;
+
for (j = 0; j < c_len; j += CONSTRAINT_LEN (constraint[j], constraint+j))
switch (constraint[j])
{
@@ -358,7 +422,11 @@ parse_input_constraint (const char **constraint_p, int input_num,
case 'E': case 'F': case 'G': case 'H':
case 's': case 'i': case 'n':
case 'I': case 'J': case 'K': case 'L': case 'M':
- case 'N': case 'O': case 'P': case ',':
+ case 'N': case 'O': case 'P':
+ break;
+
+ case ',':
+ ++alternative;
break;
/* Whether or not a numeric constraint allows a register is
@@ -408,6 +476,32 @@ parse_input_constraint (const char **constraint_p, int input_num,
*allows_mem = true;
break;
+ case '{':
+ {
+ int regno = parse_constraint_regname (constraint + j);
+ if (regno < 0)
+ {
+ error ("invalid input constraint: %s", constraint + j);
+ return false;
+ }
+ if (hardregs)
+ {
+ if (TEST_HARD_REG_BIT ((*hardregs)[0], regno)
+ || TEST_HARD_REG_BIT ((*hardregs)[alternative], regno))
+ {
+ error ("multiple inputs to hard register: %s",
+ reg_names[regno]);
+ }
+ else
+ {
+ SET_HARD_REG_BIT ((*hardregs)[1], regno);
+ SET_HARD_REG_BIT ((*hardregs)[alternative], regno);
+ }
+ }
+ *allows_reg = true;
+ break;
+ }
+
default:
if (! ISALPHA (constraint[j]))
{
diff --git a/gcc/stmt.h b/gcc/stmt.h
index a2caae7121b..05889ff3798 100644
--- a/gcc/stmt.h
+++ b/gcc/stmt.h
@@ -20,11 +20,17 @@ along with GCC; see the file COPYING3. If not see
#ifndef GCC_STMT_H
#define GCC_STMT_H
+#include "target.h"
+#include "hard-reg-set.h"
+
extern void expand_label (tree);
extern bool parse_output_constraint (const char **, int, int, int,
- bool *, bool *, bool *);
+ bool *, bool *, bool *,
+ vec<HARD_REG_SET> * = nullptr);
extern bool parse_input_constraint (const char **, int, int, int, int,
- const char * const *, bool *, bool *);
+ const char * const *, bool *, bool *,
+ vec<HARD_REG_SET> * = nullptr);
+extern int parse_constraint_regname (const char *);
extern tree resolve_asm_operand_names (tree, tree, tree, tree);
#ifdef HARD_CONST
/* Silly ifdef to avoid having all includers depend on hard-reg-set.h. */
diff --git a/gcc/testsuite/gcc.target/s390/asm-hard-reg-1.c b/gcc/testsuite/gcc.target/s390/asm-hard-reg-1.c
new file mode 100644
index 00000000000..53895d98663
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/asm-hard-reg-1.c
@@ -0,0 +1,103 @@
+/* { dg-do compile { target { lp64 } } } */
+/* { dg-options "-O2 -march=z13" } */
+/* { dg-final { check-function-bodies "**" "" "" } } */
+
+/*
+** test_in_1:
+** foo %r2
+** br %r14
+*/
+
+int
+test_in_1 (int x)
+{
+ asm ("foo %0" :: "{r2}" (x));
+ return x;
+}
+
+/*
+** test_in_2:
+** lgr (%r[0-9]+),%r2
+** lhi %r2,42
+** foo %r2
+** lgr %r2,\1
+** br %r14
+*/
+
+int
+test_in_2 (int x)
+{
+ asm ("foo %0" :: "{r2}" (42));
+ return x;
+}
+
+/*
+** test_in_3:
+** stmg %r12,%r15,96\(%r15\)
+** lay %r15,-160\(%r15\)
+** lgr (%r[0-9]+),%r2
+** ahi %r2,1
+** lgfr %r2,%r2
+** brasl %r14,foo@PLT
+** lr %r3,%r2
+** lr %r2,\1
+** foo %r3,%r2
+** lgr %r2,\1
+** lmg %r12,%r15,256\(%r15\)
+** br %r14
+*/
+
+extern int foo (int);
+
+int
+test_in_3 (int x)
+{
+ asm ("foo %0,%1\n" :: "{r3}" (foo (x + 1)), "{r2}" (x));
+ return x;
+}
+
+/*
+** test_out_1:
+** foo %r3
+** lgfr %r2,%r3
+** br %r14
+*/
+
+int
+test_out_1 (void)
+{
+ int x;
+ asm ("foo %0" : "={r3}" (x));
+ return x;
+}
+
+/*
+** test_out_2:
+** lgr (%r[0-9]+),%r2
+** foo %r2
+** ark (%r[0-9]+),\1,%r2
+** lgfr %r2,\2
+** br %r14
+*/
+
+int
+test_out_2 (int x)
+{
+ int y;
+ asm ("foo %0" : "={r2}" (y));
+ return x + y;
+}
+
+/*
+** test_inout_1:
+** foo %r2
+** lgfr %r2,%r2
+** br %r14
+*/
+
+int
+test_inout_1 (int x)
+{
+ asm ("foo %0" : "+{r2}" (x));
+ return x;
+}
diff --git a/gcc/testsuite/gcc.target/s390/asm-hard-reg-2.c b/gcc/testsuite/gcc.target/s390/asm-hard-reg-2.c
new file mode 100644
index 00000000000..9f3c221b937
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/asm-hard-reg-2.c
@@ -0,0 +1,29 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+/* { dg-final { check-function-bodies "**" "" "" } } */
+
+/*
+** test_1:
+** lr %r5,%r2
+** foo %r5,%r3
+** br %r14
+*/
+
+void
+test_1 (int x, int *y)
+{
+ asm ("foo %0,%1" :: "m{r4},{r5}" (x), "m,r" (y));
+}
+
+/*
+** test_2:
+** lr %r4,%r2
+** foo %r4,0\(%r3\)
+** br %r14
+*/
+
+void
+test_2 (int x, int *y)
+{
+ asm ("foo %0,%1" :: "m{r4},{r5}" (x), "m,r" (*y));
+}
diff --git a/gcc/testsuite/gcc.target/s390/asm-hard-reg-3.c b/gcc/testsuite/gcc.target/s390/asm-hard-reg-3.c
new file mode 100644
index 00000000000..0edcdd3cfde
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/asm-hard-reg-3.c
@@ -0,0 +1,24 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+void
+test (void)
+{
+ int x, y;
+ register int r4 asm ("r4") = 0;
+
+ asm ("" :: "{}" (42)); /* { dg-error "invalid input constraint: \{\}" } */
+ asm ("" :: "{r4" (42)); /* { dg-error "invalid input constraint: \{r4" } */
+ asm ("" :: "{r17}" (42)); /* { dg-error "invalid input constraint: \{r17\}" } */
+
+ asm ("" :: "r" (r4), "{r4}" (42)); /* { dg-error "multiple inputs to hard register: %r4" } */
+ asm ("" :: "{r4}" (42), "r" (r4)); /* { dg-error "multiple inputs to hard register: %r4" } */
+ asm ("" :: "{r4}" (42), "{r4}" (42)); /* { dg-error "multiple inputs to hard register: %r4" } */
+ asm ("" :: "{r2},{r4}" (42), "{r4},{r3}" (42));
+ asm ("" :: "{r2},{r4}" (42), "{r3},{r4}" (42)); /* { dg-error "multiple inputs to hard register: %r4" } */
+ asm ("" :: "{r4},{r2}" (42), "{r4},{r3}" (42)); /* { dg-error "multiple inputs to hard register: %r4" } */
+ asm ("" :: "{r3}{r4}" (42), "{r4}" (42)); /* { dg-error "multiple inputs to hard register: %r4" } */
+ asm ("" : "+{r4}" (x), "={r4}" (y)); /* { dg-error "multiple outputs to hard register: %r4" } */
+ asm ("" : "={r4}" (y) : "{r4}" (42), "0" (42)); /* { dg-error "multiple inputs to hard register: %r4" } */
+ asm ("" : "+{r4}" (x) : "{r4}" (y)); /* { dg-error "multiple inputs to hard register: %r4" } */
+}
diff --git a/gcc/testsuite/lib/scanasm.exp b/gcc/testsuite/lib/scanasm.exp
index 6cf9997240d..d09372096a5 100644
--- a/gcc/testsuite/lib/scanasm.exp
+++ b/gcc/testsuite/lib/scanasm.exp
@@ -896,6 +896,10 @@ proc configure_check-function-bodies { config } {
set up_config(fluff) {^\s*(?://)}
} elseif { [istarget *-*-darwin*] } {
set up_config(fluff) {^\s*(?:\.|//|@)|^L[0-9ABCESV]}
+ } elseif { [istarget s390*-*-*] } {
+ # Additionally to the defaults skip lines beginning with a # resulting
+ # from inline asm.
+ set up_config(fluff) {^\s*(?:\.|//|@|$|#)}
} else {
# Skip lines beginning with labels ('.L[...]:') or other directives
# ('.align', '.cfi_startproc', '.quad [...]', '.text', etc.), '//' or
--
2.45.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Hard register asm constraint
2024-05-24 9:13 [PATCH] Hard register asm constraint Stefan Schulze Frielinghaus
@ 2024-06-10 5:19 ` Stefan Schulze Frielinghaus
2024-06-24 5:50 ` Stefan Schulze Frielinghaus
2024-06-27 7:45 ` Georg-Johann Lay
1 sibling, 1 reply; 14+ messages in thread
From: Stefan Schulze Frielinghaus @ 2024-06-10 5:19 UTC (permalink / raw)
To: gcc-patches
Ping.
On Fri, May 24, 2024 at 11:13:12AM +0200, Stefan Schulze Frielinghaus wrote:
> This implements hard register constraints for inline asm. A hard register
> constraint is of the form {regname} where regname is any valid register. This
> basically renders register asm superfluous. For example, the snippet
>
> int test (int x, int y)
> {
> register int r4 asm ("r4") = x;
> register int r5 asm ("r5") = y;
> unsigned int copy = y;
> asm ("foo %0,%1,%2" : "+d" (r4) : "d" (r5), "d" (copy));
> return r4;
> }
>
> could be rewritten into
>
> int test (int x, int y)
> {
> asm ("foo %0,%1,%2" : "+{r4}" (x) : "{r5}" (y), "d" (y));
> return x;
> }
>
> As a side-effect this also solves the problem of call-clobbered registers.
> That being said, I was wondering whether we could utilize this feature in order
> to get rid of local register asm automatically? For example, converting
>
> // Result will be in r2 on s390
> extern int bar (void);
>
> void test (void)
> {
> register int x asm ("r2") = 42;
> bar ();
> asm ("foo %0\n" :: "r" (x));
> }
>
> into
>
> void test (void)
> {
> int x = 42;
> bar ();
> asm ("foo %0\n" :: "{r2}" (x));
> }
>
> in order to get rid of the limitation of call-clobbered registers which may
> lead to subtle bugs---especially if you think of non-obvious calls e.g.
> introduced by sanitizer/tracer/whatever. Since such a transformation has the
> potential to break existing code do you see any edge cases where this might be
> problematic or even show stoppers? Currently, even
>
> int test (void)
> {
> register int x asm ("r2") = 42;
> register int y asm ("r2") = 24;
> asm ("foo %0,%1\n" :: "r" (x), "r" (y));
> }
>
> is allowed which seems error prone to me. Thus, if 100% backwards
> compatibility would be required, then automatically converting every register
> asm to the new mechanism isn't viable. Still quite a lot could be transformed.
> Any thoughts?
>
> Currently I allow multiple alternatives as demonstrated by
> gcc/testsuite/gcc.target/s390/asm-hard-reg-2.c. However, since a hard register
> constraint is pretty specific I could also think of erroring out in case of
> alternatives. Are there any real use cases out there for multiple
> alternatives where one would like to use hard register constraints?
>
> With the current implementation we have a "user visible change" in the sense
> that for
>
> void test (void)
> {
> register int x asm ("r2") = 42;
> register int y asm ("r2") = 24;
> asm ("foo %0,%1\n" : "=r" (x), "=r" (y));
> }
>
> we do not get the error
>
> "invalid hard register usage between output operands"
>
> anymore but rather
>
> "multiple outputs to hard register: %r2"
>
> This is due to the error handling in gimplify_asm_expr (). Speaking of errors,
> I also error out earlier as before which means that e.g. in pr87600-2.c only
> the first error is reported and processing is stopped afterwards which means
> the subsequent tests fail.
>
> I've been skimming through all targets and it looks to me as if none is using
> curly brackets for their constraints. Of course, I may have missed something.
>
> Cheers,
> Stefan
>
> PS: Current state for Clang: https://reviews.llvm.org/D105142
>
> ---
> gcc/cfgexpand.cc | 42 -------
> gcc/genpreds.cc | 4 +-
> gcc/gimplify.cc | 115 +++++++++++++++++-
> gcc/lra-constraints.cc | 17 +++
> gcc/recog.cc | 14 ++-
> gcc/stmt.cc | 102 +++++++++++++++-
> gcc/stmt.h | 10 +-
> .../gcc.target/s390/asm-hard-reg-1.c | 103 ++++++++++++++++
> .../gcc.target/s390/asm-hard-reg-2.c | 29 +++++
> .../gcc.target/s390/asm-hard-reg-3.c | 24 ++++
> gcc/testsuite/lib/scanasm.exp | 4 +
> 11 files changed, 407 insertions(+), 57 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/s390/asm-hard-reg-1.c
> create mode 100644 gcc/testsuite/gcc.target/s390/asm-hard-reg-2.c
> create mode 100644 gcc/testsuite/gcc.target/s390/asm-hard-reg-3.c
>
> diff --git a/gcc/cfgexpand.cc b/gcc/cfgexpand.cc
> index 557cb28733b..47f71a2e803 100644
> --- a/gcc/cfgexpand.cc
> +++ b/gcc/cfgexpand.cc
> @@ -2955,44 +2955,6 @@ expand_asm_loc (tree string, int vol, location_t locus)
> emit_insn (body);
> }
>
> -/* Return the number of times character C occurs in string S. */
> -static int
> -n_occurrences (int c, const char *s)
> -{
> - int n = 0;
> - while (*s)
> - n += (*s++ == c);
> - return n;
> -}
> -
> -/* A subroutine of expand_asm_operands. Check that all operands have
> - the same number of alternatives. Return true if so. */
> -
> -static bool
> -check_operand_nalternatives (const vec<const char *> &constraints)
> -{
> - unsigned len = constraints.length();
> - if (len > 0)
> - {
> - int nalternatives = n_occurrences (',', constraints[0]);
> -
> - if (nalternatives + 1 > MAX_RECOG_ALTERNATIVES)
> - {
> - error ("too many alternatives in %<asm%>");
> - return false;
> - }
> -
> - for (unsigned i = 1; i < len; ++i)
> - if (n_occurrences (',', constraints[i]) != nalternatives)
> - {
> - error ("operand constraints for %<asm%> differ "
> - "in number of alternatives");
> - return false;
> - }
> - }
> - return true;
> -}
> -
> /* Check for overlap between registers marked in CLOBBERED_REGS and
> anything inappropriate in T. Emit error and return the register
> variable definition for error, NULL_TREE for ok. */
> @@ -3158,10 +3120,6 @@ expand_asm_stmt (gasm *stmt)
> = TREE_STRING_POINTER (TREE_VALUE (TREE_PURPOSE (t)));
> }
>
> - /* ??? Diagnose during gimplification? */
> - if (! check_operand_nalternatives (constraints))
> - return;
> -
> /* Count the number of meaningful clobbered registers, ignoring what
> we would ignore later. */
> auto_vec<rtx> clobber_rvec;
> diff --git a/gcc/genpreds.cc b/gcc/genpreds.cc
> index 55d149e8a40..f0d59cb0846 100644
> --- a/gcc/genpreds.cc
> +++ b/gcc/genpreds.cc
> @@ -1148,7 +1148,7 @@ write_insn_constraint_len (void)
> unsigned int i;
>
> puts ("static inline size_t\n"
> - "insn_constraint_len (char fc, const char *str ATTRIBUTE_UNUSED)\n"
> + "insn_constraint_len (char fc, const char *str)\n"
> "{\n"
> " switch (fc)\n"
> " {");
> @@ -1181,6 +1181,8 @@ write_insn_constraint_len (void)
>
> puts (" default: break;\n"
> " }\n"
> + " if (str[0] == '{')\n"
> + " return ((const char *)rawmemchr (str + 1, '}') - str) + 1;\n"
> " return 1;\n"
> "}\n");
> }
> diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc
> index b0ed58ed0f9..b4b16e75023 100644
> --- a/gcc/gimplify.cc
> +++ b/gcc/gimplify.cc
> @@ -70,6 +70,9 @@ along with GCC; see the file COPYING3. If not see
> #include "omp-offload.h"
> #include "context.h"
> #include "tree-nested.h"
> +#include "insn-config.h"
> +#include "recog.h"
> +#include "output.h"
>
> /* Identifier for a basic condition, mapping it to other basic conditions of
> its Boolean expression. Basic conditions given the same uid (in the same
> @@ -6952,6 +6955,42 @@ gimplify_addr_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
> return ret;
> }
>
> +/* Return the number of times character C occurs in string S. */
> +
> +static int
> +num_occurrences (int c, const char *s)
> +{
> + int n = 0;
> + while (*s)
> + n += (*s++ == c);
> + return n;
> +}
> +
> +/* A subroutine of gimplify_asm_expr. Check that all operands have
> + the same number of alternatives. Return -1 if this is violated. Otherwise
> + return the number of alternatives. */
> +
> +static int
> +num_alternatives (const_tree link)
> +{
> + if (link == nullptr)
> + return 0;
> +
> + const char *constraint = TREE_STRING_POINTER (TREE_VALUE (TREE_PURPOSE (link)));
> + int num = num_occurrences (',', constraint);
> +
> + if (num + 1 > MAX_RECOG_ALTERNATIVES)
> + return -1;
> +
> + for (link = TREE_CHAIN (link); link; link = TREE_CHAIN (link))
> + {
> + constraint = TREE_STRING_POINTER (TREE_VALUE (TREE_PURPOSE (link)));
> + if (num_occurrences (',', constraint) != num)
> + return -1;
> + }
> + return num + 1;
> +}
> +
> /* Gimplify the operands of an ASM_EXPR. Input operands should be a gimple
> value; output operands should be a gimple lvalue. */
>
> @@ -6982,6 +7021,35 @@ gimplify_asm_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
> clobbers = NULL;
> labels = NULL;
>
> + int num_alternatives_out = num_alternatives (ASM_OUTPUTS (expr));
> + int num_alternatives_in = num_alternatives (ASM_INPUTS (expr));
> + if (num_alternatives_out == -1 || num_alternatives_in == -1
> + || (num_alternatives_out > 0 && num_alternatives_in > 0
> + && num_alternatives_out != num_alternatives_in))
> + {
> + error ("operand constraints for %<asm%> differ "
> + "in number of alternatives");
> + return GS_ERROR;
> + }
> + int num_alternatives = MAX (num_alternatives_out, num_alternatives_in);
> +
> + /* Regarding hard register constraints ensure that each hard register is used
> + at most once over all inputs/outputs and each alternative. Keep track in
> + hardregs[0] which hard register is used via an asm register over all
> + inputs/outputs. hardregs[i] for i >= 2 describes which hard registers are
> + used for alternative i-2 over all inputs/outputs. hardregs[1] is a
> + reduction of all alternatives, i.e., hardregs[1] |= hardregs[i] for i >= 2
> + and describes whether a hard register is used in any alternative. This is
> + just a shortcut instead of recomputing the union over all alternatives;
> + possibly multiple times. */
> + auto_vec<HARD_REG_SET> hardregs (num_alternatives + 2);
> + for (int i = 0; i < num_alternatives + 2; ++i)
> + {
> + HARD_REG_SET hregset;
> + CLEAR_HARD_REG_SET (hregset);
> + hardregs.quick_push (hregset);
> + }
> +
> ret = GS_ALL_DONE;
> link_next = NULL_TREE;
> for (i = 0, link = ASM_OUTPUTS (expr); link; ++i, link = link_next)
> @@ -6998,8 +7066,8 @@ gimplify_asm_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
> if (constraint_len == 0)
> continue;
>
> - ok = parse_output_constraint (&constraint, i, 0, 0,
> - &allows_mem, &allows_reg, &is_inout);
> + ok = parse_output_constraint (&constraint, i, 0, 0, &allows_mem,
> + &allows_reg, &is_inout, &hardregs);
> if (!ok)
> {
> ret = GS_ERROR;
> @@ -7062,6 +7130,24 @@ gimplify_asm_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
> TREE_VALUE (link) = tem;
> tret = GS_OK;
> }
> + if (VAR_P (op) && DECL_HARD_REGISTER (op))
> + {
> + tree id = DECL_ASSEMBLER_NAME (op);
> + const char *asmspec = IDENTIFIER_POINTER (id) + 1;
> + int hardreg = decode_reg_name (asmspec);
> + if (hardreg >= 0)
> + {
> + if (TEST_HARD_REG_BIT (hardregs[0], hardreg)
> + || TEST_HARD_REG_BIT (hardregs[1], hardreg))
> + {
> + error ("multiple outputs to hard register: %s",
> + reg_names[hardreg]);
> + return GS_ERROR;
> + }
> + else
> + SET_HARD_REG_BIT (hardregs[0], hardreg);
> + }
> + }
> }
>
> vec_safe_push (outputs, link);
> @@ -7161,13 +7247,16 @@ gimplify_asm_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
> }
> }
>
> + for (unsigned int i = 0; i < hardregs.length (); ++i)
> + CLEAR_HARD_REG_SET (hardregs[i]);
> +
> link_next = NULL_TREE;
> for (link = ASM_INPUTS (expr); link; ++i, link = link_next)
> {
> link_next = TREE_CHAIN (link);
> constraint = TREE_STRING_POINTER (TREE_VALUE (TREE_PURPOSE (link)));
> - parse_input_constraint (&constraint, 0, 0, noutputs, 0,
> - oconstraints, &allows_mem, &allows_reg);
> + parse_input_constraint (&constraint, 0, 0, noutputs, 0, oconstraints,
> + &allows_mem, &allows_reg, &hardregs);
>
> /* If we can't make copies, we can only accept memory. */
> tree intype = TREE_TYPE (TREE_VALUE (link));
> @@ -7241,6 +7330,24 @@ gimplify_asm_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
> is_gimple_asm_val, fb_rvalue);
> if (tret == GS_ERROR)
> ret = tret;
> + tree inputv = TREE_VALUE (link);
> + if (VAR_P (inputv) && DECL_HARD_REGISTER (inputv))
> + {
> + tree id = DECL_ASSEMBLER_NAME (inputv);
> + const char *asmspec = IDENTIFIER_POINTER (id) + 1;
> + int hardreg = decode_reg_name (asmspec);
> + if (hardreg >= 0)
> + {
> + if (TEST_HARD_REG_BIT (hardregs[1], hardreg))
> + {
> + error ("multiple inputs to hard register: %s",
> + reg_names[hardreg]);
> + return GS_ERROR;
> + }
> + else
> + SET_HARD_REG_BIT (hardregs[0], hardreg);
> + }
> + }
> }
>
> TREE_CHAIN (link) = NULL_TREE;
> diff --git a/gcc/lra-constraints.cc b/gcc/lra-constraints.cc
> index e945a4da451..d81753eefaa 100644
> --- a/gcc/lra-constraints.cc
> +++ b/gcc/lra-constraints.cc
> @@ -114,6 +114,7 @@
> #include "target.h"
> #include "rtl.h"
> #include "tree.h"
> +#include "stmt.h"
> #include "predict.h"
> #include "df.h"
> #include "memmodel.h"
> @@ -2165,6 +2166,7 @@ process_alt_operands (int only_alternative)
> bool costly_p;
> enum reg_class cl;
> const HARD_REG_SET *cl_filter;
> + HARD_REG_SET hregset;
>
> /* Calculate some data common for all alternatives to speed up the
> function. */
> @@ -2536,6 +2538,21 @@ process_alt_operands (int only_alternative)
> cl_filter = nullptr;
> goto reg;
>
> + case '{':
> + {
> + /* Currently this form of constraint is only allowed in
> + asm statements which are verified during gimplify,
> + i.e., regno >= 0 holds for those. genoutput fails on
> + it. For future proofness assert it. */
> + int regno = parse_constraint_regname (p);
> + gcc_assert (regno >= 0);
> + cl = REGNO_REG_CLASS (regno);
> + CLEAR_HARD_REG_SET (hregset);
> + SET_HARD_REG_BIT (hregset, regno);
> + cl_filter = &hregset;
> + goto reg;
> + }
> +
> default:
> cn = lookup_constraint (p);
> switch (get_constraint_type (cn))
> diff --git a/gcc/recog.cc b/gcc/recog.cc
> index a6799e3f5e6..8a474cfb8a7 100644
> --- a/gcc/recog.cc
> +++ b/gcc/recog.cc
> @@ -25,6 +25,7 @@ along with GCC; see the file COPYING3. If not see
> #include "target.h"
> #include "rtl.h"
> #include "tree.h"
> +#include "stmt.h"
> #include "cfghooks.h"
> #include "df.h"
> #include "memmodel.h"
> @@ -2296,10 +2297,11 @@ asm_operand_ok (rtx op, const char *constraint, const char **constraints)
> switch (get_constraint_type (cn))
> {
> case CT_REGISTER:
> - if (!result
> - && reg_class_for_constraint (cn) != NO_REGS
> - && GET_MODE (op) != BLKmode
> - && register_operand (op, VOIDmode))
> + if ((!result
> + && reg_class_for_constraint (cn) != NO_REGS
> + && GET_MODE (op) != BLKmode
> + && register_operand (op, VOIDmode))
> + || constraint[0] == '{')
> result = 1;
> break;
>
> @@ -3231,6 +3233,10 @@ constrain_operands (int strict, alternative_mask alternatives)
> win = true;
> break;
>
> + case '{':
> + win = true;
> + break;
> +
> default:
> {
> enum constraint_num cn = lookup_constraint (p);
> diff --git a/gcc/stmt.cc b/gcc/stmt.cc
> index ae1527f0a19..1f20b09f90e 100644
> --- a/gcc/stmt.cc
> +++ b/gcc/stmt.cc
> @@ -39,6 +39,7 @@ along with GCC; see the file COPYING3. If not see
> #include "emit-rtl.h"
> #include "pretty-print.h"
> #include "diagnostic-core.h"
> +#include "output.h"
>
> #include "fold-const.h"
> #include "varasm.h"
> @@ -174,6 +175,32 @@ expand_label (tree label)
> maybe_set_first_label_num (label_r);
> }
> \f
> +/* Parse a hard register constraint and return its number or -1 in case of an
> + error. BEGIN should point to a string of the form "{regname}". For the
> + sake of simplicity assume that a register name is not longer than 31
> + characters, if not error out. */
> +
> +int
> +parse_constraint_regname (const char *begin)
> +{
> + if (*begin != '{')
> + return -1;
> + ++begin;
> + const char *end = begin;
> + while (*end != '}' && *end != '\0')
> + ++end;
> + if (*end != '}' || end == begin)
> + return -1;
> + ptrdiff_t len = end - begin;
> + if (len >= 31)
> + return -1;
> + char regname[32];
> + memcpy (regname, begin, len);
> + regname[len] = '\0';
> + int regno = decode_reg_name (regname);
> + return regno;
> +}
> +
> /* Parse the output constraint pointed to by *CONSTRAINT_P. It is the
> OPERAND_NUMth output operand, indexed from zero. There are NINPUTS
> inputs and NOUTPUTS outputs to this extended-asm. Upon return,
> @@ -190,7 +217,8 @@ expand_label (tree label)
> bool
> parse_output_constraint (const char **constraint_p, int operand_num,
> int ninputs, int noutputs, bool *allows_mem,
> - bool *allows_reg, bool *is_inout)
> + bool *allows_reg, bool *is_inout,
> + vec<HARD_REG_SET> *hardregs)
> {
> const char *constraint = *constraint_p;
> const char *p;
> @@ -244,6 +272,8 @@ parse_output_constraint (const char **constraint_p, int operand_num,
> constraint = *constraint_p;
> }
>
> + unsigned int alternative = 2;
> +
> /* Loop through the constraint string. */
> for (p = constraint + 1; *p; )
> {
> @@ -268,7 +298,11 @@ parse_output_constraint (const char **constraint_p, int operand_num,
> case 'E': case 'F': case 'G': case 'H':
> case 's': case 'i': case 'n':
> case 'I': case 'J': case 'K': case 'L': case 'M':
> - case 'N': case 'O': case 'P': case ',':
> + case 'N': case 'O': case 'P':
> + break;
> +
> + case ',':
> + ++alternative;
> break;
>
> case '0': case '1': case '2': case '3': case '4':
> @@ -289,6 +323,33 @@ parse_output_constraint (const char **constraint_p, int operand_num,
> *allows_mem = true;
> break;
>
> + case '{':
> + {
> + int regno = parse_constraint_regname (p);
> + if (regno < 0)
> + {
> + error ("invalid output constraint: %s", p);
> + return false;
> + }
> + if (hardregs)
> + {
> + if (TEST_HARD_REG_BIT ((*hardregs)[0], regno)
> + || TEST_HARD_REG_BIT ((*hardregs)[alternative], regno))
> + {
> + error ("multiple outputs to hard register: %s",
> + reg_names[regno]);
> + return false;
> + }
> + else
> + {
> + SET_HARD_REG_BIT ((*hardregs)[1], regno);
> + SET_HARD_REG_BIT ((*hardregs)[alternative], regno);
> + }
> + }
> + *allows_reg = true;
> + break;
> + }
> +
> default:
> if (!ISALPHA (*p))
> break;
> @@ -317,7 +378,8 @@ bool
> parse_input_constraint (const char **constraint_p, int input_num,
> int ninputs, int noutputs, int ninout,
> const char * const * constraints,
> - bool *allows_mem, bool *allows_reg)
> + bool *allows_mem, bool *allows_reg,
> + vec<HARD_REG_SET> *hardregs)
> {
> const char *constraint = *constraint_p;
> const char *orig_constraint = constraint;
> @@ -332,6 +394,8 @@ parse_input_constraint (const char **constraint_p, int input_num,
>
> /* Make sure constraint has neither `=', `+', nor '&'. */
>
> + unsigned int alternative = 2;
> +
> for (j = 0; j < c_len; j += CONSTRAINT_LEN (constraint[j], constraint+j))
> switch (constraint[j])
> {
> @@ -358,7 +422,11 @@ parse_input_constraint (const char **constraint_p, int input_num,
> case 'E': case 'F': case 'G': case 'H':
> case 's': case 'i': case 'n':
> case 'I': case 'J': case 'K': case 'L': case 'M':
> - case 'N': case 'O': case 'P': case ',':
> + case 'N': case 'O': case 'P':
> + break;
> +
> + case ',':
> + ++alternative;
> break;
>
> /* Whether or not a numeric constraint allows a register is
> @@ -408,6 +476,32 @@ parse_input_constraint (const char **constraint_p, int input_num,
> *allows_mem = true;
> break;
>
> + case '{':
> + {
> + int regno = parse_constraint_regname (constraint + j);
> + if (regno < 0)
> + {
> + error ("invalid input constraint: %s", constraint + j);
> + return false;
> + }
> + if (hardregs)
> + {
> + if (TEST_HARD_REG_BIT ((*hardregs)[0], regno)
> + || TEST_HARD_REG_BIT ((*hardregs)[alternative], regno))
> + {
> + error ("multiple inputs to hard register: %s",
> + reg_names[regno]);
> + }
> + else
> + {
> + SET_HARD_REG_BIT ((*hardregs)[1], regno);
> + SET_HARD_REG_BIT ((*hardregs)[alternative], regno);
> + }
> + }
> + *allows_reg = true;
> + break;
> + }
> +
> default:
> if (! ISALPHA (constraint[j]))
> {
> diff --git a/gcc/stmt.h b/gcc/stmt.h
> index a2caae7121b..05889ff3798 100644
> --- a/gcc/stmt.h
> +++ b/gcc/stmt.h
> @@ -20,11 +20,17 @@ along with GCC; see the file COPYING3. If not see
> #ifndef GCC_STMT_H
> #define GCC_STMT_H
>
> +#include "target.h"
> +#include "hard-reg-set.h"
> +
> extern void expand_label (tree);
> extern bool parse_output_constraint (const char **, int, int, int,
> - bool *, bool *, bool *);
> + bool *, bool *, bool *,
> + vec<HARD_REG_SET> * = nullptr);
> extern bool parse_input_constraint (const char **, int, int, int, int,
> - const char * const *, bool *, bool *);
> + const char * const *, bool *, bool *,
> + vec<HARD_REG_SET> * = nullptr);
> +extern int parse_constraint_regname (const char *);
> extern tree resolve_asm_operand_names (tree, tree, tree, tree);
> #ifdef HARD_CONST
> /* Silly ifdef to avoid having all includers depend on hard-reg-set.h. */
> diff --git a/gcc/testsuite/gcc.target/s390/asm-hard-reg-1.c b/gcc/testsuite/gcc.target/s390/asm-hard-reg-1.c
> new file mode 100644
> index 00000000000..53895d98663
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/s390/asm-hard-reg-1.c
> @@ -0,0 +1,103 @@
> +/* { dg-do compile { target { lp64 } } } */
> +/* { dg-options "-O2 -march=z13" } */
> +/* { dg-final { check-function-bodies "**" "" "" } } */
> +
> +/*
> +** test_in_1:
> +** foo %r2
> +** br %r14
> +*/
> +
> +int
> +test_in_1 (int x)
> +{
> + asm ("foo %0" :: "{r2}" (x));
> + return x;
> +}
> +
> +/*
> +** test_in_2:
> +** lgr (%r[0-9]+),%r2
> +** lhi %r2,42
> +** foo %r2
> +** lgr %r2,\1
> +** br %r14
> +*/
> +
> +int
> +test_in_2 (int x)
> +{
> + asm ("foo %0" :: "{r2}" (42));
> + return x;
> +}
> +
> +/*
> +** test_in_3:
> +** stmg %r12,%r15,96\(%r15\)
> +** lay %r15,-160\(%r15\)
> +** lgr (%r[0-9]+),%r2
> +** ahi %r2,1
> +** lgfr %r2,%r2
> +** brasl %r14,foo@PLT
> +** lr %r3,%r2
> +** lr %r2,\1
> +** foo %r3,%r2
> +** lgr %r2,\1
> +** lmg %r12,%r15,256\(%r15\)
> +** br %r14
> +*/
> +
> +extern int foo (int);
> +
> +int
> +test_in_3 (int x)
> +{
> + asm ("foo %0,%1\n" :: "{r3}" (foo (x + 1)), "{r2}" (x));
> + return x;
> +}
> +
> +/*
> +** test_out_1:
> +** foo %r3
> +** lgfr %r2,%r3
> +** br %r14
> +*/
> +
> +int
> +test_out_1 (void)
> +{
> + int x;
> + asm ("foo %0" : "={r3}" (x));
> + return x;
> +}
> +
> +/*
> +** test_out_2:
> +** lgr (%r[0-9]+),%r2
> +** foo %r2
> +** ark (%r[0-9]+),\1,%r2
> +** lgfr %r2,\2
> +** br %r14
> +*/
> +
> +int
> +test_out_2 (int x)
> +{
> + int y;
> + asm ("foo %0" : "={r2}" (y));
> + return x + y;
> +}
> +
> +/*
> +** test_inout_1:
> +** foo %r2
> +** lgfr %r2,%r2
> +** br %r14
> +*/
> +
> +int
> +test_inout_1 (int x)
> +{
> + asm ("foo %0" : "+{r2}" (x));
> + return x;
> +}
> diff --git a/gcc/testsuite/gcc.target/s390/asm-hard-reg-2.c b/gcc/testsuite/gcc.target/s390/asm-hard-reg-2.c
> new file mode 100644
> index 00000000000..9f3c221b937
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/s390/asm-hard-reg-2.c
> @@ -0,0 +1,29 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +/* { dg-final { check-function-bodies "**" "" "" } } */
> +
> +/*
> +** test_1:
> +** lr %r5,%r2
> +** foo %r5,%r3
> +** br %r14
> +*/
> +
> +void
> +test_1 (int x, int *y)
> +{
> + asm ("foo %0,%1" :: "m{r4},{r5}" (x), "m,r" (y));
> +}
> +
> +/*
> +** test_2:
> +** lr %r4,%r2
> +** foo %r4,0\(%r3\)
> +** br %r14
> +*/
> +
> +void
> +test_2 (int x, int *y)
> +{
> + asm ("foo %0,%1" :: "m{r4},{r5}" (x), "m,r" (*y));
> +}
> diff --git a/gcc/testsuite/gcc.target/s390/asm-hard-reg-3.c b/gcc/testsuite/gcc.target/s390/asm-hard-reg-3.c
> new file mode 100644
> index 00000000000..0edcdd3cfde
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/s390/asm-hard-reg-3.c
> @@ -0,0 +1,24 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +void
> +test (void)
> +{
> + int x, y;
> + register int r4 asm ("r4") = 0;
> +
> + asm ("" :: "{}" (42)); /* { dg-error "invalid input constraint: \{\}" } */
> + asm ("" :: "{r4" (42)); /* { dg-error "invalid input constraint: \{r4" } */
> + asm ("" :: "{r17}" (42)); /* { dg-error "invalid input constraint: \{r17\}" } */
> +
> + asm ("" :: "r" (r4), "{r4}" (42)); /* { dg-error "multiple inputs to hard register: %r4" } */
> + asm ("" :: "{r4}" (42), "r" (r4)); /* { dg-error "multiple inputs to hard register: %r4" } */
> + asm ("" :: "{r4}" (42), "{r4}" (42)); /* { dg-error "multiple inputs to hard register: %r4" } */
> + asm ("" :: "{r2},{r4}" (42), "{r4},{r3}" (42));
> + asm ("" :: "{r2},{r4}" (42), "{r3},{r4}" (42)); /* { dg-error "multiple inputs to hard register: %r4" } */
> + asm ("" :: "{r4},{r2}" (42), "{r4},{r3}" (42)); /* { dg-error "multiple inputs to hard register: %r4" } */
> + asm ("" :: "{r3}{r4}" (42), "{r4}" (42)); /* { dg-error "multiple inputs to hard register: %r4" } */
> + asm ("" : "+{r4}" (x), "={r4}" (y)); /* { dg-error "multiple outputs to hard register: %r4" } */
> + asm ("" : "={r4}" (y) : "{r4}" (42), "0" (42)); /* { dg-error "multiple inputs to hard register: %r4" } */
> + asm ("" : "+{r4}" (x) : "{r4}" (y)); /* { dg-error "multiple inputs to hard register: %r4" } */
> +}
> diff --git a/gcc/testsuite/lib/scanasm.exp b/gcc/testsuite/lib/scanasm.exp
> index 6cf9997240d..d09372096a5 100644
> --- a/gcc/testsuite/lib/scanasm.exp
> +++ b/gcc/testsuite/lib/scanasm.exp
> @@ -896,6 +896,10 @@ proc configure_check-function-bodies { config } {
> set up_config(fluff) {^\s*(?://)}
> } elseif { [istarget *-*-darwin*] } {
> set up_config(fluff) {^\s*(?:\.|//|@)|^L[0-9ABCESV]}
> + } elseif { [istarget s390*-*-*] } {
> + # Additionally to the defaults skip lines beginning with a # resulting
> + # from inline asm.
> + set up_config(fluff) {^\s*(?:\.|//|@|$|#)}
> } else {
> # Skip lines beginning with labels ('.L[...]:') or other directives
> # ('.align', '.cfi_startproc', '.quad [...]', '.text', etc.), '//' or
> --
> 2.45.1
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Hard register asm constraint
2024-06-10 5:19 ` Stefan Schulze Frielinghaus
@ 2024-06-24 5:50 ` Stefan Schulze Frielinghaus
2024-06-25 14:03 ` Paul Koning
0 siblings, 1 reply; 14+ messages in thread
From: Stefan Schulze Frielinghaus @ 2024-06-24 5:50 UTC (permalink / raw)
To: gcc-patches
Ping.
On Mon, Jun 10, 2024 at 07:19:19AM +0200, Stefan Schulze Frielinghaus wrote:
> Ping.
>
> On Fri, May 24, 2024 at 11:13:12AM +0200, Stefan Schulze Frielinghaus wrote:
> > This implements hard register constraints for inline asm. A hard register
> > constraint is of the form {regname} where regname is any valid register. This
> > basically renders register asm superfluous. For example, the snippet
> >
> > int test (int x, int y)
> > {
> > register int r4 asm ("r4") = x;
> > register int r5 asm ("r5") = y;
> > unsigned int copy = y;
> > asm ("foo %0,%1,%2" : "+d" (r4) : "d" (r5), "d" (copy));
> > return r4;
> > }
> >
> > could be rewritten into
> >
> > int test (int x, int y)
> > {
> > asm ("foo %0,%1,%2" : "+{r4}" (x) : "{r5}" (y), "d" (y));
> > return x;
> > }
> >
> > As a side-effect this also solves the problem of call-clobbered registers.
> > That being said, I was wondering whether we could utilize this feature in order
> > to get rid of local register asm automatically? For example, converting
> >
> > // Result will be in r2 on s390
> > extern int bar (void);
> >
> > void test (void)
> > {
> > register int x asm ("r2") = 42;
> > bar ();
> > asm ("foo %0\n" :: "r" (x));
> > }
> >
> > into
> >
> > void test (void)
> > {
> > int x = 42;
> > bar ();
> > asm ("foo %0\n" :: "{r2}" (x));
> > }
> >
> > in order to get rid of the limitation of call-clobbered registers which may
> > lead to subtle bugs---especially if you think of non-obvious calls e.g.
> > introduced by sanitizer/tracer/whatever. Since such a transformation has the
> > potential to break existing code do you see any edge cases where this might be
> > problematic or even show stoppers? Currently, even
> >
> > int test (void)
> > {
> > register int x asm ("r2") = 42;
> > register int y asm ("r2") = 24;
> > asm ("foo %0,%1\n" :: "r" (x), "r" (y));
> > }
> >
> > is allowed which seems error prone to me. Thus, if 100% backwards
> > compatibility would be required, then automatically converting every register
> > asm to the new mechanism isn't viable. Still quite a lot could be transformed.
> > Any thoughts?
> >
> > Currently I allow multiple alternatives as demonstrated by
> > gcc/testsuite/gcc.target/s390/asm-hard-reg-2.c. However, since a hard register
> > constraint is pretty specific I could also think of erroring out in case of
> > alternatives. Are there any real use cases out there for multiple
> > alternatives where one would like to use hard register constraints?
> >
> > With the current implementation we have a "user visible change" in the sense
> > that for
> >
> > void test (void)
> > {
> > register int x asm ("r2") = 42;
> > register int y asm ("r2") = 24;
> > asm ("foo %0,%1\n" : "=r" (x), "=r" (y));
> > }
> >
> > we do not get the error
> >
> > "invalid hard register usage between output operands"
> >
> > anymore but rather
> >
> > "multiple outputs to hard register: %r2"
> >
> > This is due to the error handling in gimplify_asm_expr (). Speaking of errors,
> > I also error out earlier as before which means that e.g. in pr87600-2.c only
> > the first error is reported and processing is stopped afterwards which means
> > the subsequent tests fail.
> >
> > I've been skimming through all targets and it looks to me as if none is using
> > curly brackets for their constraints. Of course, I may have missed something.
> >
> > Cheers,
> > Stefan
> >
> > PS: Current state for Clang: https://reviews.llvm.org/D105142
> >
> > ---
> > gcc/cfgexpand.cc | 42 -------
> > gcc/genpreds.cc | 4 +-
> > gcc/gimplify.cc | 115 +++++++++++++++++-
> > gcc/lra-constraints.cc | 17 +++
> > gcc/recog.cc | 14 ++-
> > gcc/stmt.cc | 102 +++++++++++++++-
> > gcc/stmt.h | 10 +-
> > .../gcc.target/s390/asm-hard-reg-1.c | 103 ++++++++++++++++
> > .../gcc.target/s390/asm-hard-reg-2.c | 29 +++++
> > .../gcc.target/s390/asm-hard-reg-3.c | 24 ++++
> > gcc/testsuite/lib/scanasm.exp | 4 +
> > 11 files changed, 407 insertions(+), 57 deletions(-)
> > create mode 100644 gcc/testsuite/gcc.target/s390/asm-hard-reg-1.c
> > create mode 100644 gcc/testsuite/gcc.target/s390/asm-hard-reg-2.c
> > create mode 100644 gcc/testsuite/gcc.target/s390/asm-hard-reg-3.c
> >
> > diff --git a/gcc/cfgexpand.cc b/gcc/cfgexpand.cc
> > index 557cb28733b..47f71a2e803 100644
> > --- a/gcc/cfgexpand.cc
> > +++ b/gcc/cfgexpand.cc
> > @@ -2955,44 +2955,6 @@ expand_asm_loc (tree string, int vol, location_t locus)
> > emit_insn (body);
> > }
> >
> > -/* Return the number of times character C occurs in string S. */
> > -static int
> > -n_occurrences (int c, const char *s)
> > -{
> > - int n = 0;
> > - while (*s)
> > - n += (*s++ == c);
> > - return n;
> > -}
> > -
> > -/* A subroutine of expand_asm_operands. Check that all operands have
> > - the same number of alternatives. Return true if so. */
> > -
> > -static bool
> > -check_operand_nalternatives (const vec<const char *> &constraints)
> > -{
> > - unsigned len = constraints.length();
> > - if (len > 0)
> > - {
> > - int nalternatives = n_occurrences (',', constraints[0]);
> > -
> > - if (nalternatives + 1 > MAX_RECOG_ALTERNATIVES)
> > - {
> > - error ("too many alternatives in %<asm%>");
> > - return false;
> > - }
> > -
> > - for (unsigned i = 1; i < len; ++i)
> > - if (n_occurrences (',', constraints[i]) != nalternatives)
> > - {
> > - error ("operand constraints for %<asm%> differ "
> > - "in number of alternatives");
> > - return false;
> > - }
> > - }
> > - return true;
> > -}
> > -
> > /* Check for overlap between registers marked in CLOBBERED_REGS and
> > anything inappropriate in T. Emit error and return the register
> > variable definition for error, NULL_TREE for ok. */
> > @@ -3158,10 +3120,6 @@ expand_asm_stmt (gasm *stmt)
> > = TREE_STRING_POINTER (TREE_VALUE (TREE_PURPOSE (t)));
> > }
> >
> > - /* ??? Diagnose during gimplification? */
> > - if (! check_operand_nalternatives (constraints))
> > - return;
> > -
> > /* Count the number of meaningful clobbered registers, ignoring what
> > we would ignore later. */
> > auto_vec<rtx> clobber_rvec;
> > diff --git a/gcc/genpreds.cc b/gcc/genpreds.cc
> > index 55d149e8a40..f0d59cb0846 100644
> > --- a/gcc/genpreds.cc
> > +++ b/gcc/genpreds.cc
> > @@ -1148,7 +1148,7 @@ write_insn_constraint_len (void)
> > unsigned int i;
> >
> > puts ("static inline size_t\n"
> > - "insn_constraint_len (char fc, const char *str ATTRIBUTE_UNUSED)\n"
> > + "insn_constraint_len (char fc, const char *str)\n"
> > "{\n"
> > " switch (fc)\n"
> > " {");
> > @@ -1181,6 +1181,8 @@ write_insn_constraint_len (void)
> >
> > puts (" default: break;\n"
> > " }\n"
> > + " if (str[0] == '{')\n"
> > + " return ((const char *)rawmemchr (str + 1, '}') - str) + 1;\n"
> > " return 1;\n"
> > "}\n");
> > }
> > diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc
> > index b0ed58ed0f9..b4b16e75023 100644
> > --- a/gcc/gimplify.cc
> > +++ b/gcc/gimplify.cc
> > @@ -70,6 +70,9 @@ along with GCC; see the file COPYING3. If not see
> > #include "omp-offload.h"
> > #include "context.h"
> > #include "tree-nested.h"
> > +#include "insn-config.h"
> > +#include "recog.h"
> > +#include "output.h"
> >
> > /* Identifier for a basic condition, mapping it to other basic conditions of
> > its Boolean expression. Basic conditions given the same uid (in the same
> > @@ -6952,6 +6955,42 @@ gimplify_addr_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
> > return ret;
> > }
> >
> > +/* Return the number of times character C occurs in string S. */
> > +
> > +static int
> > +num_occurrences (int c, const char *s)
> > +{
> > + int n = 0;
> > + while (*s)
> > + n += (*s++ == c);
> > + return n;
> > +}
> > +
> > +/* A subroutine of gimplify_asm_expr. Check that all operands have
> > + the same number of alternatives. Return -1 if this is violated. Otherwise
> > + return the number of alternatives. */
> > +
> > +static int
> > +num_alternatives (const_tree link)
> > +{
> > + if (link == nullptr)
> > + return 0;
> > +
> > + const char *constraint = TREE_STRING_POINTER (TREE_VALUE (TREE_PURPOSE (link)));
> > + int num = num_occurrences (',', constraint);
> > +
> > + if (num + 1 > MAX_RECOG_ALTERNATIVES)
> > + return -1;
> > +
> > + for (link = TREE_CHAIN (link); link; link = TREE_CHAIN (link))
> > + {
> > + constraint = TREE_STRING_POINTER (TREE_VALUE (TREE_PURPOSE (link)));
> > + if (num_occurrences (',', constraint) != num)
> > + return -1;
> > + }
> > + return num + 1;
> > +}
> > +
> > /* Gimplify the operands of an ASM_EXPR. Input operands should be a gimple
> > value; output operands should be a gimple lvalue. */
> >
> > @@ -6982,6 +7021,35 @@ gimplify_asm_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
> > clobbers = NULL;
> > labels = NULL;
> >
> > + int num_alternatives_out = num_alternatives (ASM_OUTPUTS (expr));
> > + int num_alternatives_in = num_alternatives (ASM_INPUTS (expr));
> > + if (num_alternatives_out == -1 || num_alternatives_in == -1
> > + || (num_alternatives_out > 0 && num_alternatives_in > 0
> > + && num_alternatives_out != num_alternatives_in))
> > + {
> > + error ("operand constraints for %<asm%> differ "
> > + "in number of alternatives");
> > + return GS_ERROR;
> > + }
> > + int num_alternatives = MAX (num_alternatives_out, num_alternatives_in);
> > +
> > + /* Regarding hard register constraints ensure that each hard register is used
> > + at most once over all inputs/outputs and each alternative. Keep track in
> > + hardregs[0] which hard register is used via an asm register over all
> > + inputs/outputs. hardregs[i] for i >= 2 describes which hard registers are
> > + used for alternative i-2 over all inputs/outputs. hardregs[1] is a
> > + reduction of all alternatives, i.e., hardregs[1] |= hardregs[i] for i >= 2
> > + and describes whether a hard register is used in any alternative. This is
> > + just a shortcut instead of recomputing the union over all alternatives;
> > + possibly multiple times. */
> > + auto_vec<HARD_REG_SET> hardregs (num_alternatives + 2);
> > + for (int i = 0; i < num_alternatives + 2; ++i)
> > + {
> > + HARD_REG_SET hregset;
> > + CLEAR_HARD_REG_SET (hregset);
> > + hardregs.quick_push (hregset);
> > + }
> > +
> > ret = GS_ALL_DONE;
> > link_next = NULL_TREE;
> > for (i = 0, link = ASM_OUTPUTS (expr); link; ++i, link = link_next)
> > @@ -6998,8 +7066,8 @@ gimplify_asm_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
> > if (constraint_len == 0)
> > continue;
> >
> > - ok = parse_output_constraint (&constraint, i, 0, 0,
> > - &allows_mem, &allows_reg, &is_inout);
> > + ok = parse_output_constraint (&constraint, i, 0, 0, &allows_mem,
> > + &allows_reg, &is_inout, &hardregs);
> > if (!ok)
> > {
> > ret = GS_ERROR;
> > @@ -7062,6 +7130,24 @@ gimplify_asm_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
> > TREE_VALUE (link) = tem;
> > tret = GS_OK;
> > }
> > + if (VAR_P (op) && DECL_HARD_REGISTER (op))
> > + {
> > + tree id = DECL_ASSEMBLER_NAME (op);
> > + const char *asmspec = IDENTIFIER_POINTER (id) + 1;
> > + int hardreg = decode_reg_name (asmspec);
> > + if (hardreg >= 0)
> > + {
> > + if (TEST_HARD_REG_BIT (hardregs[0], hardreg)
> > + || TEST_HARD_REG_BIT (hardregs[1], hardreg))
> > + {
> > + error ("multiple outputs to hard register: %s",
> > + reg_names[hardreg]);
> > + return GS_ERROR;
> > + }
> > + else
> > + SET_HARD_REG_BIT (hardregs[0], hardreg);
> > + }
> > + }
> > }
> >
> > vec_safe_push (outputs, link);
> > @@ -7161,13 +7247,16 @@ gimplify_asm_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
> > }
> > }
> >
> > + for (unsigned int i = 0; i < hardregs.length (); ++i)
> > + CLEAR_HARD_REG_SET (hardregs[i]);
> > +
> > link_next = NULL_TREE;
> > for (link = ASM_INPUTS (expr); link; ++i, link = link_next)
> > {
> > link_next = TREE_CHAIN (link);
> > constraint = TREE_STRING_POINTER (TREE_VALUE (TREE_PURPOSE (link)));
> > - parse_input_constraint (&constraint, 0, 0, noutputs, 0,
> > - oconstraints, &allows_mem, &allows_reg);
> > + parse_input_constraint (&constraint, 0, 0, noutputs, 0, oconstraints,
> > + &allows_mem, &allows_reg, &hardregs);
> >
> > /* If we can't make copies, we can only accept memory. */
> > tree intype = TREE_TYPE (TREE_VALUE (link));
> > @@ -7241,6 +7330,24 @@ gimplify_asm_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
> > is_gimple_asm_val, fb_rvalue);
> > if (tret == GS_ERROR)
> > ret = tret;
> > + tree inputv = TREE_VALUE (link);
> > + if (VAR_P (inputv) && DECL_HARD_REGISTER (inputv))
> > + {
> > + tree id = DECL_ASSEMBLER_NAME (inputv);
> > + const char *asmspec = IDENTIFIER_POINTER (id) + 1;
> > + int hardreg = decode_reg_name (asmspec);
> > + if (hardreg >= 0)
> > + {
> > + if (TEST_HARD_REG_BIT (hardregs[1], hardreg))
> > + {
> > + error ("multiple inputs to hard register: %s",
> > + reg_names[hardreg]);
> > + return GS_ERROR;
> > + }
> > + else
> > + SET_HARD_REG_BIT (hardregs[0], hardreg);
> > + }
> > + }
> > }
> >
> > TREE_CHAIN (link) = NULL_TREE;
> > diff --git a/gcc/lra-constraints.cc b/gcc/lra-constraints.cc
> > index e945a4da451..d81753eefaa 100644
> > --- a/gcc/lra-constraints.cc
> > +++ b/gcc/lra-constraints.cc
> > @@ -114,6 +114,7 @@
> > #include "target.h"
> > #include "rtl.h"
> > #include "tree.h"
> > +#include "stmt.h"
> > #include "predict.h"
> > #include "df.h"
> > #include "memmodel.h"
> > @@ -2165,6 +2166,7 @@ process_alt_operands (int only_alternative)
> > bool costly_p;
> > enum reg_class cl;
> > const HARD_REG_SET *cl_filter;
> > + HARD_REG_SET hregset;
> >
> > /* Calculate some data common for all alternatives to speed up the
> > function. */
> > @@ -2536,6 +2538,21 @@ process_alt_operands (int only_alternative)
> > cl_filter = nullptr;
> > goto reg;
> >
> > + case '{':
> > + {
> > + /* Currently this form of constraint is only allowed in
> > + asm statements which are verified during gimplify,
> > + i.e., regno >= 0 holds for those. genoutput fails on
> > + it. For future proofness assert it. */
> > + int regno = parse_constraint_regname (p);
> > + gcc_assert (regno >= 0);
> > + cl = REGNO_REG_CLASS (regno);
> > + CLEAR_HARD_REG_SET (hregset);
> > + SET_HARD_REG_BIT (hregset, regno);
> > + cl_filter = &hregset;
> > + goto reg;
> > + }
> > +
> > default:
> > cn = lookup_constraint (p);
> > switch (get_constraint_type (cn))
> > diff --git a/gcc/recog.cc b/gcc/recog.cc
> > index a6799e3f5e6..8a474cfb8a7 100644
> > --- a/gcc/recog.cc
> > +++ b/gcc/recog.cc
> > @@ -25,6 +25,7 @@ along with GCC; see the file COPYING3. If not see
> > #include "target.h"
> > #include "rtl.h"
> > #include "tree.h"
> > +#include "stmt.h"
> > #include "cfghooks.h"
> > #include "df.h"
> > #include "memmodel.h"
> > @@ -2296,10 +2297,11 @@ asm_operand_ok (rtx op, const char *constraint, const char **constraints)
> > switch (get_constraint_type (cn))
> > {
> > case CT_REGISTER:
> > - if (!result
> > - && reg_class_for_constraint (cn) != NO_REGS
> > - && GET_MODE (op) != BLKmode
> > - && register_operand (op, VOIDmode))
> > + if ((!result
> > + && reg_class_for_constraint (cn) != NO_REGS
> > + && GET_MODE (op) != BLKmode
> > + && register_operand (op, VOIDmode))
> > + || constraint[0] == '{')
> > result = 1;
> > break;
> >
> > @@ -3231,6 +3233,10 @@ constrain_operands (int strict, alternative_mask alternatives)
> > win = true;
> > break;
> >
> > + case '{':
> > + win = true;
> > + break;
> > +
> > default:
> > {
> > enum constraint_num cn = lookup_constraint (p);
> > diff --git a/gcc/stmt.cc b/gcc/stmt.cc
> > index ae1527f0a19..1f20b09f90e 100644
> > --- a/gcc/stmt.cc
> > +++ b/gcc/stmt.cc
> > @@ -39,6 +39,7 @@ along with GCC; see the file COPYING3. If not see
> > #include "emit-rtl.h"
> > #include "pretty-print.h"
> > #include "diagnostic-core.h"
> > +#include "output.h"
> >
> > #include "fold-const.h"
> > #include "varasm.h"
> > @@ -174,6 +175,32 @@ expand_label (tree label)
> > maybe_set_first_label_num (label_r);
> > }
> > \f
> > +/* Parse a hard register constraint and return its number or -1 in case of an
> > + error. BEGIN should point to a string of the form "{regname}". For the
> > + sake of simplicity assume that a register name is not longer than 31
> > + characters, if not error out. */
> > +
> > +int
> > +parse_constraint_regname (const char *begin)
> > +{
> > + if (*begin != '{')
> > + return -1;
> > + ++begin;
> > + const char *end = begin;
> > + while (*end != '}' && *end != '\0')
> > + ++end;
> > + if (*end != '}' || end == begin)
> > + return -1;
> > + ptrdiff_t len = end - begin;
> > + if (len >= 31)
> > + return -1;
> > + char regname[32];
> > + memcpy (regname, begin, len);
> > + regname[len] = '\0';
> > + int regno = decode_reg_name (regname);
> > + return regno;
> > +}
> > +
> > /* Parse the output constraint pointed to by *CONSTRAINT_P. It is the
> > OPERAND_NUMth output operand, indexed from zero. There are NINPUTS
> > inputs and NOUTPUTS outputs to this extended-asm. Upon return,
> > @@ -190,7 +217,8 @@ expand_label (tree label)
> > bool
> > parse_output_constraint (const char **constraint_p, int operand_num,
> > int ninputs, int noutputs, bool *allows_mem,
> > - bool *allows_reg, bool *is_inout)
> > + bool *allows_reg, bool *is_inout,
> > + vec<HARD_REG_SET> *hardregs)
> > {
> > const char *constraint = *constraint_p;
> > const char *p;
> > @@ -244,6 +272,8 @@ parse_output_constraint (const char **constraint_p, int operand_num,
> > constraint = *constraint_p;
> > }
> >
> > + unsigned int alternative = 2;
> > +
> > /* Loop through the constraint string. */
> > for (p = constraint + 1; *p; )
> > {
> > @@ -268,7 +298,11 @@ parse_output_constraint (const char **constraint_p, int operand_num,
> > case 'E': case 'F': case 'G': case 'H':
> > case 's': case 'i': case 'n':
> > case 'I': case 'J': case 'K': case 'L': case 'M':
> > - case 'N': case 'O': case 'P': case ',':
> > + case 'N': case 'O': case 'P':
> > + break;
> > +
> > + case ',':
> > + ++alternative;
> > break;
> >
> > case '0': case '1': case '2': case '3': case '4':
> > @@ -289,6 +323,33 @@ parse_output_constraint (const char **constraint_p, int operand_num,
> > *allows_mem = true;
> > break;
> >
> > + case '{':
> > + {
> > + int regno = parse_constraint_regname (p);
> > + if (regno < 0)
> > + {
> > + error ("invalid output constraint: %s", p);
> > + return false;
> > + }
> > + if (hardregs)
> > + {
> > + if (TEST_HARD_REG_BIT ((*hardregs)[0], regno)
> > + || TEST_HARD_REG_BIT ((*hardregs)[alternative], regno))
> > + {
> > + error ("multiple outputs to hard register: %s",
> > + reg_names[regno]);
> > + return false;
> > + }
> > + else
> > + {
> > + SET_HARD_REG_BIT ((*hardregs)[1], regno);
> > + SET_HARD_REG_BIT ((*hardregs)[alternative], regno);
> > + }
> > + }
> > + *allows_reg = true;
> > + break;
> > + }
> > +
> > default:
> > if (!ISALPHA (*p))
> > break;
> > @@ -317,7 +378,8 @@ bool
> > parse_input_constraint (const char **constraint_p, int input_num,
> > int ninputs, int noutputs, int ninout,
> > const char * const * constraints,
> > - bool *allows_mem, bool *allows_reg)
> > + bool *allows_mem, bool *allows_reg,
> > + vec<HARD_REG_SET> *hardregs)
> > {
> > const char *constraint = *constraint_p;
> > const char *orig_constraint = constraint;
> > @@ -332,6 +394,8 @@ parse_input_constraint (const char **constraint_p, int input_num,
> >
> > /* Make sure constraint has neither `=', `+', nor '&'. */
> >
> > + unsigned int alternative = 2;
> > +
> > for (j = 0; j < c_len; j += CONSTRAINT_LEN (constraint[j], constraint+j))
> > switch (constraint[j])
> > {
> > @@ -358,7 +422,11 @@ parse_input_constraint (const char **constraint_p, int input_num,
> > case 'E': case 'F': case 'G': case 'H':
> > case 's': case 'i': case 'n':
> > case 'I': case 'J': case 'K': case 'L': case 'M':
> > - case 'N': case 'O': case 'P': case ',':
> > + case 'N': case 'O': case 'P':
> > + break;
> > +
> > + case ',':
> > + ++alternative;
> > break;
> >
> > /* Whether or not a numeric constraint allows a register is
> > @@ -408,6 +476,32 @@ parse_input_constraint (const char **constraint_p, int input_num,
> > *allows_mem = true;
> > break;
> >
> > + case '{':
> > + {
> > + int regno = parse_constraint_regname (constraint + j);
> > + if (regno < 0)
> > + {
> > + error ("invalid input constraint: %s", constraint + j);
> > + return false;
> > + }
> > + if (hardregs)
> > + {
> > + if (TEST_HARD_REG_BIT ((*hardregs)[0], regno)
> > + || TEST_HARD_REG_BIT ((*hardregs)[alternative], regno))
> > + {
> > + error ("multiple inputs to hard register: %s",
> > + reg_names[regno]);
> > + }
> > + else
> > + {
> > + SET_HARD_REG_BIT ((*hardregs)[1], regno);
> > + SET_HARD_REG_BIT ((*hardregs)[alternative], regno);
> > + }
> > + }
> > + *allows_reg = true;
> > + break;
> > + }
> > +
> > default:
> > if (! ISALPHA (constraint[j]))
> > {
> > diff --git a/gcc/stmt.h b/gcc/stmt.h
> > index a2caae7121b..05889ff3798 100644
> > --- a/gcc/stmt.h
> > +++ b/gcc/stmt.h
> > @@ -20,11 +20,17 @@ along with GCC; see the file COPYING3. If not see
> > #ifndef GCC_STMT_H
> > #define GCC_STMT_H
> >
> > +#include "target.h"
> > +#include "hard-reg-set.h"
> > +
> > extern void expand_label (tree);
> > extern bool parse_output_constraint (const char **, int, int, int,
> > - bool *, bool *, bool *);
> > + bool *, bool *, bool *,
> > + vec<HARD_REG_SET> * = nullptr);
> > extern bool parse_input_constraint (const char **, int, int, int, int,
> > - const char * const *, bool *, bool *);
> > + const char * const *, bool *, bool *,
> > + vec<HARD_REG_SET> * = nullptr);
> > +extern int parse_constraint_regname (const char *);
> > extern tree resolve_asm_operand_names (tree, tree, tree, tree);
> > #ifdef HARD_CONST
> > /* Silly ifdef to avoid having all includers depend on hard-reg-set.h. */
> > diff --git a/gcc/testsuite/gcc.target/s390/asm-hard-reg-1.c b/gcc/testsuite/gcc.target/s390/asm-hard-reg-1.c
> > new file mode 100644
> > index 00000000000..53895d98663
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/s390/asm-hard-reg-1.c
> > @@ -0,0 +1,103 @@
> > +/* { dg-do compile { target { lp64 } } } */
> > +/* { dg-options "-O2 -march=z13" } */
> > +/* { dg-final { check-function-bodies "**" "" "" } } */
> > +
> > +/*
> > +** test_in_1:
> > +** foo %r2
> > +** br %r14
> > +*/
> > +
> > +int
> > +test_in_1 (int x)
> > +{
> > + asm ("foo %0" :: "{r2}" (x));
> > + return x;
> > +}
> > +
> > +/*
> > +** test_in_2:
> > +** lgr (%r[0-9]+),%r2
> > +** lhi %r2,42
> > +** foo %r2
> > +** lgr %r2,\1
> > +** br %r14
> > +*/
> > +
> > +int
> > +test_in_2 (int x)
> > +{
> > + asm ("foo %0" :: "{r2}" (42));
> > + return x;
> > +}
> > +
> > +/*
> > +** test_in_3:
> > +** stmg %r12,%r15,96\(%r15\)
> > +** lay %r15,-160\(%r15\)
> > +** lgr (%r[0-9]+),%r2
> > +** ahi %r2,1
> > +** lgfr %r2,%r2
> > +** brasl %r14,foo@PLT
> > +** lr %r3,%r2
> > +** lr %r2,\1
> > +** foo %r3,%r2
> > +** lgr %r2,\1
> > +** lmg %r12,%r15,256\(%r15\)
> > +** br %r14
> > +*/
> > +
> > +extern int foo (int);
> > +
> > +int
> > +test_in_3 (int x)
> > +{
> > + asm ("foo %0,%1\n" :: "{r3}" (foo (x + 1)), "{r2}" (x));
> > + return x;
> > +}
> > +
> > +/*
> > +** test_out_1:
> > +** foo %r3
> > +** lgfr %r2,%r3
> > +** br %r14
> > +*/
> > +
> > +int
> > +test_out_1 (void)
> > +{
> > + int x;
> > + asm ("foo %0" : "={r3}" (x));
> > + return x;
> > +}
> > +
> > +/*
> > +** test_out_2:
> > +** lgr (%r[0-9]+),%r2
> > +** foo %r2
> > +** ark (%r[0-9]+),\1,%r2
> > +** lgfr %r2,\2
> > +** br %r14
> > +*/
> > +
> > +int
> > +test_out_2 (int x)
> > +{
> > + int y;
> > + asm ("foo %0" : "={r2}" (y));
> > + return x + y;
> > +}
> > +
> > +/*
> > +** test_inout_1:
> > +** foo %r2
> > +** lgfr %r2,%r2
> > +** br %r14
> > +*/
> > +
> > +int
> > +test_inout_1 (int x)
> > +{
> > + asm ("foo %0" : "+{r2}" (x));
> > + return x;
> > +}
> > diff --git a/gcc/testsuite/gcc.target/s390/asm-hard-reg-2.c b/gcc/testsuite/gcc.target/s390/asm-hard-reg-2.c
> > new file mode 100644
> > index 00000000000..9f3c221b937
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/s390/asm-hard-reg-2.c
> > @@ -0,0 +1,29 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O2" } */
> > +/* { dg-final { check-function-bodies "**" "" "" } } */
> > +
> > +/*
> > +** test_1:
> > +** lr %r5,%r2
> > +** foo %r5,%r3
> > +** br %r14
> > +*/
> > +
> > +void
> > +test_1 (int x, int *y)
> > +{
> > + asm ("foo %0,%1" :: "m{r4},{r5}" (x), "m,r" (y));
> > +}
> > +
> > +/*
> > +** test_2:
> > +** lr %r4,%r2
> > +** foo %r4,0\(%r3\)
> > +** br %r14
> > +*/
> > +
> > +void
> > +test_2 (int x, int *y)
> > +{
> > + asm ("foo %0,%1" :: "m{r4},{r5}" (x), "m,r" (*y));
> > +}
> > diff --git a/gcc/testsuite/gcc.target/s390/asm-hard-reg-3.c b/gcc/testsuite/gcc.target/s390/asm-hard-reg-3.c
> > new file mode 100644
> > index 00000000000..0edcdd3cfde
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/s390/asm-hard-reg-3.c
> > @@ -0,0 +1,24 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O2" } */
> > +
> > +void
> > +test (void)
> > +{
> > + int x, y;
> > + register int r4 asm ("r4") = 0;
> > +
> > + asm ("" :: "{}" (42)); /* { dg-error "invalid input constraint: \{\}" } */
> > + asm ("" :: "{r4" (42)); /* { dg-error "invalid input constraint: \{r4" } */
> > + asm ("" :: "{r17}" (42)); /* { dg-error "invalid input constraint: \{r17\}" } */
> > +
> > + asm ("" :: "r" (r4), "{r4}" (42)); /* { dg-error "multiple inputs to hard register: %r4" } */
> > + asm ("" :: "{r4}" (42), "r" (r4)); /* { dg-error "multiple inputs to hard register: %r4" } */
> > + asm ("" :: "{r4}" (42), "{r4}" (42)); /* { dg-error "multiple inputs to hard register: %r4" } */
> > + asm ("" :: "{r2},{r4}" (42), "{r4},{r3}" (42));
> > + asm ("" :: "{r2},{r4}" (42), "{r3},{r4}" (42)); /* { dg-error "multiple inputs to hard register: %r4" } */
> > + asm ("" :: "{r4},{r2}" (42), "{r4},{r3}" (42)); /* { dg-error "multiple inputs to hard register: %r4" } */
> > + asm ("" :: "{r3}{r4}" (42), "{r4}" (42)); /* { dg-error "multiple inputs to hard register: %r4" } */
> > + asm ("" : "+{r4}" (x), "={r4}" (y)); /* { dg-error "multiple outputs to hard register: %r4" } */
> > + asm ("" : "={r4}" (y) : "{r4}" (42), "0" (42)); /* { dg-error "multiple inputs to hard register: %r4" } */
> > + asm ("" : "+{r4}" (x) : "{r4}" (y)); /* { dg-error "multiple inputs to hard register: %r4" } */
> > +}
> > diff --git a/gcc/testsuite/lib/scanasm.exp b/gcc/testsuite/lib/scanasm.exp
> > index 6cf9997240d..d09372096a5 100644
> > --- a/gcc/testsuite/lib/scanasm.exp
> > +++ b/gcc/testsuite/lib/scanasm.exp
> > @@ -896,6 +896,10 @@ proc configure_check-function-bodies { config } {
> > set up_config(fluff) {^\s*(?://)}
> > } elseif { [istarget *-*-darwin*] } {
> > set up_config(fluff) {^\s*(?:\.|//|@)|^L[0-9ABCESV]}
> > + } elseif { [istarget s390*-*-*] } {
> > + # Additionally to the defaults skip lines beginning with a # resulting
> > + # from inline asm.
> > + set up_config(fluff) {^\s*(?:\.|//|@|$|#)}
> > } else {
> > # Skip lines beginning with labels ('.L[...]:') or other directives
> > # ('.align', '.cfi_startproc', '.quad [...]', '.text', etc.), '//' or
> > --
> > 2.45.1
> >
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Hard register asm constraint
2024-06-24 5:50 ` Stefan Schulze Frielinghaus
@ 2024-06-25 14:03 ` Paul Koning
2024-06-25 14:35 ` Maciej W. Rozycki
2024-06-25 16:04 ` Stefan Schulze Frielinghaus
0 siblings, 2 replies; 14+ messages in thread
From: Paul Koning @ 2024-06-25 14:03 UTC (permalink / raw)
To: Stefan Schulze Frielinghaus; +Cc: gcc-patches
> On Jun 24, 2024, at 1:50 AM, Stefan Schulze Frielinghaus <stefansf@linux.ibm.com> wrote:
>
> Ping.
>
> On Mon, Jun 10, 2024 at 07:19:19AM +0200, Stefan Schulze Frielinghaus wrote:
>> Ping.
>>
>> On Fri, May 24, 2024 at 11:13:12AM +0200, Stefan Schulze Frielinghaus wrote:
>>> This implements hard register constraints for inline asm. A hard register
>>> constraint is of the form {regname} where regname is any valid register. This
>>> basically renders register asm superfluous. For example, the snippet
>>>
>>> int test (int x, int y)
>>> {
>>> register int r4 asm ("r4") = x;
>>> register int r5 asm ("r5") = y;
>>> unsigned int copy = y;
>>> asm ("foo %0,%1,%2" : "+d" (r4) : "d" (r5), "d" (copy));
>>> return r4;
>>> }
>>>
>>> could be rewritten into
>>>
>>> int test (int x, int y)
>>> {
>>> asm ("foo %0,%1,%2" : "+{r4}" (x) : "{r5}" (y), "d" (y));
>>> return x;
>>> }
I like this idea but I'm wondering: regular constraints specify what sort of value is needed, for example an int vs. a short int vs. a float. The notation you've shown doesn't seem to have that aspect.
The other comment is that I didn't see documentation updates to reflect this new feature.
paul
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Hard register asm constraint
2024-06-25 14:03 ` Paul Koning
@ 2024-06-25 14:35 ` Maciej W. Rozycki
2024-06-25 16:04 ` Stefan Schulze Frielinghaus
1 sibling, 0 replies; 14+ messages in thread
From: Maciej W. Rozycki @ 2024-06-25 14:35 UTC (permalink / raw)
To: Paul Koning; +Cc: Stefan Schulze Frielinghaus, gcc-patches
On Tue, 25 Jun 2024, Paul Koning wrote:
> >>> could be rewritten into
> >>>
> >>> int test (int x, int y)
> >>> {
> >>> asm ("foo %0,%1,%2" : "+{r4}" (x) : "{r5}" (y), "d" (y));
> >>> return x;
> >>> }
>
> I like this idea but I'm wondering: regular constraints specify what
> sort of value is needed, for example an int vs. a short int vs. a float.
Isn't that inferred from the data type of the associated expression used,
the types of `x' and `y' in this case? Then the constraints only tell the
middle end where that data comes from or goes to at the boundaries of an
asm.
Maciej
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Hard register asm constraint
2024-06-25 14:03 ` Paul Koning
2024-06-25 14:35 ` Maciej W. Rozycki
@ 2024-06-25 16:04 ` Stefan Schulze Frielinghaus
2024-06-25 17:02 ` Paul Koning
1 sibling, 1 reply; 14+ messages in thread
From: Stefan Schulze Frielinghaus @ 2024-06-25 16:04 UTC (permalink / raw)
To: Paul Koning; +Cc: gcc-patches
On Tue, Jun 25, 2024 at 10:03:34AM -0400, Paul Koning wrote:
>
>
> > On Jun 24, 2024, at 1:50 AM, Stefan Schulze Frielinghaus <stefansf@linux.ibm.com> wrote:
> >
> > Ping.
> >
> > On Mon, Jun 10, 2024 at 07:19:19AM +0200, Stefan Schulze Frielinghaus wrote:
> >> Ping.
> >>
> >> On Fri, May 24, 2024 at 11:13:12AM +0200, Stefan Schulze Frielinghaus wrote:
> >>> This implements hard register constraints for inline asm. A hard register
> >>> constraint is of the form {regname} where regname is any valid register. This
> >>> basically renders register asm superfluous. For example, the snippet
> >>>
> >>> int test (int x, int y)
> >>> {
> >>> register int r4 asm ("r4") = x;
> >>> register int r5 asm ("r5") = y;
> >>> unsigned int copy = y;
> >>> asm ("foo %0,%1,%2" : "+d" (r4) : "d" (r5), "d" (copy));
> >>> return r4;
> >>> }
> >>>
> >>> could be rewritten into
> >>>
> >>> int test (int x, int y)
> >>> {
> >>> asm ("foo %0,%1,%2" : "+{r4}" (x) : "{r5}" (y), "d" (y));
> >>> return x;
> >>> }
>
> I like this idea but I'm wondering: regular constraints specify what sort of value is needed, for example an int vs. a short int vs. a float. The notation you've shown doesn't seem to have that aspect.
As Maciej already pointed out the type of the expression should suffice.
My assumption was that an asm can deal with a value as is or its
promoted value. At least for integer values this should be fine and
AFAICS is also the case for simple constraints like "r" which do not
define any mode. I've probably overseen something but which constraint
differentiates between int vs short? However, you have a good point
with this and I should test this more.
> The other comment is that I didn't see documentation updates to reflect this new feature.
I didn't came up with documentation yet since I was not sure whether
such a proposal would be accepted at all, i.e., just wanted to hear
whether you see some show stoppers or not. Assuming this goes well I
guess it should be documented under simple constraints
https://gcc.gnu.org/onlinedocs/gcc/Simple-Constraints.html
Thanks,
Stefan
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Hard register asm constraint
2024-06-25 16:04 ` Stefan Schulze Frielinghaus
@ 2024-06-25 17:02 ` Paul Koning
2024-06-26 12:54 ` Stefan Schulze Frielinghaus
0 siblings, 1 reply; 14+ messages in thread
From: Paul Koning @ 2024-06-25 17:02 UTC (permalink / raw)
To: Stefan Schulze Frielinghaus; +Cc: gcc-patches
> On Jun 25, 2024, at 12:04 PM, Stefan Schulze Frielinghaus <stefansf@linux.ibm.com> wrote:
>
> On Tue, Jun 25, 2024 at 10:03:34AM -0400, Paul Koning wrote:
>>
>>>>> ...
>>>>> could be rewritten into
>>>>>
>>>>> int test (int x, int y)
>>>>> {
>>>>> asm ("foo %0,%1,%2" : "+{r4}" (x) : "{r5}" (y), "d" (y));
>>>>> return x;
>>>>> }
>>
>> I like this idea but I'm wondering: regular constraints specify what sort of value is needed, for example an int vs. a short int vs. a float. The notation you've shown doesn't seem to have that aspect.
>
> As Maciej already pointed out the type of the expression should suffice.
> My assumption was that an asm can deal with a value as is or its
> promoted value. At least for integer values this should be fine and
> AFAICS is also the case for simple constraints like "r" which do not
> define any mode. I've probably overseen something but which constraint
> differentiates between int vs short? However, you have a good point
> with this and I should test this more.
I thought there was but I may be confused. On the other hand, there definitely are (machine dependent) constraints that distinguish, say, float from integer registers; pdp11 is an example. If you were to use an "a" constraint, that means a floating point register and the compiler will detect attempts to pass non-float operands ("Inconsistent operand constraints...").
I see that the existing "register int ..." syntax appears to check that the register is the right type for the data type given for it, so for example on pdp11,
register int ac1 asm ("ac1") = i;
fails ("register ... isn't suitable for data type"). I assume your new syntax would perform the same check and produce roughly the same error message. You might verify that. On pdp11, trying to use, for example, "r0" for a float, or "ac0" for an int, would produce that error.
With all that, I think your approach does work right and the question I raised isn't actually a problem.
>> The other comment is that I didn't see documentation updates to reflect this new feature.
>
> I didn't came up with documentation yet since I was not sure whether
> such a proposal would be accepted at all, i.e., just wanted to hear
> whether you see some show stoppers or not. Assuming this goes well I
> guess it should be documented under simple constraints
> https://gcc.gnu.org/onlinedocs/gcc/Simple-Constraints.html
>
> Thanks,
> Stefan
That seems right. I think it would be good for one of the global maintainers, or people of similar stature, to say whether Stefan's proposal is one that should be considered. My 2c worth is that it should be.
The documentation might want to mention that the compiler will confirm that the register specified is suitable for the type given to it, just as it does for the old syntax.
paul
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Hard register asm constraint
2024-06-25 17:02 ` Paul Koning
@ 2024-06-26 12:54 ` Stefan Schulze Frielinghaus
2024-06-26 15:10 ` Paul Koning
0 siblings, 1 reply; 14+ messages in thread
From: Stefan Schulze Frielinghaus @ 2024-06-26 12:54 UTC (permalink / raw)
To: Paul Koning; +Cc: gcc-patches
On Tue, Jun 25, 2024 at 01:02:39PM -0400, Paul Koning wrote:
>
>
> > On Jun 25, 2024, at 12:04 PM, Stefan Schulze Frielinghaus <stefansf@linux.ibm.com> wrote:
> >
> > On Tue, Jun 25, 2024 at 10:03:34AM -0400, Paul Koning wrote:
> >>
> >>>>> ...
> >>>>> could be rewritten into
> >>>>>
> >>>>> int test (int x, int y)
> >>>>> {
> >>>>> asm ("foo %0,%1,%2" : "+{r4}" (x) : "{r5}" (y), "d" (y));
> >>>>> return x;
> >>>>> }
> >>
> >> I like this idea but I'm wondering: regular constraints specify what sort of value is needed, for example an int vs. a short int vs. a float. The notation you've shown doesn't seem to have that aspect.
> >
> > As Maciej already pointed out the type of the expression should suffice.
> > My assumption was that an asm can deal with a value as is or its
> > promoted value. At least for integer values this should be fine and
> > AFAICS is also the case for simple constraints like "r" which do not
> > define any mode. I've probably overseen something but which constraint
> > differentiates between int vs short? However, you have a good point
> > with this and I should test this more.
>
> I thought there was but I may be confused. On the other hand, there definitely are (machine dependent) constraints that distinguish, say, float from integer registers; pdp11 is an example. If you were to use an "a" constraint, that means a floating point register and the compiler will detect attempts to pass non-float operands ("Inconsistent operand constraints...").
>
> I see that the existing "register int ..." syntax appears to check that the register is the right type for the data type given for it, so for example on pdp11,
>
> register int ac1 asm ("ac1") = i;
>
> fails ("register ... isn't suitable for data type"). I assume your new syntax would perform the same check and produce roughly the same error message. You might verify that. On pdp11, trying to use, for example, "r0" for a float, or "ac0" for an int, would produce that error.
Right, so far I don't error out here which I will change. It basically
results in bit casting floats to ints currently.
Just one thing to note: this is not a novel feature but pretty similar
to Rust's explicit register operands:
https://doc.rust-lang.org/rust-by-example/unsafe/asm.html#explicit-register-operands
Cheers,
Stefan
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Hard register asm constraint
2024-06-26 12:54 ` Stefan Schulze Frielinghaus
@ 2024-06-26 15:10 ` Paul Koning
2024-06-27 6:25 ` Stefan Schulze Frielinghaus
0 siblings, 1 reply; 14+ messages in thread
From: Paul Koning @ 2024-06-26 15:10 UTC (permalink / raw)
To: Stefan Schulze Frielinghaus; +Cc: gcc-patches
> On Jun 26, 2024, at 8:54 AM, Stefan Schulze Frielinghaus <stefansf@linux.ibm.com> wrote:
>
> On Tue, Jun 25, 2024 at 01:02:39PM -0400, Paul Koning wrote:
>>
>>
>>> On Jun 25, 2024, at 12:04 PM, Stefan Schulze Frielinghaus <stefansf@linux.ibm.com> wrote:
>>>
>>> On Tue, Jun 25, 2024 at 10:03:34AM -0400, Paul Koning wrote:
>>>>
>>>>>>> ...
>>>>>>> could be rewritten into
>>>>>>>
>>>>>>> int test (int x, int y)
>>>>>>> {
>>>>>>> asm ("foo %0,%1,%2" : "+{r4}" (x) : "{r5}" (y), "d" (y));
>>>>>>> return x;
>>>>>>> }
>>>>
>>>> I like this idea but I'm wondering: regular constraints specify what sort of value is needed, for example an int vs. a short int vs. a float. The notation you've shown doesn't seem to have that aspect.
>>>
>>> As Maciej already pointed out the type of the expression should suffice.
>>> My assumption was that an asm can deal with a value as is or its
>>> promoted value. At least for integer values this should be fine and
>>> AFAICS is also the case for simple constraints like "r" which do not
>>> define any mode. I've probably overseen something but which constraint
>>> differentiates between int vs short? However, you have a good point
>>> with this and I should test this more.
>>
>> I thought there was but I may be confused. On the other hand, there definitely are (machine dependent) constraints that distinguish, say, float from integer registers; pdp11 is an example. If you were to use an "a" constraint, that means a floating point register and the compiler will detect attempts to pass non-float operands ("Inconsistent operand constraints...").
>>
>> I see that the existing "register int ..." syntax appears to check that the register is the right type for the data type given for it, so for example on pdp11,
>>
>> register int ac1 asm ("ac1") = i;
>>
>> fails ("register ... isn't suitable for data type"). I assume your new syntax would perform the same check and produce roughly the same error message. You might verify that. On pdp11, trying to use, for example, "r0" for a float, or "ac0" for an int, would produce that error.
>
> Right, so far I don't error out here which I will change. It basically
> results in bit casting floats to ints currently.
That would be bad. For one thing, a PDP11 float doesn't fit in an integer register.
That also brings up another point (which applies to more mainstream targets as well): for data types that require multiple registers, say a register pair for a double length value, how is that handled? One possible answer is to reject that. Another would be to load a register pair.
This case applies to a "long int" on pdp11, or 32 bit MIPS, and probably a bunch of others.
paul
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Hard register asm constraint
2024-06-26 15:10 ` Paul Koning
@ 2024-06-27 6:25 ` Stefan Schulze Frielinghaus
0 siblings, 0 replies; 14+ messages in thread
From: Stefan Schulze Frielinghaus @ 2024-06-27 6:25 UTC (permalink / raw)
To: Paul Koning; +Cc: gcc-patches
On Wed, Jun 26, 2024 at 11:10:38AM -0400, Paul Koning wrote:
>
>
> > On Jun 26, 2024, at 8:54 AM, Stefan Schulze Frielinghaus <stefansf@linux.ibm.com> wrote:
> >
> > On Tue, Jun 25, 2024 at 01:02:39PM -0400, Paul Koning wrote:
> >>
> >>
> >>> On Jun 25, 2024, at 12:04 PM, Stefan Schulze Frielinghaus <stefansf@linux.ibm.com> wrote:
> >>>
> >>> On Tue, Jun 25, 2024 at 10:03:34AM -0400, Paul Koning wrote:
> >>>>
> >>>>>>> ...
> >>>>>>> could be rewritten into
> >>>>>>>
> >>>>>>> int test (int x, int y)
> >>>>>>> {
> >>>>>>> asm ("foo %0,%1,%2" : "+{r4}" (x) : "{r5}" (y), "d" (y));
> >>>>>>> return x;
> >>>>>>> }
> >>>>
> >>>> I like this idea but I'm wondering: regular constraints specify what sort of value is needed, for example an int vs. a short int vs. a float. The notation you've shown doesn't seem to have that aspect.
> >>>
> >>> As Maciej already pointed out the type of the expression should suffice.
> >>> My assumption was that an asm can deal with a value as is or its
> >>> promoted value. At least for integer values this should be fine and
> >>> AFAICS is also the case for simple constraints like "r" which do not
> >>> define any mode. I've probably overseen something but which constraint
> >>> differentiates between int vs short? However, you have a good point
> >>> with this and I should test this more.
> >>
> >> I thought there was but I may be confused. On the other hand, there definitely are (machine dependent) constraints that distinguish, say, float from integer registers; pdp11 is an example. If you were to use an "a" constraint, that means a floating point register and the compiler will detect attempts to pass non-float operands ("Inconsistent operand constraints...").
> >>
> >> I see that the existing "register int ..." syntax appears to check that the register is the right type for the data type given for it, so for example on pdp11,
> >>
> >> register int ac1 asm ("ac1") = i;
> >>
> >> fails ("register ... isn't suitable for data type"). I assume your new syntax would perform the same check and produce roughly the same error message. You might verify that. On pdp11, trying to use, for example, "r0" for a float, or "ac0" for an int, would produce that error.
> >
> > Right, so far I don't error out here which I will change. It basically
> > results in bit casting floats to ints currently.
>
> That would be bad. For one thing, a PDP11 float doesn't fit in an integer register.
>
> That also brings up another point (which applies to more mainstream targets as well): for data types that require multiple registers, say a register pair for a double length value, how is that handled? One possible answer is to reject that. Another would be to load a register pair.
>
> This case applies to a "long int" on pdp11, or 32 bit MIPS, and probably a bunch of others.
Absolutely, also on mainstream targets you could think of 128-bit integers
or long doubles which typically don't fit in (single) GPRs. I should
definitely add error handling for this. Similar, I don't error out for
non-primitive data types.
I will give register pairs a try.
Thanks for all your comments so far :)
Cheers,
Stefan
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Hard register asm constraint
2024-05-24 9:13 [PATCH] Hard register asm constraint Stefan Schulze Frielinghaus
2024-06-10 5:19 ` Stefan Schulze Frielinghaus
@ 2024-06-27 7:45 ` Georg-Johann Lay
2024-06-27 8:51 ` Stefan Schulze Frielinghaus
1 sibling, 1 reply; 14+ messages in thread
From: Georg-Johann Lay @ 2024-06-27 7:45 UTC (permalink / raw)
To: Stefan Schulze Frielinghaus, gcc-patches
Am 24.05.24 um 11:13 Am 25.06.24 um 16:03 schrieb Paul Koning:
>
>
>> On Jun 24, 2024, at 1:50 AM, Stefan Schulze Frielinghaus <stefansf@linux.ibm.com> wrote:
>>
>> Ping.
>>
>> On Mon, Jun 10, 2024 at 07:19:19AM +0200, Stefan Schulze Frielinghaus wrote:
>>> Ping.
>>>
>>> On Fri, May 24, 2024 at 11:13:12AM +0200, Stefan Schulze Frielinghaus wrote:
>>>> This implements hard register constraints for inline asm. A hard register
>>>> constraint is of the form {regname} where regname is any valid register. This
>>>> basically renders register asm superfluous. For example, the snippet
>>>>
>>>> int test (int x, int y)
>>>> {
>>>> register int r4 asm ("r4") = x;
>>>> register int r5 asm ("r5") = y;
>>>> unsigned int copy = y;
>>>> asm ("foo %0,%1,%2" : "+d" (r4) : "d" (r5), "d" (copy));
>>>> return r4;
>>>> }
>>>>
>>>> could be rewritten into
>>>>
>>>> int test (int x, int y)
>>>> {
>>>> asm ("foo %0,%1,%2" : "+{r4}" (x) : "{r5}" (y), "d" (y));
>>>> return x;
>>>> }
>
> I like this idea but I'm wondering: regular constraints specify what sort of value is needed, for example an int vs. a short int vs. a float. The notation you've shown doesn't seem to have that aspect.
>
> The other comment is that I didn't see documentation updates to reflect this new feature.
>
> paul
>
Stefan Schulze Frielinghaus:
> This implements hard register constraints for inline asm. A hard register
> constraint is of the form {regname} where regname is any valid register. This
> basically renders register asm superfluous. For example, the snippet
>
> int test (int x, int y)
> {
> register int r4 asm ("r4") = x;
> register int r5 asm ("r5") = y;
> unsigned int copy = y;
> asm ("foo %0,%1,%2" : "+d" (r4) : "d" (r5), "d" (copy));
> return r4;
> }
>
> could be rewritten into
>
> int test (int x, int y)
> {
> asm ("foo %0,%1,%2" : "+{r4}" (x) : "{r5}" (y), "d" (y));
> return x;
> }
Hi, can this also be used in machine descriptions?
It would make some insn handling much simpler, for example in
the avr backend.
That backend has insns that represent assembly sequences in libgcc
which have a smaller register footprint than plain calls. However
this requires that such insns have explicit description of which regs
go in and out.
The current solution uses hard regs, which works, but a proper
implementation would use register constraints. I tries that a while
ago, and register constraints lead to a code bloat even in places that
don't use these constraints due to the zillions of new register classes
like R22_1, R22;2, R22_4, R20_1, R20_2, R20_4 etc. that were required.
Your approach would allow to use hard register constraints in insns,
and so far the only problem is to determine how much hard regs are
used by the constraint. The gen tools that generates cc code from md
would use the operand's machine mode to infer the number of hard regs.
Johann
> As a side-effect this also solves the problem of call-clobbered registers.
> That being said, I was wondering whether we could utilize this feature in order
> to get rid of local register asm automatically? For example, converting
>
> // Result will be in r2 on s390
> extern int bar (void);
>
> void test (void)
> {
> register int x asm ("r2") = 42;
> bar ();
> asm ("foo %0\n" :: "r" (x));
> }
>
> into
>
> void test (void)
> {
> int x = 42;
> bar ();
> asm ("foo %0\n" :: "{r2}" (x));
> }
>
> in order to get rid of the limitation of call-clobbered registers which may
> lead to subtle bugs---especially if you think of non-obvious calls e.g.
> introduced by sanitizer/tracer/whatever. Since such a transformation has the
> potential to break existing code do you see any edge cases where this might be
> problematic or even show stoppers? Currently, even
>
> int test (void)
> {
> register int x asm ("r2") = 42;
> register int y asm ("r2") = 24;
> asm ("foo %0,%1\n" :: "r" (x), "r" (y));
> }
>
> is allowed which seems error prone to me. Thus, if 100% backwards
> compatibility would be required, then automatically converting every register
> asm to the new mechanism isn't viable. Still quite a lot could be transformed.
> Any thoughts?
>
> Currently I allow multiple alternatives as demonstrated by
> gcc/testsuite/gcc.target/s390/asm-hard-reg-2.c. However, since a hard register
> constraint is pretty specific I could also think of erroring out in case of
> alternatives. Are there any real use cases out there for multiple
> alternatives where one would like to use hard register constraints?
>
> With the current implementation we have a "user visible change" in the sense
> that for
>
> void test (void)
> {
> register int x asm ("r2") = 42;
> register int y asm ("r2") = 24;
> asm ("foo %0,%1\n" : "=r" (x), "=r" (y));
> }
>
> we do not get the error
>
> "invalid hard register usage between output operands"
>
> anymore but rather
>
> "multiple outputs to hard register: %r2"
>
> This is due to the error handling in gimplify_asm_expr (). Speaking of errors,
> I also error out earlier as before which means that e.g. in pr87600-2.c only
> the first error is reported and processing is stopped afterwards which means
> the subsequent tests fail.
>
> I've been skimming through all targets and it looks to me as if none is using
> curly brackets for their constraints. Of course, I may have missed something.
>
> Cheers,
> Stefan
>
> PS: Current state for Clang: https://reviews.llvm.org/D105142
>
> ---
> gcc/cfgexpand.cc | 42 -------
> gcc/genpreds.cc | 4 +-
> gcc/gimplify.cc | 115 +++++++++++++++++-
> gcc/lra-constraints.cc | 17 +++
> gcc/recog.cc | 14 ++-
> gcc/stmt.cc | 102 +++++++++++++++-
> gcc/stmt.h | 10 +-
> .../gcc.target/s390/asm-hard-reg-1.c | 103 ++++++++++++++++
> .../gcc.target/s390/asm-hard-reg-2.c | 29 +++++
> .../gcc.target/s390/asm-hard-reg-3.c | 24 ++++
> gcc/testsuite/lib/scanasm.exp | 4 +
> 11 files changed, 407 insertions(+), 57 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/s390/asm-hard-reg-1.c
> create mode 100644 gcc/testsuite/gcc.target/s390/asm-hard-reg-2.c
> create mode 100644 gcc/testsuite/gcc.target/s390/asm-hard-reg-3.c
>
> diff --git a/gcc/cfgexpand.cc b/gcc/cfgexpand.cc
> index 557cb28733b..47f71a2e803 100644
> --- a/gcc/cfgexpand.cc
> +++ b/gcc/cfgexpand.cc
> @@ -2955,44 +2955,6 @@ expand_asm_loc (tree string, int vol, location_t locus)
> emit_insn (body);
> }
>
> -/* Return the number of times character C occurs in string S. */
> -static int
> -n_occurrences (int c, const char *s)
> -{
> - int n = 0;
> - while (*s)
> - n += (*s++ == c);
> - return n;
> -}
> -
> -/* A subroutine of expand_asm_operands. Check that all operands have
> - the same number of alternatives. Return true if so. */
> -
> -static bool
> -check_operand_nalternatives (const vec<const char *> &constraints)
> -{
> - unsigned len = constraints.length();
> - if (len > 0)
> - {
> - int nalternatives = n_occurrences (',', constraints[0]);
> -
> - if (nalternatives + 1 > MAX_RECOG_ALTERNATIVES)
> - {
> - error ("too many alternatives in %<asm%>");
> - return false;
> - }
> -
> - for (unsigned i = 1; i < len; ++i)
> - if (n_occurrences (',', constraints[i]) != nalternatives)
> - {
> - error ("operand constraints for %<asm%> differ "
> - "in number of alternatives");
> - return false;
> - }
> - }
> - return true;
> -}
> -
> /* Check for overlap between registers marked in CLOBBERED_REGS and
> anything inappropriate in T. Emit error and return the register
> variable definition for error, NULL_TREE for ok. */
> @@ -3158,10 +3120,6 @@ expand_asm_stmt (gasm *stmt)
> = TREE_STRING_POINTER (TREE_VALUE (TREE_PURPOSE (t)));
> }
>
> - /* ??? Diagnose during gimplification? */
> - if (! check_operand_nalternatives (constraints))
> - return;
> -
> /* Count the number of meaningful clobbered registers, ignoring what
> we would ignore later. */
> auto_vec<rtx> clobber_rvec;
> diff --git a/gcc/genpreds.cc b/gcc/genpreds.cc
> index 55d149e8a40..f0d59cb0846 100644
> --- a/gcc/genpreds.cc
> +++ b/gcc/genpreds.cc
> @@ -1148,7 +1148,7 @@ write_insn_constraint_len (void)
> unsigned int i;
>
> puts ("static inline size_t\n"
> - "insn_constraint_len (char fc, const char *str ATTRIBUTE_UNUSED)\n"
> + "insn_constraint_len (char fc, const char *str)\n"
> "{\n"
> " switch (fc)\n"
> " {");
> @@ -1181,6 +1181,8 @@ write_insn_constraint_len (void)
>
> puts (" default: break;\n"
> " }\n"
> + " if (str[0] == '{')\n"
> + " return ((const char *)rawmemchr (str + 1, '}') - str) + 1;\n"
> " return 1;\n"
> "}\n");
> }
> diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc
> index b0ed58ed0f9..b4b16e75023 100644
> --- a/gcc/gimplify.cc
> +++ b/gcc/gimplify.cc
> @@ -70,6 +70,9 @@ along with GCC; see the file COPYING3. If not see
> #include "omp-offload.h"
> #include "context.h"
> #include "tree-nested.h"
> +#include "insn-config.h"
> +#include "recog.h"
> +#include "output.h"
>
> /* Identifier for a basic condition, mapping it to other basic conditions of
> its Boolean expression. Basic conditions given the same uid (in the same
> @@ -6952,6 +6955,42 @@ gimplify_addr_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
> return ret;
> }
>
> +/* Return the number of times character C occurs in string S. */
> +
> +static int
> +num_occurrences (int c, const char *s)
> +{
> + int n = 0;
> + while (*s)
> + n += (*s++ == c);
> + return n;
> +}
> +
> +/* A subroutine of gimplify_asm_expr. Check that all operands have
> + the same number of alternatives. Return -1 if this is violated. Otherwise
> + return the number of alternatives. */
> +
> +static int
> +num_alternatives (const_tree link)
> +{
> + if (link == nullptr)
> + return 0;
> +
> + const char *constraint = TREE_STRING_POINTER (TREE_VALUE (TREE_PURPOSE (link)));
> + int num = num_occurrences (',', constraint);
> +
> + if (num + 1 > MAX_RECOG_ALTERNATIVES)
> + return -1;
> +
> + for (link = TREE_CHAIN (link); link; link = TREE_CHAIN (link))
> + {
> + constraint = TREE_STRING_POINTER (TREE_VALUE (TREE_PURPOSE (link)));
> + if (num_occurrences (',', constraint) != num)
> + return -1;
> + }
> + return num + 1;
> +}
> +
> /* Gimplify the operands of an ASM_EXPR. Input operands should be a gimple
> value; output operands should be a gimple lvalue. */
>
> @@ -6982,6 +7021,35 @@ gimplify_asm_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
> clobbers = NULL;
> labels = NULL;
>
> + int num_alternatives_out = num_alternatives (ASM_OUTPUTS (expr));
> + int num_alternatives_in = num_alternatives (ASM_INPUTS (expr));
> + if (num_alternatives_out == -1 || num_alternatives_in == -1
> + || (num_alternatives_out > 0 && num_alternatives_in > 0
> + && num_alternatives_out != num_alternatives_in))
> + {
> + error ("operand constraints for %<asm%> differ "
> + "in number of alternatives");
> + return GS_ERROR;
> + }
> + int num_alternatives = MAX (num_alternatives_out, num_alternatives_in);
> +
> + /* Regarding hard register constraints ensure that each hard register is used
> + at most once over all inputs/outputs and each alternative. Keep track in
> + hardregs[0] which hard register is used via an asm register over all
> + inputs/outputs. hardregs[i] for i >= 2 describes which hard registers are
> + used for alternative i-2 over all inputs/outputs. hardregs[1] is a
> + reduction of all alternatives, i.e., hardregs[1] |= hardregs[i] for i >= 2
> + and describes whether a hard register is used in any alternative. This is
> + just a shortcut instead of recomputing the union over all alternatives;
> + possibly multiple times. */
> + auto_vec<HARD_REG_SET> hardregs (num_alternatives + 2);
> + for (int i = 0; i < num_alternatives + 2; ++i)
> + {
> + HARD_REG_SET hregset;
> + CLEAR_HARD_REG_SET (hregset);
> + hardregs.quick_push (hregset);
> + }
> +
> ret = GS_ALL_DONE;
> link_next = NULL_TREE;
> for (i = 0, link = ASM_OUTPUTS (expr); link; ++i, link = link_next)
> @@ -6998,8 +7066,8 @@ gimplify_asm_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
> if (constraint_len == 0)
> continue;
>
> - ok = parse_output_constraint (&constraint, i, 0, 0,
> - &allows_mem, &allows_reg, &is_inout);
> + ok = parse_output_constraint (&constraint, i, 0, 0, &allows_mem,
> + &allows_reg, &is_inout, &hardregs);
> if (!ok)
> {
> ret = GS_ERROR;
> @@ -7062,6 +7130,24 @@ gimplify_asm_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
> TREE_VALUE (link) = tem;
> tret = GS_OK;
> }
> + if (VAR_P (op) && DECL_HARD_REGISTER (op))
> + {
> + tree id = DECL_ASSEMBLER_NAME (op);
> + const char *asmspec = IDENTIFIER_POINTER (id) + 1;
> + int hardreg = decode_reg_name (asmspec);
> + if (hardreg >= 0)
> + {
> + if (TEST_HARD_REG_BIT (hardregs[0], hardreg)
> + || TEST_HARD_REG_BIT (hardregs[1], hardreg))
> + {
> + error ("multiple outputs to hard register: %s",
> + reg_names[hardreg]);
> + return GS_ERROR;
> + }
> + else
> + SET_HARD_REG_BIT (hardregs[0], hardreg);
> + }
> + }
> }
>
> vec_safe_push (outputs, link);
> @@ -7161,13 +7247,16 @@ gimplify_asm_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
> }
> }
>
> + for (unsigned int i = 0; i < hardregs.length (); ++i)
> + CLEAR_HARD_REG_SET (hardregs[i]);
> +
> link_next = NULL_TREE;
> for (link = ASM_INPUTS (expr); link; ++i, link = link_next)
> {
> link_next = TREE_CHAIN (link);
> constraint = TREE_STRING_POINTER (TREE_VALUE (TREE_PURPOSE (link)));
> - parse_input_constraint (&constraint, 0, 0, noutputs, 0,
> - oconstraints, &allows_mem, &allows_reg);
> + parse_input_constraint (&constraint, 0, 0, noutputs, 0, oconstraints,
> + &allows_mem, &allows_reg, &hardregs);
>
> /* If we can't make copies, we can only accept memory. */
> tree intype = TREE_TYPE (TREE_VALUE (link));
> @@ -7241,6 +7330,24 @@ gimplify_asm_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
> is_gimple_asm_val, fb_rvalue);
> if (tret == GS_ERROR)
> ret = tret;
> + tree inputv = TREE_VALUE (link);
> + if (VAR_P (inputv) && DECL_HARD_REGISTER (inputv))
> + {
> + tree id = DECL_ASSEMBLER_NAME (inputv);
> + const char *asmspec = IDENTIFIER_POINTER (id) + 1;
> + int hardreg = decode_reg_name (asmspec);
> + if (hardreg >= 0)
> + {
> + if (TEST_HARD_REG_BIT (hardregs[1], hardreg))
> + {
> + error ("multiple inputs to hard register: %s",
> + reg_names[hardreg]);
> + return GS_ERROR;
> + }
> + else
> + SET_HARD_REG_BIT (hardregs[0], hardreg);
> + }
> + }
> }
>
> TREE_CHAIN (link) = NULL_TREE;
> diff --git a/gcc/lra-constraints.cc b/gcc/lra-constraints.cc
> index e945a4da451..d81753eefaa 100644
> --- a/gcc/lra-constraints.cc
> +++ b/gcc/lra-constraints.cc
> @@ -114,6 +114,7 @@
> #include "target.h"
> #include "rtl.h"
> #include "tree.h"
> +#include "stmt.h"
> #include "predict.h"
> #include "df.h"
> #include "memmodel.h"
> @@ -2165,6 +2166,7 @@ process_alt_operands (int only_alternative)
> bool costly_p;
> enum reg_class cl;
> const HARD_REG_SET *cl_filter;
> + HARD_REG_SET hregset;
>
> /* Calculate some data common for all alternatives to speed up the
> function. */
> @@ -2536,6 +2538,21 @@ process_alt_operands (int only_alternative)
> cl_filter = nullptr;
> goto reg;
>
> + case '{':
> + {
> + /* Currently this form of constraint is only allowed in
> + asm statements which are verified during gimplify,
> + i.e., regno >= 0 holds for those. genoutput fails on
> + it. For future proofness assert it. */
> + int regno = parse_constraint_regname (p);
> + gcc_assert (regno >= 0);
> + cl = REGNO_REG_CLASS (regno);
> + CLEAR_HARD_REG_SET (hregset);
> + SET_HARD_REG_BIT (hregset, regno);
> + cl_filter = &hregset;
> + goto reg;
> + }
> +
> default:
> cn = lookup_constraint (p);
> switch (get_constraint_type (cn))
> diff --git a/gcc/recog.cc b/gcc/recog.cc
> index a6799e3f5e6..8a474cfb8a7 100644
> --- a/gcc/recog.cc
> +++ b/gcc/recog.cc
> @@ -25,6 +25,7 @@ along with GCC; see the file COPYING3. If not see
> #include "target.h"
> #include "rtl.h"
> #include "tree.h"
> +#include "stmt.h"
> #include "cfghooks.h"
> #include "df.h"
> #include "memmodel.h"
> @@ -2296,10 +2297,11 @@ asm_operand_ok (rtx op, const char *constraint, const char **constraints)
> switch (get_constraint_type (cn))
> {
> case CT_REGISTER:
> - if (!result
> - && reg_class_for_constraint (cn) != NO_REGS
> - && GET_MODE (op) != BLKmode
> - && register_operand (op, VOIDmode))
> + if ((!result
> + && reg_class_for_constraint (cn) != NO_REGS
> + && GET_MODE (op) != BLKmode
> + && register_operand (op, VOIDmode))
> + || constraint[0] == '{')
> result = 1;
> break;
>
> @@ -3231,6 +3233,10 @@ constrain_operands (int strict, alternative_mask alternatives)
> win = true;
> break;
>
> + case '{':
> + win = true;
> + break;
> +
> default:
> {
> enum constraint_num cn = lookup_constraint (p);
> diff --git a/gcc/stmt.cc b/gcc/stmt.cc
> index ae1527f0a19..1f20b09f90e 100644
> --- a/gcc/stmt.cc
> +++ b/gcc/stmt.cc
> @@ -39,6 +39,7 @@ along with GCC; see the file COPYING3. If not see
> #include "emit-rtl.h"
> #include "pretty-print.h"
> #include "diagnostic-core.h"
> +#include "output.h"
>
> #include "fold-const.h"
> #include "varasm.h"
> @@ -174,6 +175,32 @@ expand_label (tree label)
> maybe_set_first_label_num (label_r);
> }
> \f
> +/* Parse a hard register constraint and return its number or -1 in case of an
> + error. BEGIN should point to a string of the form "{regname}". For the
> + sake of simplicity assume that a register name is not longer than 31
> + characters, if not error out. */
> +
> +int
> +parse_constraint_regname (const char *begin)
> +{
> + if (*begin != '{')
> + return -1;
> + ++begin;
> + const char *end = begin;
> + while (*end != '}' && *end != '\0')
> + ++end;
> + if (*end != '}' || end == begin)
> + return -1;
> + ptrdiff_t len = end - begin;
> + if (len >= 31)
> + return -1;
> + char regname[32];
> + memcpy (regname, begin, len);
> + regname[len] = '\0';
> + int regno = decode_reg_name (regname);
> + return regno;
> +}
> +
> /* Parse the output constraint pointed to by *CONSTRAINT_P. It is the
> OPERAND_NUMth output operand, indexed from zero. There are NINPUTS
> inputs and NOUTPUTS outputs to this extended-asm. Upon return,
> @@ -190,7 +217,8 @@ expand_label (tree label)
> bool
> parse_output_constraint (const char **constraint_p, int operand_num,
> int ninputs, int noutputs, bool *allows_mem,
> - bool *allows_reg, bool *is_inout)
> + bool *allows_reg, bool *is_inout,
> + vec<HARD_REG_SET> *hardregs)
> {
> const char *constraint = *constraint_p;
> const char *p;
> @@ -244,6 +272,8 @@ parse_output_constraint (const char **constraint_p, int operand_num,
> constraint = *constraint_p;
> }
>
> + unsigned int alternative = 2;
> +
> /* Loop through the constraint string. */
> for (p = constraint + 1; *p; )
> {
> @@ -268,7 +298,11 @@ parse_output_constraint (const char **constraint_p, int operand_num,
> case 'E': case 'F': case 'G': case 'H':
> case 's': case 'i': case 'n':
> case 'I': case 'J': case 'K': case 'L': case 'M':
> - case 'N': case 'O': case 'P': case ',':
> + case 'N': case 'O': case 'P':
> + break;
> +
> + case ',':
> + ++alternative;
> break;
>
> case '0': case '1': case '2': case '3': case '4':
> @@ -289,6 +323,33 @@ parse_output_constraint (const char **constraint_p, int operand_num,
> *allows_mem = true;
> break;
>
> + case '{':
> + {
> + int regno = parse_constraint_regname (p);
> + if (regno < 0)
> + {
> + error ("invalid output constraint: %s", p);
> + return false;
> + }
> + if (hardregs)
> + {
> + if (TEST_HARD_REG_BIT ((*hardregs)[0], regno)
> + || TEST_HARD_REG_BIT ((*hardregs)[alternative], regno))
> + {
> + error ("multiple outputs to hard register: %s",
> + reg_names[regno]);
> + return false;
> + }
> + else
> + {
> + SET_HARD_REG_BIT ((*hardregs)[1], regno);
> + SET_HARD_REG_BIT ((*hardregs)[alternative], regno);
> + }
> + }
> + *allows_reg = true;
> + break;
> + }
> +
> default:
> if (!ISALPHA (*p))
> break;
> @@ -317,7 +378,8 @@ bool
> parse_input_constraint (const char **constraint_p, int input_num,
> int ninputs, int noutputs, int ninout,
> const char * const * constraints,
> - bool *allows_mem, bool *allows_reg)
> + bool *allows_mem, bool *allows_reg,
> + vec<HARD_REG_SET> *hardregs)
> {
> const char *constraint = *constraint_p;
> const char *orig_constraint = constraint;
> @@ -332,6 +394,8 @@ parse_input_constraint (const char **constraint_p, int input_num,
>
> /* Make sure constraint has neither `=', `+', nor '&'. */
>
> + unsigned int alternative = 2;
> +
> for (j = 0; j < c_len; j += CONSTRAINT_LEN (constraint[j], constraint+j))
> switch (constraint[j])
> {
> @@ -358,7 +422,11 @@ parse_input_constraint (const char **constraint_p, int input_num,
> case 'E': case 'F': case 'G': case 'H':
> case 's': case 'i': case 'n':
> case 'I': case 'J': case 'K': case 'L': case 'M':
> - case 'N': case 'O': case 'P': case ',':
> + case 'N': case 'O': case 'P':
> + break;
> +
> + case ',':
> + ++alternative;
> break;
>
> /* Whether or not a numeric constraint allows a register is
> @@ -408,6 +476,32 @@ parse_input_constraint (const char **constraint_p, int input_num,
> *allows_mem = true;
> break;
>
> + case '{':
> + {
> + int regno = parse_constraint_regname (constraint + j);
> + if (regno < 0)
> + {
> + error ("invalid input constraint: %s", constraint + j);
> + return false;
> + }
> + if (hardregs)
> + {
> + if (TEST_HARD_REG_BIT ((*hardregs)[0], regno)
> + || TEST_HARD_REG_BIT ((*hardregs)[alternative], regno))
> + {
> + error ("multiple inputs to hard register: %s",
> + reg_names[regno]);
> + }
> + else
> + {
> + SET_HARD_REG_BIT ((*hardregs)[1], regno);
> + SET_HARD_REG_BIT ((*hardregs)[alternative], regno);
> + }
> + }
> + *allows_reg = true;
> + break;
> + }
> +
> default:
> if (! ISALPHA (constraint[j]))
> {
> diff --git a/gcc/stmt.h b/gcc/stmt.h
> index a2caae7121b..05889ff3798 100644
> --- a/gcc/stmt.h
> +++ b/gcc/stmt.h
> @@ -20,11 +20,17 @@ along with GCC; see the file COPYING3. If not see
> #ifndef GCC_STMT_H
> #define GCC_STMT_H
>
> +#include "target.h"
> +#include "hard-reg-set.h"
> +
> extern void expand_label (tree);
> extern bool parse_output_constraint (const char **, int, int, int,
> - bool *, bool *, bool *);
> + bool *, bool *, bool *,
> + vec<HARD_REG_SET> * = nullptr);
> extern bool parse_input_constraint (const char **, int, int, int, int,
> - const char * const *, bool *, bool *);
> + const char * const *, bool *, bool *,
> + vec<HARD_REG_SET> * = nullptr);
> +extern int parse_constraint_regname (const char *);
> extern tree resolve_asm_operand_names (tree, tree, tree, tree);
> #ifdef HARD_CONST
> /* Silly ifdef to avoid having all includers depend on hard-reg-set.h. */
> diff --git a/gcc/testsuite/gcc.target/s390/asm-hard-reg-1.c b/gcc/testsuite/gcc.target/s390/asm-hard-reg-1.c
> new file mode 100644
> index 00000000000..53895d98663
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/s390/asm-hard-reg-1.c
> @@ -0,0 +1,103 @@
> +/* { dg-do compile { target { lp64 } } } */
> +/* { dg-options "-O2 -march=z13" } */
> +/* { dg-final { check-function-bodies "**" "" "" } } */
> +
> +/*
> +** test_in_1:
> +** foo %r2
> +** br %r14
> +*/
> +
> +int
> +test_in_1 (int x)
> +{
> + asm ("foo %0" :: "{r2}" (x));
> + return x;
> +}
> +
> +/*
> +** test_in_2:
> +** lgr (%r[0-9]+),%r2
> +** lhi %r2,42
> +** foo %r2
> +** lgr %r2,\1
> +** br %r14
> +*/
> +
> +int
> +test_in_2 (int x)
> +{
> + asm ("foo %0" :: "{r2}" (42));
> + return x;
> +}
> +
> +/*
> +** test_in_3:
> +** stmg %r12,%r15,96\(%r15\)
> +** lay %r15,-160\(%r15\)
> +** lgr (%r[0-9]+),%r2
> +** ahi %r2,1
> +** lgfr %r2,%r2
> +** brasl %r14,foo@PLT
> +** lr %r3,%r2
> +** lr %r2,\1
> +** foo %r3,%r2
> +** lgr %r2,\1
> +** lmg %r12,%r15,256\(%r15\)
> +** br %r14
> +*/
> +
> +extern int foo (int);
> +
> +int
> +test_in_3 (int x)
> +{
> + asm ("foo %0,%1\n" :: "{r3}" (foo (x + 1)), "{r2}" (x));
> + return x;
> +}
> +
> +/*
> +** test_out_1:
> +** foo %r3
> +** lgfr %r2,%r3
> +** br %r14
> +*/
> +
> +int
> +test_out_1 (void)
> +{
> + int x;
> + asm ("foo %0" : "={r3}" (x));
> + return x;
> +}
> +
> +/*
> +** test_out_2:
> +** lgr (%r[0-9]+),%r2
> +** foo %r2
> +** ark (%r[0-9]+),\1,%r2
> +** lgfr %r2,\2
> +** br %r14
> +*/
> +
> +int
> +test_out_2 (int x)
> +{
> + int y;
> + asm ("foo %0" : "={r2}" (y));
> + return x + y;
> +}
> +
> +/*
> +** test_inout_1:
> +** foo %r2
> +** lgfr %r2,%r2
> +** br %r14
> +*/
> +
> +int
> +test_inout_1 (int x)
> +{
> + asm ("foo %0" : "+{r2}" (x));
> + return x;
> +}
> diff --git a/gcc/testsuite/gcc.target/s390/asm-hard-reg-2.c b/gcc/testsuite/gcc.target/s390/asm-hard-reg-2.c
> new file mode 100644
> index 00000000000..9f3c221b937
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/s390/asm-hard-reg-2.c
> @@ -0,0 +1,29 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +/* { dg-final { check-function-bodies "**" "" "" } } */
> +
> +/*
> +** test_1:
> +** lr %r5,%r2
> +** foo %r5,%r3
> +** br %r14
> +*/
> +
> +void
> +test_1 (int x, int *y)
> +{
> + asm ("foo %0,%1" :: "m{r4},{r5}" (x), "m,r" (y));
> +}
> +
> +/*
> +** test_2:
> +** lr %r4,%r2
> +** foo %r4,0\(%r3\)
> +** br %r14
> +*/
> +
> +void
> +test_2 (int x, int *y)
> +{
> + asm ("foo %0,%1" :: "m{r4},{r5}" (x), "m,r" (*y));
> +}
> diff --git a/gcc/testsuite/gcc.target/s390/asm-hard-reg-3.c b/gcc/testsuite/gcc.target/s390/asm-hard-reg-3.c
> new file mode 100644
> index 00000000000..0edcdd3cfde
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/s390/asm-hard-reg-3.c
> @@ -0,0 +1,24 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +void
> +test (void)
> +{
> + int x, y;
> + register int r4 asm ("r4") = 0;
> +
> + asm ("" :: "{}" (42)); /* { dg-error "invalid input constraint: \{\}" } */
> + asm ("" :: "{r4" (42)); /* { dg-error "invalid input constraint: \{r4" } */
> + asm ("" :: "{r17}" (42)); /* { dg-error "invalid input constraint: \{r17\}" } */
> +
> + asm ("" :: "r" (r4), "{r4}" (42)); /* { dg-error "multiple inputs to hard register: %r4" } */
> + asm ("" :: "{r4}" (42), "r" (r4)); /* { dg-error "multiple inputs to hard register: %r4" } */
> + asm ("" :: "{r4}" (42), "{r4}" (42)); /* { dg-error "multiple inputs to hard register: %r4" } */
> + asm ("" :: "{r2},{r4}" (42), "{r4},{r3}" (42));
> + asm ("" :: "{r2},{r4}" (42), "{r3},{r4}" (42)); /* { dg-error "multiple inputs to hard register: %r4" } */
> + asm ("" :: "{r4},{r2}" (42), "{r4},{r3}" (42)); /* { dg-error "multiple inputs to hard register: %r4" } */
> + asm ("" :: "{r3}{r4}" (42), "{r4}" (42)); /* { dg-error "multiple inputs to hard register: %r4" } */
> + asm ("" : "+{r4}" (x), "={r4}" (y)); /* { dg-error "multiple outputs to hard register: %r4" } */
> + asm ("" : "={r4}" (y) : "{r4}" (42), "0" (42)); /* { dg-error "multiple inputs to hard register: %r4" } */
> + asm ("" : "+{r4}" (x) : "{r4}" (y)); /* { dg-error "multiple inputs to hard register: %r4" } */
> +}
> diff --git a/gcc/testsuite/lib/scanasm.exp b/gcc/testsuite/lib/scanasm.exp
> index 6cf9997240d..d09372096a5 100644
> --- a/gcc/testsuite/lib/scanasm.exp
> +++ b/gcc/testsuite/lib/scanasm.exp
> @@ -896,6 +896,10 @@ proc configure_check-function-bodies { config } {
> set up_config(fluff) {^\s*(?://)}
> } elseif { [istarget *-*-darwin*] } {
> set up_config(fluff) {^\s*(?:\.|//|@)|^L[0-9ABCESV]}
> + } elseif { [istarget s390*-*-*] } {
> + # Additionally to the defaults skip lines beginning with a # resulting
> + # from inline asm.
> + set up_config(fluff) {^\s*(?:\.|//|@|$|#)}
> } else {
> # Skip lines beginning with labels ('.L[...]:') or other directives
> # ('.align', '.cfi_startproc', '.quad [...]', '.text', etc.), '//' or
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Hard register asm constraint
2024-06-27 7:45 ` Georg-Johann Lay
@ 2024-06-27 8:51 ` Stefan Schulze Frielinghaus
2024-06-28 9:46 ` Georg-Johann Lay
0 siblings, 1 reply; 14+ messages in thread
From: Stefan Schulze Frielinghaus @ 2024-06-27 8:51 UTC (permalink / raw)
To: Georg-Johann Lay; +Cc: gcc-patches
On Thu, Jun 27, 2024 at 09:45:32AM +0200, Georg-Johann Lay wrote:
>
>
> Am 24.05.24 um 11:13 Am 25.06.24 um 16:03 schrieb Paul Koning:
> >
> >
> > > On Jun 24, 2024, at 1:50 AM, Stefan Schulze Frielinghaus <stefansf@linux.ibm.com> wrote:
> > >
> > > Ping.
> > >
> > > On Mon, Jun 10, 2024 at 07:19:19AM +0200, Stefan Schulze Frielinghaus wrote:
> > > > Ping.
> > > >
> > > > On Fri, May 24, 2024 at 11:13:12AM +0200, Stefan Schulze Frielinghaus wrote:
> > > > > This implements hard register constraints for inline asm. A hard register
> > > > > constraint is of the form {regname} where regname is any valid register. This
> > > > > basically renders register asm superfluous. For example, the snippet
> > > > >
> > > > > int test (int x, int y)
> > > > > {
> > > > > register int r4 asm ("r4") = x;
> > > > > register int r5 asm ("r5") = y;
> > > > > unsigned int copy = y;
> > > > > asm ("foo %0,%1,%2" : "+d" (r4) : "d" (r5), "d" (copy));
> > > > > return r4;
> > > > > }
> > > > >
> > > > > could be rewritten into
> > > > >
> > > > > int test (int x, int y)
> > > > > {
> > > > > asm ("foo %0,%1,%2" : "+{r4}" (x) : "{r5}" (y), "d" (y));
> > > > > return x;
> > > > > }
> >
> > I like this idea but I'm wondering: regular constraints specify what sort of value is needed, for example an int vs. a short int vs. a float. The notation you've shown doesn't seem to have that aspect.
> >
> > The other comment is that I didn't see documentation updates to reflect this new feature.
> >
> > paul
> >
> Stefan Schulze Frielinghaus:
> > This implements hard register constraints for inline asm. A hard register
> > constraint is of the form {regname} where regname is any valid register. This
> > basically renders register asm superfluous. For example, the snippet
> >
> > int test (int x, int y)
> > {
> > register int r4 asm ("r4") = x;
> > register int r5 asm ("r5") = y;
> > unsigned int copy = y;
> > asm ("foo %0,%1,%2" : "+d" (r4) : "d" (r5), "d" (copy));
> > return r4;
> > }
> >
> > could be rewritten into
> >
> > int test (int x, int y)
> > {
> > asm ("foo %0,%1,%2" : "+{r4}" (x) : "{r5}" (y), "d" (y));
> > return x;
> > }
>
> Hi, can this also be used in machine descriptions?
>
> It would make some insn handling much simpler, for example in
> the avr backend.
>
> That backend has insns that represent assembly sequences in libgcc
> which have a smaller register footprint than plain calls. However
> this requires that such insns have explicit description of which regs
> go in and out.
>
> The current solution uses hard regs, which works, but a proper
> implementation would use register constraints. I tries that a while
> ago, and register constraints lead to a code bloat even in places that
> don't use these constraints due to the zillions of new register classes
> like R22_1, R22;2, R22_4, R20_1, R20_2, R20_4 etc. that were required.
>
> Your approach would allow to use hard register constraints in insns,
> and so far the only problem is to determine how much hard regs are
> used by the constraint. The gen tools that generates cc code from md
> would use the operand's machine mode to infer the number of hard regs.
I have this on my todo list but ignored it for the very first draft. At
the moment this already fails because genoutput cannot parse the
constraint format.
In my "alpha draft" I implemented this feature by emitting moves to hard
registers during expand. This had the limitation that I couldn't
support multiple alternatives in combination with hard-register
constraints. I'm still not sure whether this is a feature we really
want or whether it should be rather denied. Anyhow, with this kind of
implementation I doubt that this would be feasible for machine
descriptions. I moved on with my current draft where the constraint
manifests during register allocation. This also allows multiple
alternatives. I think one of the (major?) advantages of doing it this
way is that operands are kept in pseudos which means they are
automagically saved/restored over function boundaries and what not. Or
in other words, the register constraint manifests at the asm boundary
which is probably what users expect and should be less error prone
(again just thinking of implicit code which gets injected as e.g. by
sanitizers introducing calls etc.).
So long story short, I would like to look into this but currently it
doesn't work. I'm also not sure to which extend this could be used.
However, once I have some more time I will have a look at the avr
backend for examples.
Cheers,
Stefan
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Hard register asm constraint
2024-06-27 8:51 ` Stefan Schulze Frielinghaus
@ 2024-06-28 9:46 ` Georg-Johann Lay
2024-06-28 12:20 ` Stefan Schulze Frielinghaus
0 siblings, 1 reply; 14+ messages in thread
From: Georg-Johann Lay @ 2024-06-28 9:46 UTC (permalink / raw)
To: Stefan Schulze Frielinghaus; +Cc: gcc-patches
Am 27.06.24 um 10:51 schrieb Stefan Schulze Frielinghaus:
> On Thu, Jun 27, 2024 at 09:45:32AM +0200, Georg-Johann Lay wrote:
>> Am 24.05.24 um 11:13 Am 25.06.24 um 16:03 schrieb Paul Koning:
>>>> On Jun 24, 2024, at 1:50 AM, Stefan Schulze Frielinghaus <stefansf@linux.ibm.com> wrote:
>>>> On Mon, Jun 10, 2024 at 07:19:19AM +0200, Stefan Schulze Frielinghaus wrote:
>>>>> On Fri, May 24, 2024 at 11:13:12AM +0200, Stefan Schulze Frielinghaus wrote:
>>>>>> This implements hard register constraints for inline asm. A hard register
>>>>>> constraint is of the form {regname} where regname is any valid register. This
>>>>>> basically renders register asm superfluous. For example, the snippet
>>>>>>
>>>>>> int test (int x, int y)
>>>>>> {
>>>>>> register int r4 asm ("r4") = x;
>>>>>> register int r5 asm ("r5") = y;
>>>>>> unsigned int copy = y;
>>>>>> asm ("foo %0,%1,%2" : "+d" (r4) : "d" (r5), "d" (copy));
>>>>>> return r4;
>>>>>> }
>>>>>>
>>>>>> could be rewritten into
>>>>>>
>>>>>> int test (int x, int y)
>>>>>> {
>>>>>> asm ("foo %0,%1,%2" : "+{r4}" (x) : "{r5}" (y), "d" (y));
>>>>>> return x;
>>>>>> }
>>>
>>> I like this idea but I'm wondering: regular constraints specify what sort of value is needed, for example an int vs. a short int vs. a float. The notation you've shown doesn't seem to have that aspect.
>>>
>>> The other comment is that I didn't see documentation updates to reflect this new feature.
>>>
>>> paul
>>>
>> Stefan Schulze Frielinghaus:
>>> This implements hard register constraints for inline asm. A hard register
>>> constraint is of the form {regname} where regname is any valid register. This
>>> basically renders register asm superfluous. For example, the snippet
>>>
>>> int test (int x, int y)
>>> {
>>> register int r4 asm ("r4") = x;
>>> register int r5 asm ("r5") = y;
>>> unsigned int copy = y;
>>> asm ("foo %0,%1,%2" : "+d" (r4) : "d" (r5), "d" (copy));
>>> return r4;
>>> }
>>>
>>> could be rewritten into
>>>
>>> int test (int x, int y)
>>> {
>>> asm ("foo %0,%1,%2" : "+{r4}" (x) : "{r5}" (y), "d" (y));
>>> return x;
>>> }
>>
>> Hi, can this also be used in machine descriptions?
>>
>> It would make some insn handling much simpler, for example in
>> the avr backend.
>>
>> That backend has insns that represent assembly sequences in libgcc
>> which have a smaller register footprint than plain calls. However
>> this requires that such insns have explicit description of which regs
>> go in and out.
>>
>> The current solution uses hard regs, which works, but a proper
>> implementation would use register constraints. I tries that a while
>> ago, and register constraints lead to a code bloat even in places that
>> don't use these constraints due to the zillions of new register classes
>> like R22_1, R22;2, R22_4, R20_1, R20_2, R20_4 etc. that were required.
>>
>> Your approach would allow to use hard register constraints in insns,
>> and so far the only problem is to determine how much hard regs are
>> used by the constraint. The gen tools that generates cc code from md
>> would use the operand's machine mode to infer the number of hard regs.
>
> I have this on my todo list but ignored it for the very first draft. At
> the moment this already fails because genoutput cannot parse the
> constraint format.
>
> In my "alpha draft" I implemented this feature by emitting moves to hard
> registers during expand. This had the limitation that I couldn't
One problem is that you cannot just introduce hard registers at that
time because a hard reg may live across the sequence, see for example
avr.cc::avr_emit3_fix_outputs() and avr_fix_operands().
> support multiple alternatives in combination with hard-register
> constraints. I'm still not sure whether this is a feature we really
> want or whether it should be rather denied. Anyhow, with this kind of
> implementation I doubt that this would be feasible for machine
> descriptions. I moved on with my current draft where the constraint
> manifests during register allocation. This also allows multiple
> alternatives. I think one of the (major?) advantages of doing it this
> way is that operands are kept in pseudos which means they are
> automagically saved/restored over function boundaries and what not. Or
> in other words, the register constraint manifests at the asm boundary
> which is probably what users expect and should be less error prone
As far as I know, a local register variable is only supposed to be
loaded to the specified register when the variable is used as an
operand to some inline asm. Only in such asm statements, the
variable will live in the specified register. So "surviving" a
function call is not even a problem to solve with the current local
regvar semantic?
> (again just thinking of implicit code which gets injected as e.g. by
> sanitizers introducing calls etc.).
>
> So long story short, I would like to look into this but currently it
> doesn't work. I'm also not sure to which extend this could be used.
> However, once I have some more time I will have a look at the avr
> backend for examples.
>
> Cheers,
> Stefan
Great. When you have any questions about the avr backend, don't
hesitate to ask me.
Cheers,
Johann
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Hard register asm constraint
2024-06-28 9:46 ` Georg-Johann Lay
@ 2024-06-28 12:20 ` Stefan Schulze Frielinghaus
0 siblings, 0 replies; 14+ messages in thread
From: Stefan Schulze Frielinghaus @ 2024-06-28 12:20 UTC (permalink / raw)
To: Georg-Johann Lay; +Cc: gcc-patches
On Fri, Jun 28, 2024 at 11:46:08AM +0200, Georg-Johann Lay wrote:
> Am 27.06.24 um 10:51 schrieb Stefan Schulze Frielinghaus:
> > On Thu, Jun 27, 2024 at 09:45:32AM +0200, Georg-Johann Lay wrote:
> > > Am 24.05.24 um 11:13 Am 25.06.24 um 16:03 schrieb Paul Koning:
> > > > > On Jun 24, 2024, at 1:50 AM, Stefan Schulze Frielinghaus <stefansf@linux.ibm.com> wrote:
> > > > > On Mon, Jun 10, 2024 at 07:19:19AM +0200, Stefan Schulze Frielinghaus wrote:
> > > > > > On Fri, May 24, 2024 at 11:13:12AM +0200, Stefan Schulze Frielinghaus wrote:
> > > > > > > This implements hard register constraints for inline asm. A hard register
> > > > > > > constraint is of the form {regname} where regname is any valid register. This
> > > > > > > basically renders register asm superfluous. For example, the snippet
> > > > > > >
> > > > > > > int test (int x, int y)
> > > > > > > {
> > > > > > > register int r4 asm ("r4") = x;
> > > > > > > register int r5 asm ("r5") = y;
> > > > > > > unsigned int copy = y;
> > > > > > > asm ("foo %0,%1,%2" : "+d" (r4) : "d" (r5), "d" (copy));
> > > > > > > return r4;
> > > > > > > }
> > > > > > >
> > > > > > > could be rewritten into
> > > > > > >
> > > > > > > int test (int x, int y)
> > > > > > > {
> > > > > > > asm ("foo %0,%1,%2" : "+{r4}" (x) : "{r5}" (y), "d" (y));
> > > > > > > return x;
> > > > > > > }
> > > >
> > > > I like this idea but I'm wondering: regular constraints specify what sort of value is needed, for example an int vs. a short int vs. a float. The notation you've shown doesn't seem to have that aspect.
> > > >
> > > > The other comment is that I didn't see documentation updates to reflect this new feature.
> > > >
> > > > paul
> > > >
> > > Stefan Schulze Frielinghaus:
> > > > This implements hard register constraints for inline asm. A hard register
> > > > constraint is of the form {regname} where regname is any valid register. This
> > > > basically renders register asm superfluous. For example, the snippet
> > > >
> > > > int test (int x, int y)
> > > > {
> > > > register int r4 asm ("r4") = x;
> > > > register int r5 asm ("r5") = y;
> > > > unsigned int copy = y;
> > > > asm ("foo %0,%1,%2" : "+d" (r4) : "d" (r5), "d" (copy));
> > > > return r4;
> > > > }
> > > >
> > > > could be rewritten into
> > > >
> > > > int test (int x, int y)
> > > > {
> > > > asm ("foo %0,%1,%2" : "+{r4}" (x) : "{r5}" (y), "d" (y));
> > > > return x;
> > > > }
> > >
> > > Hi, can this also be used in machine descriptions?
> > >
> > > It would make some insn handling much simpler, for example in
> > > the avr backend.
> > >
> > > That backend has insns that represent assembly sequences in libgcc
> > > which have a smaller register footprint than plain calls. However
> > > this requires that such insns have explicit description of which regs
> > > go in and out.
> > >
> > > The current solution uses hard regs, which works, but a proper
> > > implementation would use register constraints. I tries that a while
> > > ago, and register constraints lead to a code bloat even in places that
> > > don't use these constraints due to the zillions of new register classes
> > > like R22_1, R22;2, R22_4, R20_1, R20_2, R20_4 etc. that were required.
> > >
> > > Your approach would allow to use hard register constraints in insns,
> > > and so far the only problem is to determine how much hard regs are
> > > used by the constraint. The gen tools that generates cc code from md
> > > would use the operand's machine mode to infer the number of hard regs.
> >
> > I have this on my todo list but ignored it for the very first draft. At
> > the moment this already fails because genoutput cannot parse the
> > constraint format.
> >
> > In my "alpha draft" I implemented this feature by emitting moves to hard
> > registers during expand. This had the limitation that I couldn't
>
> One problem is that you cannot just introduce hard registers at that
> time because a hard reg may live across the sequence, see for example
> avr.cc::avr_emit3_fix_outputs() and avr_fix_operands().
Yea I was fearing this. I did some testing on x86_64 and s390 including
explicit function calls, sanitizers etc. but of course this was not
complete which is why I think that the current draft is more robust.
>
> > support multiple alternatives in combination with hard-register
> > constraints. I'm still not sure whether this is a feature we really
> > want or whether it should be rather denied. Anyhow, with this kind of
> > implementation I doubt that this would be feasible for machine
> > descriptions. I moved on with my current draft where the constraint
> > manifests during register allocation. This also allows multiple
> > alternatives. I think one of the (major?) advantages of doing it this
> > way is that operands are kept in pseudos which means they are
> > automagically saved/restored over function boundaries and what not. Or
> > in other words, the register constraint manifests at the asm boundary
> > which is probably what users expect and should be less error prone
>
> As far as I know, a local register variable is only supposed to be
> loaded to the specified register when the variable is used as an
> operand to some inline asm. Only in such asm statements, the
> variable will live in the specified register. So "surviving" a
> function call is not even a problem to solve with the current local
> regvar semantic?
I was hoping for this, too, and AFAICS Clang behaves that way but for
GCC this isn't the case which is also documented here
https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html
Warning: In the above example, be aware that a register (for example
r0) can be call-clobbered by subsequent code, including function calls
and library calls for arithmetic operators on other variables (for
example the initialization of p2). In this case, use temporary
variables for expressions between the register assignments:
This may also lead to subtle bugs like
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100908
Cheers,
Stefan
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2024-06-29 2:50 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-24 9:13 [PATCH] Hard register asm constraint Stefan Schulze Frielinghaus
2024-06-10 5:19 ` Stefan Schulze Frielinghaus
2024-06-24 5:50 ` Stefan Schulze Frielinghaus
2024-06-25 14:03 ` Paul Koning
2024-06-25 14:35 ` Maciej W. Rozycki
2024-06-25 16:04 ` Stefan Schulze Frielinghaus
2024-06-25 17:02 ` Paul Koning
2024-06-26 12:54 ` Stefan Schulze Frielinghaus
2024-06-26 15:10 ` Paul Koning
2024-06-27 6:25 ` Stefan Schulze Frielinghaus
2024-06-27 7:45 ` Georg-Johann Lay
2024-06-27 8:51 ` Stefan Schulze Frielinghaus
2024-06-28 9:46 ` Georg-Johann Lay
2024-06-28 12:20 ` Stefan Schulze Frielinghaus
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).