Re: [PATCH v2 13/22] tcg/i386: Split out constraint sets to tcg-target-con-set.h
On Tue, 19 Jan 2021 at 23:48, Richard Henderson wrote: > > On 1/19/21 5:27 AM, Peter Maydell wrote: > > On Fri, 15 Jan 2021 at 21:20, Richard Henderson > > wrote: > >> > >> This exports the constraint sets from tcg_target_op_def to > >> a place we will be able to manipulate more in future. > >> > >> Signed-off-by: Richard Henderson > >> --- > >> tcg/i386/tcg-target-con-set.h | 54 ++ > >> tcg/i386/tcg-target.h | 1 + > >> tcg/tcg.c | 122 + > >> tcg/i386/tcg-target.c.inc | 194 -- > >> 4 files changed, 244 insertions(+), 127 deletions(-) > >> create mode 100644 tcg/i386/tcg-target-con-set.h > > > >> +#define C_O2_I1(O1, O2, I1) C_PFX3(c_o2_i1_, O1, O2, I1), > >> +#define C_O2_I2(O1, O2, I1, I2) C_PFX4(c_o2_i2_, O1, O2, I1, I2), > >> +#define C_O2_I3(O1, O2, I1, I2, I3) C_PFX5(c_o2_i3_, O1, O2, I1, I2, > >> I3), > >> +#define C_O2_I4(O1, O2, I1, I2, I3, I4) \ > >> +C_PFX6(c_o2_i4_, O1, O2, I1, I2, I3, I4), > > > > Personally this is the kind of code where I would follow > > CODING_STYLE.rst's suggestion of "If wrapping the line at 80 > > columns is obviously less readable and more awkward, prefer not > > to wrap it; better to have an 85 character line than one which > > is awkwardly wrapped.". The parallelism between the lines > > is much easier to see without the linebreak. > > The maximum within this section is column 92. > Still ok with that? Yes. -- PMM
Re: [PATCH v2 13/22] tcg/i386: Split out constraint sets to tcg-target-con-set.h
On 1/19/21 5:27 AM, Peter Maydell wrote:
> On Fri, 15 Jan 2021 at 21:20, Richard Henderson
> wrote:
>>
>> This exports the constraint sets from tcg_target_op_def to
>> a place we will be able to manipulate more in future.
>>
>> Signed-off-by: Richard Henderson
>> ---
>> tcg/i386/tcg-target-con-set.h | 54 ++
>> tcg/i386/tcg-target.h | 1 +
>> tcg/tcg.c | 122 +
>> tcg/i386/tcg-target.c.inc | 194 --
>> 4 files changed, 244 insertions(+), 127 deletions(-)
>> create mode 100644 tcg/i386/tcg-target-con-set.h
>
>> +#define C_O2_I1(O1, O2, I1) C_PFX3(c_o2_i1_, O1, O2, I1),
>> +#define C_O2_I2(O1, O2, I1, I2) C_PFX4(c_o2_i2_, O1, O2, I1, I2),
>> +#define C_O2_I3(O1, O2, I1, I2, I3) C_PFX5(c_o2_i3_, O1, O2, I1, I2,
>> I3),
>> +#define C_O2_I4(O1, O2, I1, I2, I3, I4) \
>> +C_PFX6(c_o2_i4_, O1, O2, I1, I2, I3, I4),
>
> Personally this is the kind of code where I would follow
> CODING_STYLE.rst's suggestion of "If wrapping the line at 80
> columns is obviously less readable and more awkward, prefer not
> to wrap it; better to have an 85 character line than one which
> is awkwardly wrapped.". The parallelism between the lines
> is much easier to see without the linebreak.
The maximum within this section is column 92.
Still ok with that?
> I know the macro magic should make it impossible, but maybe
> we should have an assert that we definitely have a valid
> array index here ?
Ok.
>> case INDEX_op_andc_i32:
>> case INDEX_op_andc_i64:
>> -{
>> -static const TCGTargetOpDef andc
>> -= { .args_ct_str = { "r", "r", "rI" } };
>> -return &andc;
>> -}
>> -break;
>> +return C_O1_I2(r, 0, rI);
>
> Old constraint was r r rI; new one is r 0 rI ?
Oops, good catch. And of course the error would have worked in testing.
r~
Re: [PATCH v2 13/22] tcg/i386: Split out constraint sets to tcg-target-con-set.h
On Fri, 15 Jan 2021 at 21:20, Richard Henderson
wrote:
>
> This exports the constraint sets from tcg_target_op_def to
> a place we will be able to manipulate more in future.
>
> Signed-off-by: Richard Henderson
> ---
> tcg/i386/tcg-target-con-set.h | 54 ++
> tcg/i386/tcg-target.h | 1 +
> tcg/tcg.c | 122 +
> tcg/i386/tcg-target.c.inc | 194 --
> 4 files changed, 244 insertions(+), 127 deletions(-)
> create mode 100644 tcg/i386/tcg-target-con-set.h
> +#define C_O2_I1(O1, O2, I1) C_PFX3(c_o2_i1_, O1, O2, I1),
> +#define C_O2_I2(O1, O2, I1, I2) C_PFX4(c_o2_i2_, O1, O2, I1, I2),
> +#define C_O2_I3(O1, O2, I1, I2, I3) C_PFX5(c_o2_i3_, O1, O2, I1, I2, I3),
> +#define C_O2_I4(O1, O2, I1, I2, I3, I4) \
> +C_PFX6(c_o2_i4_, O1, O2, I1, I2, I3, I4),
Personally this is the kind of code where I would follow
CODING_STYLE.rst's suggestion of "If wrapping the line at 80
columns is obviously less readable and more awkward, prefer not
to wrap it; better to have an 85 character line than one which
is awkwardly wrapped.". The parallelism between the lines
is much easier to see without the linebreak.
> @@ -2418,9 +2536,13 @@ static void process_op_defs(TCGContext *s)
> continue;
> }
>
> +#ifdef TCG_TARGET_CON_SET_H
> +tdefs = &constraint_sets[tcg_target_op_def(op)];
I know the macro magic should make it impossible, but maybe
we should have an assert that we definitely have a valid
array index here ?
> case INDEX_op_andc_i32:
> case INDEX_op_andc_i64:
> -{
> -static const TCGTargetOpDef andc
> -= { .args_ct_str = { "r", "r", "rI" } };
> -return &andc;
> -}
> -break;
> +return C_O1_I2(r, 0, rI);
Old constraint was r r rI; new one is r 0 rI ?
Otherwise
Reviewed-by: Peter Maydell
thanks
-- PMM
[PATCH v2 13/22] tcg/i386: Split out constraint sets to tcg-target-con-set.h
This exports the constraint sets from tcg_target_op_def to
a place we will be able to manipulate more in future.
Signed-off-by: Richard Henderson
---
tcg/i386/tcg-target-con-set.h | 54 ++
tcg/i386/tcg-target.h | 1 +
tcg/tcg.c | 122 +
tcg/i386/tcg-target.c.inc | 194 --
4 files changed, 244 insertions(+), 127 deletions(-)
create mode 100644 tcg/i386/tcg-target-con-set.h
diff --git a/tcg/i386/tcg-target-con-set.h b/tcg/i386/tcg-target-con-set.h
new file mode 100644
index 00..66123ab193
--- /dev/null
+++ b/tcg/i386/tcg-target-con-set.h
@@ -0,0 +1,54 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Define i386 target-specific constraint sets.
+ * Copyright (c) 2021 Linaro
+ */
+
+/*
+ * C_On_Im(...) defines a constraint set with outputs and inputs.
+ * Each operand should be a sequence of constraint letters as defined by
+ * tcg-target-con-str.h; the constraint combination is inclusive or.
+ *
+ * C_N1_Im(...) defines a constraint set with 1 output and inputs,
+ * except that the output must use a new register.
+ */
+C_O0_I1(r)
+C_O0_I2(L, L)
+C_O0_I2(qi, r)
+C_O0_I2(re, r)
+C_O0_I2(ri, r)
+C_O0_I2(r, re)
+C_O0_I2(s, L)
+C_O0_I2(x, r)
+C_O0_I3(L, L, L)
+C_O0_I3(s, L, L)
+C_O0_I4(L, L, L, L)
+C_O0_I4(r, r, ri, ri)
+C_O1_I1(r, 0)
+C_O1_I1(r, L)
+C_O1_I1(r, q)
+C_O1_I1(r, r)
+C_O1_I1(x, r)
+C_O1_I1(x, x)
+C_O1_I2(Q, 0, Q)
+C_O1_I2(q, r, re)
+C_O1_I2(r, 0, ci)
+C_O1_I2(r, 0, r)
+C_O1_I2(r, 0, re)
+C_O1_I2(r, 0, reZ)
+C_O1_I2(r, 0, ri)
+C_O1_I2(r, 0, rI)
+C_O1_I2(r, L, L)
+C_O1_I2(r, r, re)
+C_O1_I2(r, r, ri)
+C_O1_I2(x, x, x)
+C_N1_I2(r, r, r)
+C_N1_I2(r, r, rW)
+C_O1_I3(x, x, x, x)
+C_O1_I4(r, r, re, r, 0)
+C_O1_I4(r, r, r, ri, ri)
+C_O2_I1(r, r, L)
+C_O2_I2(a, d, a, r)
+C_O2_I2(r, r, L, L)
+C_O2_I3(a, d, 0, 1, r)
+C_O2_I4(r, r, 0, 1, re, re)
diff --git a/tcg/i386/tcg-target.h b/tcg/i386/tcg-target.h
index b693d3692d..48a6f2a336 100644
--- a/tcg/i386/tcg-target.h
+++ b/tcg/i386/tcg-target.h
@@ -235,5 +235,6 @@ static inline void tb_target_set_jmp_target(uintptr_t
tc_ptr, uintptr_t jmp_rx,
#define TCG_TARGET_NEED_LDST_LABELS
#endif
#define TCG_TARGET_NEED_POOL_LABELS
+#define TCG_TARGET_CON_SET_H
#endif
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 7b4d0b3f69..36fdeef10f 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -69,7 +69,9 @@
/* Forward declarations for functions declared in tcg-target.c.inc and
used here. */
static void tcg_target_init(TCGContext *s);
+#ifndef TCG_TARGET_CON_SET_H
static const TCGTargetOpDef *tcg_target_op_def(TCGOpcode);
+#endif
static void tcg_target_qemu_prologue(TCGContext *s);
static bool patch_reloc(tcg_insn_unit *code_ptr, int type,
intptr_t value, intptr_t addend);
@@ -347,6 +349,122 @@ static void set_jmp_reset_offset(TCGContext *s, int which)
s->tb_jmp_reset_offset[which] = tcg_current_code_size(s);
}
+#ifdef TCG_TARGET_CON_SET_H
+#define C_PFX1(P, A)P##A
+#define C_PFX2(P, A, B) P##A##_##B
+#define C_PFX3(P, A, B, C) P##A##_##B##_##C
+#define C_PFX4(P, A, B, C, D) P##A##_##B##_##C##_##D
+#define C_PFX5(P, A, B, C, D, E)P##A##_##B##_##C##_##D##_##E
+#define C_PFX6(P, A, B, C, D, E, F) P##A##_##B##_##C##_##D##_##E##_##F
+
+/* Define an enumeration for the various combinations. */
+
+#define C_O0_I1(I1) C_PFX1(c_o0_i1_, I1),
+#define C_O0_I2(I1, I2) C_PFX2(c_o0_i2_, I1, I2),
+#define C_O0_I3(I1, I2, I3) C_PFX3(c_o0_i3_, I1, I2, I3),
+#define C_O0_I4(I1, I2, I3, I4) C_PFX4(c_o0_i4_, I1, I2, I3, I4),
+
+#define C_O1_I1(O1, I1) C_PFX2(c_o1_i1_, O1, I1),
+#define C_O1_I2(O1, I1, I2) C_PFX3(c_o1_i2_, O1, I1, I2),
+#define C_O1_I3(O1, I1, I2, I3) C_PFX4(c_o1_i3_, O1, I1, I2, I3),
+#define C_O1_I4(O1, I1, I2, I3, I4) C_PFX5(c_o1_i4_, O1, I1, I2, I3, I4),
+
+#define C_N1_I2(O1, I1, I2) C_PFX3(c_n1_i2_, O1, I1, I2),
+
+#define C_O2_I1(O1, O2, I1) C_PFX3(c_o2_i1_, O1, O2, I1),
+#define C_O2_I2(O1, O2, I1, I2) C_PFX4(c_o2_i2_, O1, O2, I1, I2),
+#define C_O2_I3(O1, O2, I1, I2, I3) C_PFX5(c_o2_i3_, O1, O2, I1, I2, I3),
+#define C_O2_I4(O1, O2, I1, I2, I3, I4) \
+C_PFX6(c_o2_i4_, O1, O2, I1, I2, I3, I4),
+
+typedef enum {
+#include "tcg-target-con-set.h"
+} TCGConstraintSetIndex;
+
+static TCGConstraintSetIndex tcg_target_op_def(TCGOpcode);
+
+#undef C_O0_I1
+#undef C_O0_I2
+#undef C_O0_I3
+#undef C_O0_I4
+#undef C_O1_I1
+#undef C_O1_I2
+#undef C_O1_I3
+#undef C_O1_I4
+#undef C_N1_I2
+#undef C_O2_I1
+#undef C_O2_I2
+#undef C_O2_I3
+#undef C_O2_I4
+
+/* Put all of the constraint sets into an array, indexed by the enum. */
+
+#define C_O0_I1(I1) { .args_ct_str = { #I1 } },
+#define C_O0_I2(I1, I2) { .args_ct_str = { #I1, #I2 } },
+#define C_O0_I3(I1, I2, I3) { .args_ct_str = { #I1, #I2, #I3 } },
+#de
