Re: patch for hp-timing.h

[Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>]

On 03/02/23 10:29, Wilco Dijkstra via Libc-alpha wrote:
> Hi Jun,
> 
>> hp-timing is only used on benchtest today, but it can be used on other
>> measurements in the future.
> 
> It can't as proposed below since this would overflow in ~16 seconds.
> We could improve this by removing the zero bits from the multiply
> and doing some shifts, maybe that will make the interval long enough
> for integer multiply. Otherwise I'd suggest the floating point version since
> it's not like the benchtests don't already use floating point.
> 
> Cheers,
> Wilco

Do we really need to handle more than ~16 seconds for the hp-timing? It
is usually used along with a accumulator and I would expect that once you
need to measure more than 16s using clock_gettime works as expected.

> 
> diff --git a/benchtests/Makefile b/benchtests/Makefile
> index 292976b26b..a624614207 100644
> --- a/benchtests/Makefile
> +++ b/benchtests/Makefile
> @@ -499,4 +499,5 @@ $(objpfx)bench-%.c: %-inputs $(bench-deps)
>  	  cat $($*-INCLUDE); \
>  	fi; \
>  	$(PYTHON) scripts/bench.py $(patsubst %-inputs,%,$<); } > $@-tmp
> +	cp -f $@-tmp $@-bak
> 
> Unintended change I guess?
> 
>  	mv -f $@-tmp $@
> diff --git a/sysdeps/aarch64/hp-timing.h b/sysdeps/aarch64/hp-timing.h
> index f7f7ac7cae..c699effe6a 100644
> --- a/sysdeps/aarch64/hp-timing.h
> +++ b/sysdeps/aarch64/hp-timing.h
> @@ -41,7 +41,7 @@ typedef uint64_t hp_timing_t;
>  #define HP_TIMING_DIFF(Diff, Start, End)			\
>  ({  hp_timing_t freq;						\
>      __asm__ __volatile__ ("mrs %0, cntfrq_el0" : "=r" (freq));	\
> -   (Diff) = ((End) - (Start)) * (UINT64_C(1000000000) / freq);	\
> +   (Diff) = (((End) - (Start)) * UINT64_C(1000000000)) / freq; \
>  })
>  
>  #endif	/* hp-timing.h */