Message ID | 1304420620-8409-1-git-send-email-hdegoede@redhat.com |
---|---|
State | New |
Headers | show |
Hi Hans, On Tue, May 03, 2011 at 01:03:40PM +0200, Hans de Goede wrote: > diff --git a/target-i386/kvm.c b/target-i386/kvm.c > index a13599d..e9e8d54 100644 > --- a/target-i386/kvm.c > +++ b/target-i386/kvm.c > @@ -950,7 +950,7 @@ static int kvm_get_xsave(CPUState *env) > @@ -966,7 +966,7 @@ static int kvm_get_xsave(CPUState *env) > cwd = (uint16_t)xsave->region[0]; > swd = (uint16_t)(xsave->region[0] >> 16); > twd = (uint16_t)xsave->region[1]; > - fop = (uint16_t)(xsave->region[1] >> 16); > + /* fop = (uint16_t)(xsave->region[1] >> 16); */ Wouldn't it be better to drop this line? > env->fpstt = (swd >> 11) & 7; > env->fpus = swd; > env->fpuc = cwd; > diff --git a/tcg/tcg.c b/tcg/tcg.c > index 8748c05..11a8daf 100644 > --- a/tcg/tcg.c > +++ b/tcg/tcg.c > @@ -585,7 +585,7 @@ void tcg_register_helper(void *func, const char *name) > void tcg_gen_callN(TCGContext *s, TCGv_ptr func, unsigned int flags, > int sizemask, TCGArg ret, int nargs, TCGArg *args) > { > -#ifdef TCG_TARGET_I386 > +#if defined TCG_TARGET_I386 && TCG_TARGET_REG_BITS < 64 This function uses #if defined(TCG_TARGET_I386) in other places, so I'd use parentheses here for consistency. > int call_type; > #endif > int i; > @@ -612,7 +612,7 @@ void tcg_gen_callN(TCGContext *s, TCGv_ptr func, unsigned int flags, > > *gen_opc_ptr++ = INDEX_op_call; > nparam = gen_opparam_ptr++; > -#ifdef TCG_TARGET_I386 > +#if defined TCG_TARGET_I386 && TCG_TARGET_REG_BITS < 64 Same here. Apart from that, I also need this patch to be able to build qemu on fedora 15 :) Christophe
On Tue, May 17, 2011 at 01:25:10PM +0200, Christophe Fergeau wrote: > Hi Hans, > > On Tue, May 03, 2011 at 01:03:40PM +0200, Hans de Goede wrote: > > diff --git a/target-i386/kvm.c b/target-i386/kvm.c > > index a13599d..e9e8d54 100644 > > --- a/target-i386/kvm.c > > +++ b/target-i386/kvm.c > > @@ -950,7 +950,7 @@ static int kvm_get_xsave(CPUState *env) > > @@ -966,7 +966,7 @@ static int kvm_get_xsave(CPUState *env) > > cwd = (uint16_t)xsave->region[0]; > > swd = (uint16_t)(xsave->region[0] >> 16); > > twd = (uint16_t)xsave->region[1]; > > - fop = (uint16_t)(xsave->region[1] >> 16); > > + /* fop = (uint16_t)(xsave->region[1] >> 16); */ > > Wouldn't it be better to drop this line? > > > env->fpstt = (swd >> 11) & 7; > > env->fpus = swd; > > env->fpuc = cwd; > > diff --git a/tcg/tcg.c b/tcg/tcg.c > > index 8748c05..11a8daf 100644 > > --- a/tcg/tcg.c > > +++ b/tcg/tcg.c > > @@ -585,7 +585,7 @@ void tcg_register_helper(void *func, const char *name) > > void tcg_gen_callN(TCGContext *s, TCGv_ptr func, unsigned int flags, > > int sizemask, TCGArg ret, int nargs, TCGArg *args) > > { > > -#ifdef TCG_TARGET_I386 > > +#if defined TCG_TARGET_I386 && TCG_TARGET_REG_BITS < 64 > > This function uses #if defined(TCG_TARGET_I386) in other places, so I'd use > parentheses here for consistency. > > > int call_type; > > #endif > > int i; > > @@ -612,7 +612,7 @@ void tcg_gen_callN(TCGContext *s, TCGv_ptr func, unsigned int flags, > > > > *gen_opc_ptr++ = INDEX_op_call; > > nparam = gen_opparam_ptr++; > > -#ifdef TCG_TARGET_I386 > > +#if defined TCG_TARGET_I386 && TCG_TARGET_REG_BITS < 64 > > Same here. > > Apart from that, I also need this patch to be able to build qemu on fedora > 15 :) > You should also probably split the patch in two parts, as it touch totally different parts of QEMU, and thus is going to be reviewed/applied by different person. I am fine with the TCG part after the comments from Christophe, so I'll apply it as soon as the new version is out.
On (Tue) 03 May 2011 [13:03:40], Hans de Goede wrote: > --- > target-i386/kvm.c | 4 ++-- > tcg/tcg.c | 4 ++-- > 2 files changed, 4 insertions(+), 4 deletions(-) Thanks; just got hit by this. However, there are a couple of whitespace issues: > --- a/tcg/tcg.c > +++ b/tcg/tcg.c > @@ -585,7 +585,7 @@ void tcg_register_helper(void *func, const char *name) > void tcg_gen_callN(TCGContext *s, TCGv_ptr func, unsigned int flags, > int sizemask, TCGArg ret, int nargs, TCGArg *args) > { > -#ifdef TCG_TARGET_I386 > +#if defined TCG_TARGET_I386 && TCG_TARGET_REG_BITS < 64 > int call_type; > #endif > int i; > @@ -612,7 +612,7 @@ void tcg_gen_callN(TCGContext *s, TCGv_ptr func, unsigned int flags, > > *gen_opc_ptr++ = INDEX_op_call; > nparam = gen_opparam_ptr++; > -#ifdef TCG_TARGET_I386 > +#if defined TCG_TARGET_I386 && TCG_TARGET_REG_BITS < 64 > call_type = (flags & TCG_CALL_TYPE_MASK); > #endif > if (ret != TCG_CALL_DUMMY_ARG) { Both these lines have a trailing space. Care to resubmit with Anthony in CC? You can add: Acked-by: Amit Shah <amit.shah@redhat.com> Amit
Hi, Here are Hans's patches split and with the various issues that were pointed out fixed. I'm not sure how I'm supposed to handle acks, original author, ... when reworking patches this way, let me know if I should proceed differently. Christophe Christophe Fergeau (2): tcg: Fix unused-but-set-variable warning kvm: Fix unused-but-set-variable warning target-i386/kvm.c | 3 +-- tcg/tcg.c | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-)
On (Mon) 30 May 2011 [12:28:01], Christophe Fergeau wrote: > Hi, > > Here are Hans's patches split and with the various issues that were pointed > out fixed. I'm not sure how I'm supposed to handle acks, original author, > ... when reworking patches this way, let me know if I should proceed > differently. You should keep From: as the same person, keep his Signed-off-by, add your Signed-off-by and mention what you have changed in the series. Thanks for re-sending this, btw. Amit
On Mon, May 30, 2011 at 04:23:43PM +0530, Amit Shah wrote: > You should keep From: as the same person, keep his Signed-off-by, add > your Signed-off-by and mention what you have changed in the series. Hans's patches didn't have a S-o-b. I can resend the same patches again with my S-o-b (I'm fine with doing this without Hans's since I came up with exactly the same patches independently from Hans) and detailing the changes from the initial patch. Does that sound good? Christophe
On (Mon) 30 May 2011 [15:56:28], Christophe Fergeau wrote: > On Mon, May 30, 2011 at 04:23:43PM +0530, Amit Shah wrote: > > You should keep From: as the same person, keep his Signed-off-by, add > > your Signed-off-by and mention what you have changed in the series. > > Hans's patches didn't have a S-o-b. I can resend the same patches again > with my S-o-b (I'm fine with doing this without Hans's since I came up with > exactly the same patches independently from Hans) and detailing the changes > from the initial patch. Does that sound good? Yes, that's fine. Amit
Here is another version of these patches. Christophe Changes: v3 - added S-o-b v2 - split patches - removed whitespace at end of lines in tcm. - changed #if defined XXX to #if defined(XXX) - removed commented out line from kvm.c in the 1st patch v1 - initial version from Hans Christophe Fergeau (2): tcg: Fix unused-but-set-variable warning kvm: Fix unused-but-set-variable warning target-i386/kvm.c | 3 +-- tcg/tcg.c | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/target-i386/kvm.c b/target-i386/kvm.c index a13599d..e9e8d54 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -950,7 +950,7 @@ static int kvm_get_xsave(CPUState *env) #ifdef KVM_CAP_XSAVE struct kvm_xsave* xsave; int ret, i; - uint16_t cwd, swd, twd, fop; + uint16_t cwd, swd, twd; if (!kvm_has_xsave()) { return kvm_get_fpu(env); @@ -966,7 +966,7 @@ static int kvm_get_xsave(CPUState *env) cwd = (uint16_t)xsave->region[0]; swd = (uint16_t)(xsave->region[0] >> 16); twd = (uint16_t)xsave->region[1]; - fop = (uint16_t)(xsave->region[1] >> 16); + /* fop = (uint16_t)(xsave->region[1] >> 16); */ env->fpstt = (swd >> 11) & 7; env->fpus = swd; env->fpuc = cwd; diff --git a/tcg/tcg.c b/tcg/tcg.c index 8748c05..11a8daf 100644 --- a/tcg/tcg.c +++ b/tcg/tcg.c @@ -585,7 +585,7 @@ void tcg_register_helper(void *func, const char *name) void tcg_gen_callN(TCGContext *s, TCGv_ptr func, unsigned int flags, int sizemask, TCGArg ret, int nargs, TCGArg *args) { -#ifdef TCG_TARGET_I386 +#if defined TCG_TARGET_I386 && TCG_TARGET_REG_BITS < 64 int call_type; #endif int i; @@ -612,7 +612,7 @@ void tcg_gen_callN(TCGContext *s, TCGv_ptr func, unsigned int flags, *gen_opc_ptr++ = INDEX_op_call; nparam = gen_opparam_ptr++; -#ifdef TCG_TARGET_I386 +#if defined TCG_TARGET_I386 && TCG_TARGET_REG_BITS < 64 call_type = (flags & TCG_CALL_TYPE_MASK); #endif if (ret != TCG_CALL_DUMMY_ARG) {