| Message ID | 1318010451-24790-1-git-send-email-Stefan.Weil@Kath-Kirche-Ladenburg.de |
|---|---|
| State | Rejected |
| Headers | show |
On 10/07/2011 11:00 AM, Stefan Weil wrote:
> +#define TCGReg int /* TODO: Remove this line when tcg-target.c uses TCGReg. */
Nack. This is too ugly to live.
How hard can it be to simply change the prototype in each
of the backend files instead? Yes, full conversion to
TCGReg is desirable, but this is not C++ -- integers and
enums are interchangable without casts.
r~
Am 07.10.2011 20:13, schrieb Richard Henderson: > On 10/07/2011 11:00 AM, Stefan Weil wrote: >> +#define TCGReg int /* TODO: Remove this line when tcg-target.c uses >> TCGReg. */ > > Nack. This is too ugly to live. > > How hard can it be to simply change the prototype in each > of the backend files instead? Yes, full conversion to > TCGReg is desirable, but this is not C++ -- integers and > enums are interchangable without casts. > > r~ The goal of my patch is to get code which compiles again on all hosts with minimum risk and which is a base for further improvements. I cannot run build tests for all possible hosts, and even changing 4 prototypes for each host is a risk when it is not tested. Yes, you can review the changes, some developers can try builds, but that takes a lot of time. As soon as my patch is applied, it is possible to add more TCGReg usage to tcg.c, s390/tcg-target.c and all other TCG targets in independent patches. I'm just preparing patches for tcg.c and i386/tcg-target.*. But let me repeat: these new patches need a good review which will take some time. The bug fix should be applied soon. Maybe you can change your mind and send an ack. - Stefan
On Fri, Oct 7, 2011 at 6:26 PM, Stefan Weil <sw@weilnetz.de> wrote: > Am 07.10.2011 20:13, schrieb Richard Henderson: >> >> On 10/07/2011 11:00 AM, Stefan Weil wrote: >>> >>> +#define TCGReg int /* TODO: Remove this line when tcg-target.c uses >>> TCGReg. */ >> >> Nack. This is too ugly to live. Fully agree. >> >> How hard can it be to simply change the prototype in each >> of the backend files instead? Yes, full conversion to >> TCGReg is desirable, but this is not C++ -- integers and >> enums are interchangable without casts. >> >> r~ > > The goal of my patch is to get code which compiles again > on all hosts with minimum risk and which is a base for further > improvements. > > I cannot run build tests for all possible hosts, and even > changing 4 prototypes for each host is a risk when it is not > tested. Yes, you can review the changes, some developers can > try builds, but that takes a lot of time. The only problem with changing ints to enums is that enums may be unsigned depending on the compiler, otherwise it is safe. I don't think any arithmetic is performed with the register numbers. > As soon as my patch is applied, it is possible to add > more TCGReg usage to tcg.c, s390/tcg-target.c and all > other TCG targets in independent patches. > > I'm just preparing patches for tcg.c and i386/tcg-target.*. > > But let me repeat: these new patches need a good review > which will take some time. The bug fix should be applied soon. > Maybe you can change your mind and send an ack. > > - Stefan > >
diff --git a/tcg/arm/tcg-target.h b/tcg/arm/tcg-target.h index 33afd97..047b6d0 100644 --- a/tcg/arm/tcg-target.h +++ b/tcg/arm/tcg-target.h @@ -27,7 +27,7 @@ #undef TCG_TARGET_WORDS_BIGENDIAN #undef TCG_TARGET_STACK_GROWSUP -enum { +typedef enum { TCG_REG_R0 = 0, TCG_REG_R1, TCG_REG_R2, @@ -44,7 +44,9 @@ enum { TCG_REG_R13, TCG_REG_R14, TCG_REG_PC, -}; +} TCGReg; + +#define TCGReg int /* TODO: Remove this line when tcg-target.c uses TCGReg. */ #define TCG_TARGET_NB_REGS 16 diff --git a/tcg/hppa/tcg-target.h b/tcg/hppa/tcg-target.h index ec9a7bf..8c220ae 100644 --- a/tcg/hppa/tcg-target.h +++ b/tcg/hppa/tcg-target.h @@ -32,7 +32,7 @@ #define TCG_TARGET_NB_REGS 32 -enum { +typedef enum { TCG_REG_R0 = 0, TCG_REG_R1, TCG_REG_RP, @@ -65,7 +65,9 @@ enum { TCG_REG_RET1, TCG_REG_SP, TCG_REG_R31, -}; +} TCGReg; + +#define TCGReg int /* TODO: Remove this line when tcg-target.c uses TCGReg. */ #define TCG_CT_CONST_0 0x0100 #define TCG_CT_CONST_S5 0x0200 diff --git a/tcg/i386/tcg-target.h b/tcg/i386/tcg-target.h index b9c9d4e..0466216 100644 --- a/tcg/i386/tcg-target.h +++ b/tcg/i386/tcg-target.h @@ -36,7 +36,7 @@ # define TCG_TARGET_NB_REGS 8 #endif -enum { +typedef enum { TCG_REG_EAX = 0, TCG_REG_ECX, TCG_REG_EDX, @@ -64,7 +64,9 @@ enum { TCG_REG_RBP = TCG_REG_EBP, TCG_REG_RSI = TCG_REG_ESI, TCG_REG_RDI = TCG_REG_EDI, -}; +} TCGReg; + +#define TCGReg int /* TODO: Remove this line when tcg-target.c uses TCGReg. */ #define TCG_CT_CONST_S32 0x100 #define TCG_CT_CONST_U32 0x200 diff --git a/tcg/ia64/tcg-target.h b/tcg/ia64/tcg-target.h index 578cf29..fc83559 100644 --- a/tcg/ia64/tcg-target.h +++ b/tcg/ia64/tcg-target.h @@ -26,7 +26,8 @@ /* We only map the first 64 registers */ #define TCG_TARGET_NB_REGS 64 -enum { + +typedef enum { TCG_REG_R0 = 0, TCG_REG_R1, TCG_REG_R2, @@ -91,7 +92,9 @@ enum { TCG_REG_R61, TCG_REG_R62, TCG_REG_R63, -}; +} TCGReg; + +#define TCGReg int /* TODO: Remove this line when tcg-target.c uses TCGReg. */ #define TCG_CT_CONST_ZERO 0x100 #define TCG_CT_CONST_S22 0x200 diff --git a/tcg/mips/tcg-target.h b/tcg/mips/tcg-target.h index e2a2571..1b01206 100644 --- a/tcg/mips/tcg-target.h +++ b/tcg/mips/tcg-target.h @@ -31,7 +31,7 @@ #define TCG_TARGET_NB_REGS 32 -enum { +typedef enum { TCG_REG_ZERO = 0, TCG_REG_AT, TCG_REG_V0, @@ -64,7 +64,9 @@ enum { TCG_REG_SP, TCG_REG_FP, TCG_REG_RA, -}; +} TCGReg; + +#define TCGReg int /* TODO: Remove this line when tcg-target.c uses TCGReg. */ #define TCG_CT_CONST_ZERO 0x100 #define TCG_CT_CONST_U16 0x200 diff --git a/tcg/ppc/tcg-target.h b/tcg/ppc/tcg-target.h index 5c2d612..d737809 100644 --- a/tcg/ppc/tcg-target.h +++ b/tcg/ppc/tcg-target.h @@ -26,7 +26,7 @@ #define TCG_TARGET_WORDS_BIGENDIAN #define TCG_TARGET_NB_REGS 32 -enum { +typedef enum { TCG_REG_R0 = 0, TCG_REG_R1, TCG_REG_R2, @@ -59,7 +59,9 @@ enum { TCG_REG_R29, TCG_REG_R30, TCG_REG_R31 -}; +} TCGReg; + +#define TCGReg int /* TODO: Remove this line when tcg-target.c uses TCGReg. */ /* used for function call generation */ #define TCG_REG_CALL_STACK TCG_REG_R1 diff --git a/tcg/ppc64/tcg-target.h b/tcg/ppc64/tcg-target.h index 8d1fb73..a37259e 100644 --- a/tcg/ppc64/tcg-target.h +++ b/tcg/ppc64/tcg-target.h @@ -26,7 +26,7 @@ #define TCG_TARGET_WORDS_BIGENDIAN #define TCG_TARGET_NB_REGS 32 -enum { +typedef enum { TCG_REG_R0 = 0, TCG_REG_R1, TCG_REG_R2, @@ -59,7 +59,9 @@ enum { TCG_REG_R29, TCG_REG_R30, TCG_REG_R31 -}; +} TCGReg; + +#define TCGReg int /* TODO: Remove this line when tcg-target.c uses TCGReg. */ /* used for function call generation */ #define TCG_REG_CALL_STACK TCG_REG_R1 diff --git a/tcg/sparc/tcg-target.h b/tcg/sparc/tcg-target.h index 1464ef4..527b32f 100644 --- a/tcg/sparc/tcg-target.h +++ b/tcg/sparc/tcg-target.h @@ -27,7 +27,7 @@ #define TCG_TARGET_NB_REGS 32 -enum { +typedef enum { TCG_REG_G0 = 0, TCG_REG_G1, TCG_REG_G2, @@ -60,7 +60,9 @@ enum { TCG_REG_I5, TCG_REG_I6, TCG_REG_I7, -}; +} TCGReg; + +#define TCGReg int /* TODO: Remove this line when tcg-target.c uses TCGReg. */ #define TCG_CT_CONST_S11 0x100 #define TCG_CT_CONST_S13 0x200 diff --git a/tcg/tcg.c b/tcg/tcg.c index 30f3aef..a887d15 100644 --- a/tcg/tcg.c +++ b/tcg/tcg.c @@ -71,14 +71,14 @@ static void patch_reloc(uint8_t *code_ptr, int type, /* Forward declarations for functions declared and used in tcg-target.c. */ static int target_parse_constraint(TCGArgConstraint *ct, const char **pct_str); -static void tcg_out_ld(TCGContext *s, TCGType type, int ret, int arg1, +static void tcg_out_ld(TCGContext *s, TCGType type, TCGReg ret, TCGReg arg1, tcg_target_long arg2); -static void tcg_out_mov(TCGContext *s, TCGType type, int ret, int arg); +static void tcg_out_mov(TCGContext *s, TCGType type, TCGReg ret, TCGReg arg); static void tcg_out_movi(TCGContext *s, TCGType type, - int ret, tcg_target_long arg); + TCGReg ret, tcg_target_long arg); static void tcg_out_op(TCGContext *s, TCGOpcode opc, const TCGArg *args, const int *const_args); -static void tcg_out_st(TCGContext *s, TCGType type, int arg, int arg1, +static void tcg_out_st(TCGContext *s, TCGType type, TCGReg arg, TCGReg arg1, tcg_target_long arg2); static int tcg_target_const_match(tcg_target_long val, const TCGArgConstraint *arg_ct);
s390 already uses TCGReg - this patch defines TCGReg for all other TCG targets. The targets still don't use TCGReg, therefore a temporary define maps 'TCGReg' to 'int' for all unfinished targets. This define allows usage of TCGReg in tcg.c thus fixing some forward declarations which had broken compilation on s390 hosts since commit c0ad3001bf12292b137b05e1c4643f31c6b0a727. Signed-off-by: Stefan Weil <Stefan.Weil@Kath-Kirche-Ladenburg.de> --- tcg/arm/tcg-target.h | 6 ++++-- tcg/hppa/tcg-target.h | 6 ++++-- tcg/i386/tcg-target.h | 6 ++++-- tcg/ia64/tcg-target.h | 7 +++++-- tcg/mips/tcg-target.h | 6 ++++-- tcg/ppc/tcg-target.h | 6 ++++-- tcg/ppc64/tcg-target.h | 6 ++++-- tcg/sparc/tcg-target.h | 6 ++++-- tcg/tcg.c | 8 ++++---- 9 files changed, 37 insertions(+), 20 deletions(-)