[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 13/22] tcg/i386: Split out constraint sets to tcg-target-c
From: |
Peter Maydell |
Subject: |
Re: [PATCH v2 13/22] tcg/i386: Split out constraint sets to tcg-target-con-set.h |
Date: |
Tue, 19 Jan 2021 15:27:37 +0000 |
On Fri, 15 Jan 2021 at 21:20, Richard Henderson
<richard.henderson@linaro.org> 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 <richard.henderson@linaro.org>
> ---
> 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 <peter.maydell@linaro.org>
thanks
-- PMM