| Message ID | alpine.LNX.2.00.1108111516440.7304@linmac |
|---|---|
| State | New |
| Headers | show |
On 11.08.2011, at 13:24, malc wrote: > On Thu, 11 Aug 2011, Alexander Graf wrote: > >> While compiling current HEAD on a ppc64 box, I was confronted with the >> following compile errors: >> >> tcg/optimize.c: In function ?tcg_constant_folding?: >> tcg/optimize.c:546: error: ?INDEX_op_not_i32? undeclared (first use in this function) >> tcg/optimize.c:546: error: (Each undeclared identifier is reported only once >> tcg/optimize.c:546: error: for each function it appears in.) >> tcg/optimize.c:546: error: ?INDEX_op_not_i64? undeclared (first use in this function) >> tcg/optimize.c:573: error: ?INDEX_op_ext32u_i64? undeclared (first use in this function) >> make[1]: *** [tcg/optimize.o] Error 1 >> >> Obviously, the optimize.c tries to use TCG opcode constants that are optional >> and thus not defined in some targets, such as ppc64. >> >> This patch guards them with the proper #ifdefs, so compilation works again. >> >> Signed-off-by: Alexander Graf <agraf@suse.de> >> --- >> tcg/optimize.c | 8 +++++++- >> 1 files changed, 7 insertions(+), 1 deletions(-) >> >> diff --git a/tcg/optimize.c b/tcg/optimize.c >> index 7eb5eb1..7b4954c 100644 >> --- a/tcg/optimize.c >> +++ b/tcg/optimize.c >> @@ -543,7 +543,11 @@ static TCGArg *tcg_constant_folding(TCGContext *s, uint16_t *tcg_opc_ptr, >> gen_args += 2; >> args += 2; >> break; >> +#if ((TCG_TARGET_REG_BITS == 32) && defined(TCG_TARGET_HAS_not_i32)) || \ >> + ((TCG_TARGET_REG_BITS == 64) && defined(TCG_TARGET_HAS_not_i32) && \ >> + defined(TCG_TARGET_HAS_not_i64)) >> CASE_OP_32_64(not): >> +#endif >> #ifdef TCG_TARGET_HAS_ext8s_i32 >> case INDEX_op_ext8s_i32: >> #endif >> @@ -568,8 +572,10 @@ static TCGArg *tcg_constant_folding(TCGContext *s, uint16_t *tcg_opc_ptr, >> #ifdef TCG_TARGET_HAS_ext16u_i64 >> case INDEX_op_ext16u_i64: >> #endif >> -#if TCG_TARGET_REG_BITS == 64 >> +#if (TCG_TARGET_REG_BITS == 64) && defined(TCG_TARGET_HAS_ext32s_i64) >> case INDEX_op_ext32s_i64: >> +#endif >> +#if (TCG_TARGET_REG_BITS == 64) && defined(TCG_TARGET_HAS_ext32u_i64) >> case INDEX_op_ext32u_i64: >> #endif >> if (temps[args[1]].state == TCG_TEMP_CONST) { >> > > Or (not even compile tested): Sure, but then the next target would pop up with optionals not implemented. The real alternative would be to make these ops non-optional :) Alex > > diff --git a/tcg/ppc64/tcg-target.c b/tcg/ppc64/tcg-target.c > index 02a6cb2..2669403 100644 > --- a/tcg/ppc64/tcg-target.c > +++ b/tcg/ppc64/tcg-target.c > @@ -1449,6 +1449,11 @@ static void tcg_out_op (TCGContext *s, TCGOpcode opc, const TCGArg *args, > tcg_out32 (s, NEG | RT (args[0]) | RA (args[1])); > break; > > + case INDEX_op_neg_i32: > + case INDEX_op_not_i64: > + tcg_out32 (s, NOR | SAB (args[1], args[0], args[1])); > + break; > + > case INDEX_op_add_i64: > if (const_args[2]) > ppc_addi64 (s, args[0], args[1], args[2]); > @@ -1553,6 +1558,10 @@ static void tcg_out_op (TCGContext *s, TCGOpcode opc, const TCGArg *args, > tcg_out32 (s, c | RS (args[1]) | RA (args[0])); > break; > > + case INDEX_op_ext32u_i64: > + tcg_out_rld (s, RLDICR, ret, ret, 0, 32); > + break; > + > case INDEX_op_setcond_i32: > tcg_out_setcond (s, TCG_TYPE_I32, args[3], args[0], args[1], args[2], > const_args[2]); > diff --git a/tcg/ppc64/tcg-target.h b/tcg/ppc64/tcg-target.h > index 8a6db11..71a5fe9 100644 > --- a/tcg/ppc64/tcg-target.h > +++ b/tcg/ppc64/tcg-target.h > @@ -76,7 +76,7 @@ enum { > /* #define TCG_TARGET_HAS_ext16u_i32 */ > /* #define TCG_TARGET_HAS_bswap16_i32 */ > /* #define TCG_TARGET_HAS_bswap32_i32 */ > -/* #define TCG_TARGET_HAS_not_i32 */ > +#define TCG_TARGET_HAS_not_i32 > #define TCG_TARGET_HAS_neg_i32 > /* #define TCG_TARGET_HAS_andc_i32 */ > /* #define TCG_TARGET_HAS_orc_i32 */ > @@ -91,11 +91,11 @@ enum { > #define TCG_TARGET_HAS_ext32s_i64 > /* #define TCG_TARGET_HAS_ext8u_i64 */ > /* #define TCG_TARGET_HAS_ext16u_i64 */ > -/* #define TCG_TARGET_HAS_ext32u_i64 */ > +#define TCG_TARGET_HAS_ext32u_i64 > /* #define TCG_TARGET_HAS_bswap16_i64 */ > /* #define TCG_TARGET_HAS_bswap32_i64 */ > /* #define TCG_TARGET_HAS_bswap64_i64 */ > -/* #define TCG_TARGET_HAS_not_i64 */ > +#define TCG_TARGET_HAS_not_i64 > #define TCG_TARGET_HAS_neg_i64 > /* #define TCG_TARGET_HAS_andc_i64 */ > /* #define TCG_TARGET_HAS_orc_i64 */ > > -- > mailto:av1474@comtv.ru
On 08/11/2011 02:40 PM, Alexander Graf wrote: > On 11.08.2011, at 13:24, malc wrote: > > > On Thu, 11 Aug 2011, Alexander Graf wrote: > > > >> While compiling current HEAD on a ppc64 box, I was confronted with the > >> following compile errors: > >> > >> tcg/optimize.c: In function ?tcg_constant_folding?: > >> tcg/optimize.c:546: error: ?INDEX_op_not_i32? undeclared (first use in this function) > >> tcg/optimize.c:546: error: (Each undeclared identifier is reported only once > >> tcg/optimize.c:546: error: for each function it appears in.) > >> tcg/optimize.c:546: error: ?INDEX_op_not_i64? undeclared (first use in this function) > >> tcg/optimize.c:573: error: ?INDEX_op_ext32u_i64? undeclared (first use in this function) > >> make[1]: *** [tcg/optimize.o] Error 1 > >> > >> Obviously, the optimize.c tries to use TCG opcode constants that are optional > >> and thus not defined in some targets, such as ppc64. > >> > >> This patch guards them with the proper #ifdefs, so compilation works again. > >> > >> Signed-off-by: Alexander Graf<agraf@suse.de> > >> --- > >> tcg/optimize.c | 8 +++++++- > >> 1 files changed, 7 insertions(+), 1 deletions(-) > >> > >> diff --git a/tcg/optimize.c b/tcg/optimize.c > >> index 7eb5eb1..7b4954c 100644 > >> --- a/tcg/optimize.c > >> +++ b/tcg/optimize.c > >> @@ -543,7 +543,11 @@ static TCGArg *tcg_constant_folding(TCGContext *s, uint16_t *tcg_opc_ptr, > >> gen_args += 2; > >> args += 2; > >> break; > >> +#if ((TCG_TARGET_REG_BITS == 32)&& defined(TCG_TARGET_HAS_not_i32)) || \ > >> + ((TCG_TARGET_REG_BITS == 64)&& defined(TCG_TARGET_HAS_not_i32)&& \ > >> + defined(TCG_TARGET_HAS_not_i64)) > >> CASE_OP_32_64(not): > >> +#endif > >> #ifdef TCG_TARGET_HAS_ext8s_i32 > >> case INDEX_op_ext8s_i32: > >> #endif > >> @@ -568,8 +572,10 @@ static TCGArg *tcg_constant_folding(TCGContext *s, uint16_t *tcg_opc_ptr, > >> #ifdef TCG_TARGET_HAS_ext16u_i64 > >> case INDEX_op_ext16u_i64: > >> #endif > >> -#if TCG_TARGET_REG_BITS == 64 > >> +#if (TCG_TARGET_REG_BITS == 64)&& defined(TCG_TARGET_HAS_ext32s_i64) > >> case INDEX_op_ext32s_i64: > >> +#endif > >> +#if (TCG_TARGET_REG_BITS == 64)&& defined(TCG_TARGET_HAS_ext32u_i64) > >> case INDEX_op_ext32u_i64: > >> #endif > >> if (temps[args[1]].state == TCG_TEMP_CONST) { > >> > > > > Or (not even compile tested): > > Sure, but then the next target would pop up with optionals not implemented. The real alternative would be to make these ops non-optional :) > Or to have automatic generation of the optionals based on the primitives, if the optionals are not present.
On 11.08.2011, at 14:28, Avi Kivity wrote: > On 08/11/2011 02:40 PM, Alexander Graf wrote: >> On 11.08.2011, at 13:24, malc wrote: >> >> > On Thu, 11 Aug 2011, Alexander Graf wrote: >> > >> >> While compiling current HEAD on a ppc64 box, I was confronted with the >> >> following compile errors: >> >> >> >> tcg/optimize.c: In function ?tcg_constant_folding?: >> >> tcg/optimize.c:546: error: ?INDEX_op_not_i32? undeclared (first use in this function) >> >> tcg/optimize.c:546: error: (Each undeclared identifier is reported only once >> >> tcg/optimize.c:546: error: for each function it appears in.) >> >> tcg/optimize.c:546: error: ?INDEX_op_not_i64? undeclared (first use in this function) >> >> tcg/optimize.c:573: error: ?INDEX_op_ext32u_i64? undeclared (first use in this function) >> >> make[1]: *** [tcg/optimize.o] Error 1 >> >> >> >> Obviously, the optimize.c tries to use TCG opcode constants that are optional >> >> and thus not defined in some targets, such as ppc64. >> >> >> >> This patch guards them with the proper #ifdefs, so compilation works again. >> >> >> >> Signed-off-by: Alexander Graf<agraf@suse.de> >> >> --- >> >> tcg/optimize.c | 8 +++++++- >> >> 1 files changed, 7 insertions(+), 1 deletions(-) >> >> >> >> diff --git a/tcg/optimize.c b/tcg/optimize.c >> >> index 7eb5eb1..7b4954c 100644 >> >> --- a/tcg/optimize.c >> >> +++ b/tcg/optimize.c >> >> @@ -543,7 +543,11 @@ static TCGArg *tcg_constant_folding(TCGContext *s, uint16_t *tcg_opc_ptr, >> >> gen_args += 2; >> >> args += 2; >> >> break; >> >> +#if ((TCG_TARGET_REG_BITS == 32)&& defined(TCG_TARGET_HAS_not_i32)) || \ >> >> + ((TCG_TARGET_REG_BITS == 64)&& defined(TCG_TARGET_HAS_not_i32)&& \ >> >> + defined(TCG_TARGET_HAS_not_i64)) >> >> CASE_OP_32_64(not): >> >> +#endif >> >> #ifdef TCG_TARGET_HAS_ext8s_i32 >> >> case INDEX_op_ext8s_i32: >> >> #endif >> >> @@ -568,8 +572,10 @@ static TCGArg *tcg_constant_folding(TCGContext *s, uint16_t *tcg_opc_ptr, >> >> #ifdef TCG_TARGET_HAS_ext16u_i64 >> >> case INDEX_op_ext16u_i64: >> >> #endif >> >> -#if TCG_TARGET_REG_BITS == 64 >> >> +#if (TCG_TARGET_REG_BITS == 64)&& defined(TCG_TARGET_HAS_ext32s_i64) >> >> case INDEX_op_ext32s_i64: >> >> +#endif >> >> +#if (TCG_TARGET_REG_BITS == 64)&& defined(TCG_TARGET_HAS_ext32u_i64) >> >> case INDEX_op_ext32u_i64: >> >> #endif >> >> if (temps[args[1]].state == TCG_TEMP_CONST) { >> >> >> > >> > Or (not even compile tested): >> >> Sure, but then the next target would pop up with optionals not implemented. The real alternative would be to make these ops non-optional :) >> > > Or to have automatic generation of the optionals based on the primitives, if the optionals are not present. That's what's happening in the background already, no? The line is towards tcg users though, not it tcg internal code. Alex
On 08/11/2011 03:36 PM, Alexander Graf wrote: > > > > Or to have automatic generation of the optionals based on the primitives, if the optionals are not present. > > That's what's happening in the background already, no? The line is towards tcg users though, not it tcg internal code. > Yes, and it doesn't make sense to optimize through synthetic instructions.
On Thu, Aug 11, 2011 at 12:58 PM, Avi Kivity <avi@redhat.com> wrote: > On 08/11/2011 03:36 PM, Alexander Graf wrote: >> >> > >> > Or to have automatic generation of the optionals based on the >> > primitives, if the optionals are not present. >> >> That's what's happening in the background already, no? The line is towards >> tcg users though, not it tcg internal code. >> > > Yes, and it doesn't make sense to optimize through synthetic instructions. I'd just create all INDEX_op_* enums and adjust TCG targets etc. to call tcg_abort() in the default case.
On 08/11/2011 09:38 AM, Blue Swirl wrote: > On Thu, Aug 11, 2011 at 12:58 PM, Avi Kivity <avi@redhat.com> wrote: >> On 08/11/2011 03:36 PM, Alexander Graf wrote: >>> >>>> >>>> Or to have automatic generation of the optionals based on the >>>> primitives, if the optionals are not present. >>> >>> That's what's happening in the background already, no? The line is towards >>> tcg users though, not it tcg internal code. >>> >> >> Yes, and it doesn't make sense to optimize through synthetic instructions. > > I'd just create all INDEX_op_* enums and adjust TCG targets etc. to > call tcg_abort() in the default case. > Seconded. We can still expand the optional enums exactly as we do now, but make sure that the enum is always present. That'll clean up a *lot* of ifdefs throughout tcg/*. r~
On 11.08.2011, at 13:14, Richard Henderson <rth@twiddle.net> wrote: > On 08/11/2011 09:38 AM, Blue Swirl wrote: >> On Thu, Aug 11, 2011 at 12:58 PM, Avi Kivity <avi@redhat.com> wrote: >>> On 08/11/2011 03:36 PM, Alexander Graf wrote: >>>> >>>>> >>>>> Or to have automatic generation of the optionals based on the >>>>> primitives, if the optionals are not present. >>>> >>>> That's what's happening in the background already, no? The line is towards >>>> tcg users though, not it tcg internal code. >>>> >>> >>> Yes, and it doesn't make sense to optimize through synthetic instructions. >> >> I'd just create all INDEX_op_* enums and adjust TCG targets etc. to >> call tcg_abort() in the default case. >> > > Seconded. We can still expand the optional enums exactly as we do now, > but make sure that the enum is always present. That'll clean up a *lot* > of ifdefs throughout tcg/*. I agree, but that is a longterm fix. For now, I'd just like to be able to compile again :). So I'd propose to take this patch in for now and then get rid of the op ifdefs later. Alex >
diff --git a/tcg/ppc64/tcg-target.c b/tcg/ppc64/tcg-target.c index 02a6cb2..2669403 100644 --- a/tcg/ppc64/tcg-target.c +++ b/tcg/ppc64/tcg-target.c @@ -1449,6 +1449,11 @@ static void tcg_out_op (TCGContext *s, TCGOpcode opc, const TCGArg *args, tcg_out32 (s, NEG | RT (args[0]) | RA (args[1])); break; + case INDEX_op_neg_i32: + case INDEX_op_not_i64: + tcg_out32 (s, NOR | SAB (args[1], args[0], args[1])); + break; + case INDEX_op_add_i64: if (const_args[2]) ppc_addi64 (s, args[0], args[1], args[2]); @@ -1553,6 +1558,10 @@ static void tcg_out_op (TCGContext *s, TCGOpcode opc, const TCGArg *args, tcg_out32 (s, c | RS (args[1]) | RA (args[0])); break; + case INDEX_op_ext32u_i64: + tcg_out_rld (s, RLDICR, ret, ret, 0, 32); + break; + case INDEX_op_setcond_i32: tcg_out_setcond (s, TCG_TYPE_I32, args[3], args[0], args[1], args[2], const_args[2]); diff --git a/tcg/ppc64/tcg-target.h b/tcg/ppc64/tcg-target.h index 8a6db11..71a5fe9 100644 --- a/tcg/ppc64/tcg-target.h +++ b/tcg/ppc64/tcg-target.h @@ -76,7 +76,7 @@ enum { /* #define TCG_TARGET_HAS_ext16u_i32 */ /* #define TCG_TARGET_HAS_bswap16_i32 */ /* #define TCG_TARGET_HAS_bswap32_i32 */ -/* #define TCG_TARGET_HAS_not_i32 */ +#define TCG_TARGET_HAS_not_i32 #define TCG_TARGET_HAS_neg_i32 /* #define TCG_TARGET_HAS_andc_i32 */ /* #define TCG_TARGET_HAS_orc_i32 */ @@ -91,11 +91,11 @@ enum { #define TCG_TARGET_HAS_ext32s_i64 /* #define TCG_TARGET_HAS_ext8u_i64 */ /* #define TCG_TARGET_HAS_ext16u_i64 */ -/* #define TCG_TARGET_HAS_ext32u_i64 */ +#define TCG_TARGET_HAS_ext32u_i64 /* #define TCG_TARGET_HAS_bswap16_i64 */ /* #define TCG_TARGET_HAS_bswap32_i64 */ /* #define TCG_TARGET_HAS_bswap64_i64 */ -/* #define TCG_TARGET_HAS_not_i64 */ +#define TCG_TARGET_HAS_not_i64 #define TCG_TARGET_HAS_neg_i64 /* #define TCG_TARGET_HAS_andc_i64 */ /* #define TCG_TARGET_HAS_orc_i64 */