Bug 29329 - hp-timing not correct for graviton
Summary: hp-timing not correct for graviton
Status: RESOLVED FIXED
Alias: None
Product: glibc
Classification: Unclassified
Component: libc (show other bugs)
Version: unspecified
: P2 minor
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
: 29330 (view as bug list)
Depends on:
Blocks:
 
Reported: 2022-07-07 16:41 UTC by Tang, Jun
Modified: 2023-03-08 16:31 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tang, Jun 2022-07-07 16:41:05 UTC
hp-timing is only used on benchtests today, but it can be used on other things in the future. It's broken on graviton beacuse the freq we have is 1050000000. 1000000000ULL/1050000000ULL = 0 and thus the difference becomes 0.

The following patch can fix the bug:

```
diff --git a/sysdeps/aarch64/hp-timing.h b/sysdeps/aarch64/hp-timing.h
index f7f7ac7cae..270e0c2e24 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) = (hp_timing_t)((End) - (Start)) * ((1000000000.0) / freq);  \
 })

 #endif /* hp-timing.h */
```
Comment 1 Florian Weimer 2022-07-07 17:50:56 UTC
*** Bug 29330 has been marked as a duplicate of this bug. ***
Comment 2 Tang, Jun 2023-01-31 17:12:25 UTC
The proposed patch is

```
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 */
```
Comment 3 Wilco 2023-03-08 16:31:36 UTC
Fixed:

commit 311a7e0256975275d97077f1af338bc9caf0c837
Author: Jun Tang <juntangc@amazon.com>
Date:   Wed Feb 22 16:45:59 2023 +0000

    AArch64: Fix HP_TIMING_DIFF computation [BZ# 29329]
    
    Fix the computation to allow for cntfrq_el0 being larger than 1GHz.
    Assume cntfrq_el0 is a multiple of 1MHz to increase the maximum
    interval (1024 seconds at 1GHz).
    
    Reviewed-by: Wilco Dijkstra  <Wilco.Dijkstra@arm.com>