[Bug libc/29329] New: hp-timing not correct for graviton

[Bug libc/29329] New: hp-timing not correct for graviton

[juntangc at amazon dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=29329

            Bug ID: 29329
           Summary: hp-timing not correct for graviton
           Product: glibc
           Version: unspecified
            Status: UNCONFIRMED
          Severity: minor
          Priority: P2
         Component: libc
          Assignee: unassigned at sourceware dot org
          Reporter: juntangc at amazon dot com
                CC: drepper.fsp at gmail dot com
  Target Milestone: ---

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 */
```

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29329] hp-timing not correct for graviton

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=29329

--- Comment #1 from Florian Weimer <fweimer at redhat dot com> ---
*** Bug 29330 has been marked as a duplicate of this bug. ***

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29329] hp-timing not correct for graviton

[juntangc at amazon dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=29329

--- Comment #2 from Tang, Jun <juntangc at amazon dot com> ---
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 */
```

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29329] hp-timing not correct for graviton

[wdijkstr at arm dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=29329

Wilco <wdijkstr at arm dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|UNCONFIRMED                 |RESOLVED
                 CC|                            |wdijkstr at arm dot com

--- Comment #3 from Wilco <wdijkstr at arm dot com> ---
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>

-- 
You are receiving this mail because:
You are on the CC list for the bug.