| Message ID | 1307370348-28400-2-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
Am 06.06.2011 um 16:25 schrieb Paolo Bonzini: > This is not a CPU interface. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > diff --git a/qemu-common.h b/qemu-common.h > index b851b20..7484ef8 100644 > --- a/qemu-common.h > +++ b/qemu-common.h > @@ -4,6 +4,10 @@ > > #include "config-host.h" > > +#if defined(__arm__) || defined(__sparc__) || defined(__mips__) || > defined(__hppa__) || defined(__ia64__) > +#define WORDS_ALIGNED > +#endif Since it depends on the host and you're placing it directly under config-host.h inclusion, might it make sense to move the decision into configure instead, so that it ends up in config-host.h? Andreas
On 06/06/2011 07:15 PM, Andreas Färber wrote: > Am 06.06.2011 um 16:25 schrieb Paolo Bonzini: > >> This is not a CPU interface. >> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > >> diff --git a/qemu-common.h b/qemu-common.h >> index b851b20..7484ef8 100644 >> --- a/qemu-common.h >> +++ b/qemu-common.h >> @@ -4,6 +4,10 @@ >> >> #include "config-host.h" >> >> +#if defined(__arm__) || defined(__sparc__) || defined(__mips__) || >> defined(__hppa__) || defined(__ia64__) >> +#define WORDS_ALIGNED >> +#endif > > Since it depends on the host and you're placing it directly under > config-host.h inclusion, might it make sense to move the decision into > configure instead, so that it ends up in config-host.h? Richard's suggestion will get rid of this altogether. Paolo
On 06/06/2011 10:15 AM, Andreas Färber wrote: > Am 06.06.2011 um 16:25 schrieb Paolo Bonzini: >> +#if defined(__arm__) || defined(__sparc__) || defined(__mips__) || defined(__hppa__) || defined(__ia64__) >> +#define WORDS_ALIGNED >> +#endif > > Since it depends on the host and you're placing it directly under > config-host.h inclusion, might it make sense to move the decision > into configure instead, so that it ends up in config-host.h? Hum, I now understand what Paulo was talking about elsewhere in the thread. If he takes my suggestion to re-write the unaligned functions with GCC packed support, these host ifdefs go away, and this objection disappears. The question becomes one of ordering. Do we take his existing 3-part patch as-is, and the packed patch as a followup? Do we convert to packed accesses first and move it around after? Do we do it all in one step? r~
On 06/07/2011 12:15 AM, Richard Henderson wrote: > Do we take his existing 3-part patch as-is, and the packed patch as a followup? > Do we convert to packed accesses first and move it around after? > Do we do it all in one step? Either of the first two works for me. However, since this series was a start towards fixing real bugs reported by Coverity: qemu-kvm-0.14.0/hw/scsi-bus.c:190: sign_extension: Suspicious implicit sign extension: "cmd[10]" with type "unsigned char" (8 bits, unsigned) is promoted in "cmd[13] | (cmd[12] << 8) | (cmd[11] << 16) | (cmd[10] << 24)" to type "int" (32 bits, signed), then sign-extended to type "unsigned long" (64 bits, unsigned). If "cmd[13] | (cmd[12] << 8) | (cmd[11] << 16) | (cmd[10] << 24)" is greater than 0x7FFFFFFF, the upper bits of the result will all be 1. ... and there were objections on requiring recent GCC, perhaps it's better to just commit it as is. Paolo
diff --git a/cpu-common.h b/cpu-common.h index 7a86d63..7979a18 100644 --- a/cpu-common.h +++ b/cpu-common.h @@ -3,10 +3,6 @@ /* CPU interfaces that are target indpendent. */ -#if defined(__arm__) || defined(__sparc__) || defined(__mips__) || defined(__hppa__) || defined(__ia64__) -#define WORDS_ALIGNED -#endif - #ifdef TARGET_PHYS_ADDR_BITS #include "targphys.h" #endif diff --git a/qemu-common.h b/qemu-common.h index b851b20..7484ef8 100644 --- a/qemu-common.h +++ b/qemu-common.h @@ -4,6 +4,10 @@ #include "config-host.h" +#if defined(__arm__) || defined(__sparc__) || defined(__mips__) || defined(__hppa__) || defined(__ia64__) +#define WORDS_ALIGNED +#endif + #define QEMU_NORETURN __attribute__ ((__noreturn__)) #ifdef CONFIG_GCC_ATTRIBUTE_WARN_UNUSED_RESULT #define QEMU_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
This is not a CPU interface. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- cpu-common.h | 4 ---- qemu-common.h | 4 ++++ 2 files changed, 4 insertions(+), 4 deletions(-)