| Message ID | 1399051958-9459-1-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On 05/02/2014 10:32 AM, Peter Maydell wrote: > We have an unfortunate naming clash between the functions > ldl_p, stl_p, etc defined in bswap.h (which have semantics > "load/store in host endianness") and the #defines of the same > name in cpu-all.h (which have the semantics "load/store in > target endianness"). > > Fortunately it turns out that the only users of the bswap.h > functions are all within bswap.h itself, so we can simply > rename them to include a _he_ infix for "host endianness". > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > Frankly I'm surprised that the only users of these functions > are the ones within bswap.h itself, but it's a lucky escape > from having to audit an enormous pile of code... > > We had talked about changing the "target-endian" accessors > to be ldl_te_p &c, but given the uses aren't tangled together > as I feared they would be, I'm not sure we can justify the > global function rename. I'm surprised too, but... good news, I guess. Reviewed-by: Richard Henderson <rth@twiddle.net> r~
On 2 May 2014 19:48, Richard Henderson <rth@twiddle.net> wrote: > On 05/02/2014 10:32 AM, Peter Maydell wrote: >> We have an unfortunate naming clash between the functions >> ldl_p, stl_p, etc defined in bswap.h (which have semantics >> "load/store in host endianness") and the #defines of the same >> name in cpu-all.h (which have the semantics "load/store in >> target endianness"). >> >> Fortunately it turns out that the only users of the bswap.h >> functions are all within bswap.h itself, so we can simply >> rename them to include a _he_ infix for "host endianness". >> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> >> --- >> Frankly I'm surprised that the only users of these functions >> are the ones within bswap.h itself, but it's a lucky escape >> from having to audit an enormous pile of code... >> >> We had talked about changing the "target-endian" accessors >> to be ldl_te_p &c, but given the uses aren't tangled together >> as I feared they would be, I'm not sure we can justify the >> global function rename. > > I'm surprised too, but... good news, I guess. > > Reviewed-by: Richard Henderson <rth@twiddle.net> Anybody care to suggest a submaintainer tree this should go in via? thanks -- PMM
On 05/15/2014 11:13 AM, Peter Maydell wrote: > On 2 May 2014 19:48, Richard Henderson <rth@twiddle.net> wrote: >> On 05/02/2014 10:32 AM, Peter Maydell wrote: >>> We have an unfortunate naming clash between the functions >>> ldl_p, stl_p, etc defined in bswap.h (which have semantics >>> "load/store in host endianness") and the #defines of the same >>> name in cpu-all.h (which have the semantics "load/store in >>> target endianness"). >>> >>> Fortunately it turns out that the only users of the bswap.h >>> functions are all within bswap.h itself, so we can simply >>> rename them to include a _he_ infix for "host endianness". >>> >>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> >>> --- >>> Frankly I'm surprised that the only users of these functions >>> are the ones within bswap.h itself, but it's a lucky escape >>> from having to audit an enormous pile of code... >>> >>> We had talked about changing the "target-endian" accessors >>> to be ldl_te_p &c, but given the uses aren't tangled together >>> as I feared they would be, I'm not sure we can justify the >>> global function rename. >> >> I'm surprised too, but... good news, I guess. >> >> Reviewed-by: Richard Henderson <rth@twiddle.net> > > Anybody care to suggest a submaintainer tree this should > go in via? Trivial? Ha, ha, only serious. r~
diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h index 0f9c6cf..78c1ced 100644 --- a/include/qemu/bswap.h +++ b/include/qemu/bswap.h @@ -215,9 +215,10 @@ typedef union { * q: 64 bits * * endian is: - * (empty): host endian + * he : host endian * be : big endian * le : little endian + * (except for byte accesses, which have no endian infix). */ static inline int ldub_p(const void *ptr) @@ -239,82 +240,82 @@ static inline void stb_p(void *ptr, uint8_t v) operations. Thus we don't need to play games with packed attributes, or inline byte-by-byte stores. */ -static inline int lduw_p(const void *ptr) +static inline int lduw_he_p(const void *ptr) { uint16_t r; memcpy(&r, ptr, sizeof(r)); return r; } -static inline int ldsw_p(const void *ptr) +static inline int ldsw_he_p(const void *ptr) { int16_t r; memcpy(&r, ptr, sizeof(r)); return r; } -static inline void stw_p(void *ptr, uint16_t v) +static inline void stw_he_p(void *ptr, uint16_t v) { memcpy(ptr, &v, sizeof(v)); } -static inline int ldl_p(const void *ptr) +static inline int ldl_he_p(const void *ptr) { int32_t r; memcpy(&r, ptr, sizeof(r)); return r; } -static inline void stl_p(void *ptr, uint32_t v) +static inline void stl_he_p(void *ptr, uint32_t v) { memcpy(ptr, &v, sizeof(v)); } -static inline uint64_t ldq_p(const void *ptr) +static inline uint64_t ldq_he_p(const void *ptr) { uint64_t r; memcpy(&r, ptr, sizeof(r)); return r; } -static inline void stq_p(void *ptr, uint64_t v) +static inline void stq_he_p(void *ptr, uint64_t v) { memcpy(ptr, &v, sizeof(v)); } static inline int lduw_le_p(const void *ptr) { - return (uint16_t)le_bswap(lduw_p(ptr), 16); + return (uint16_t)le_bswap(lduw_he_p(ptr), 16); } static inline int ldsw_le_p(const void *ptr) { - return (int16_t)le_bswap(lduw_p(ptr), 16); + return (int16_t)le_bswap(lduw_he_p(ptr), 16); } static inline int ldl_le_p(const void *ptr) { - return le_bswap(ldl_p(ptr), 32); + return le_bswap(ldl_he_p(ptr), 32); } static inline uint64_t ldq_le_p(const void *ptr) { - return le_bswap(ldq_p(ptr), 64); + return le_bswap(ldq_he_p(ptr), 64); } static inline void stw_le_p(void *ptr, uint16_t v) { - stw_p(ptr, le_bswap(v, 16)); + stw_he_p(ptr, le_bswap(v, 16)); } static inline void stl_le_p(void *ptr, uint32_t v) { - stl_p(ptr, le_bswap(v, 32)); + stl_he_p(ptr, le_bswap(v, 32)); } static inline void stq_le_p(void *ptr, uint64_t v) { - stq_p(ptr, le_bswap(v, 64)); + stq_he_p(ptr, le_bswap(v, 64)); } /* float access */ @@ -349,37 +350,37 @@ static inline void stfq_le_p(void *ptr, float64 v) static inline int lduw_be_p(const void *ptr) { - return (uint16_t)be_bswap(lduw_p(ptr), 16); + return (uint16_t)be_bswap(lduw_he_p(ptr), 16); } static inline int ldsw_be_p(const void *ptr) { - return (int16_t)be_bswap(lduw_p(ptr), 16); + return (int16_t)be_bswap(lduw_he_p(ptr), 16); } static inline int ldl_be_p(const void *ptr) { - return be_bswap(ldl_p(ptr), 32); + return be_bswap(ldl_he_p(ptr), 32); } static inline uint64_t ldq_be_p(const void *ptr) { - return be_bswap(ldq_p(ptr), 64); + return be_bswap(ldq_he_p(ptr), 64); } static inline void stw_be_p(void *ptr, uint16_t v) { - stw_p(ptr, be_bswap(v, 16)); + stw_he_p(ptr, be_bswap(v, 16)); } static inline void stl_be_p(void *ptr, uint32_t v) { - stl_p(ptr, be_bswap(v, 32)); + stl_he_p(ptr, be_bswap(v, 32)); } static inline void stq_be_p(void *ptr, uint64_t v) { - stq_p(ptr, be_bswap(v, 64)); + stq_he_p(ptr, be_bswap(v, 64)); } /* float access */
We have an unfortunate naming clash between the functions ldl_p, stl_p, etc defined in bswap.h (which have semantics "load/store in host endianness") and the #defines of the same name in cpu-all.h (which have the semantics "load/store in target endianness"). Fortunately it turns out that the only users of the bswap.h functions are all within bswap.h itself, so we can simply rename them to include a _he_ infix for "host endianness". Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- Frankly I'm surprised that the only users of these functions are the ones within bswap.h itself, but it's a lucky escape from having to audit an enormous pile of code... We had talked about changing the "target-endian" accessors to be ldl_te_p &c, but given the uses aren't tangled together as I feared they would be, I'm not sure we can justify the global function rename. include/qemu/bswap.h | 45 +++++++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 22 deletions(-)