[pulseaudio-discuss] [PATCH v3] netbsd: Stop depending upon nonstandard __WORDSIZE

[Pali Rohár]

On Thursday 17 December 2015 00:44:03 Kamil Rytarowski wrote:
> On 16.12.2015 11:02, Pali Rohár wrote:
> > On Wednesday 16 December 2015 10:40:05 Kamil Rytarowski wrote:
> >> There is no way to check CPU type in a portable way across ABIs.
> >> 
> >> Go for pa_mult_s16_volume() version without need for int64_t. 
> >> It's actually the same as in src/tests/mult-s16-test.c.
> >> 
> >> This change fixes build on NetBSD. --- 
> >> src/pulsecore/sample-util.h | 5 ----- src/tests/mult-s16-test.c
> >> | 9 ++------- 2 files changed, 2 insertions(+), 12 deletions(-)
> >> 
> >> diff --git a/src/pulsecore/sample-util.h
> >> b/src/pulsecore/sample-util.h index c817bc9..9862938 100644 ---
> >> a/src/pulsecore/sample-util.h +++ b/src/pulsecore/sample-util.h 
> >> @@ -55,10 +55,6 @@ void pa_deinterleave(const void *src, void
> >> *dst[], unsigned channels, size_t ss, void
> >> pa_sample_clamp(pa_sample_format_t format, void *dst, size_t
> >> dstr, const void *src, size_t sstr, unsigned n);
> >> 
> >> static inline int32_t pa_mult_s16_volume(int16_t v, int32_t cv)
> >> { -#if __WORDSIZE == 64 || ((ULONG_MAX) > (UINT_MAX)) -    /*
> >> Multiply with 64 bit integers on 64 bit platforms */ -    return
> >> (v * (int64_t) cv) >> 16; -#else /* Multiplying the 32 bit volume
> >> factor with the * 16 bit sample might result in an 48 bit value.
> >> We * want to do without 64 bit integers and hence do @@ -68,7
> >> +64,6 @@ static inline int32_t pa_mult_s16_volume(int16_t v,
> >> int32_t cv) { int32_t hi = cv >> 16; int32_t lo = cv & 0xFFFF; 
> >> return ((v * lo) >> 16) + (v * hi); -#endif }
> >> 
> > 
> > Hi! There is probably no portable way for checking CPU type. But
> > you do not need to know CPU type here. You need to know if CPU has
> > fast 64bit numeric operations (and use 64bit speed optimization),
> > right?
> > 
> > And this could be possible by checking size of int_fast32_t type 
> > (defined in stdint.h). If is same as size of int_fast64_t type then
> > you could use 64bit optimization otherwise not (= 32bit integers
> > are faster then 64bit).
> > 
> > But I will let this suggestion for reviewing of other people.
> > 
> 
> This sounds like a good way to go.
> 
> I noted that we trigger UB in pa_mult_s16_volume(), as results of bit
> shifts over signed integers are undefined. I will try to address it
> too (as a separate commit).

You can cast signed integers to unsigned and do shifts. But you need to
make sure that casting does not break support for negative numbers. This
could be quite hacky.

> >> pa_usec_t pa_bytes_to_usec_round_up(uint64_t length, const
> >> pa_sample_spec *spec); diff --git a/src/tests/mult-s16-test.c
> >> b/src/tests/mult-s16-test.c index d2a351c..20ad107 100644 ---
> >> a/src/tests/mult-s16-test.c +++ b/src/tests/mult-s16-test.c @@
> >> -93,13 +93,8 @@ int main(int argc, char *argv[]) { if
> >> (!getenv("MAKE_CHECK")) pa_log_set_level(PA_LOG_DEBUG);
> >> 
> >> -#if __WORDSIZE == 64 || ((ULONG_MAX) > (UINT_MAX)) -
> >> pa_log_debug("This seems to be 64-bit code."); -#elif  __WORDSIZE
> >> == 32 -    pa_log_debug("This seems to be 32-bit code."); -#else 
> >> -    pa_log_debug("Don't know if this is 32- or 64-bit code."); 
> >> -#endif +    pa_log_debug("On this platform, integer size is %zu
> >> and long size is %zu", +            sizeof(int), sizeof(long));
> >> 
> >> s = suite_create("Mult-s16"); tc = tcase_create("mult-s16");
> > 
> > With your change this comment looks now as not useful (but maybe
> > I'm mistaken).
> > 
> 
> I'm going to change the message to: whether there are detected fast
> 64-bit integer operations.

-- 
Pali Rohár
pali.rohar at gmail.com