Re: gprofng patch to review

[Vladimir Mezentsev <vladimir.mezentsev@oracle.com>]

On 11/9/22 07:00, Kurt Goebel wrote:
>
>
> https://sourceware.org/pipermail/binutils/2022-November/124238.html

Hi Markus,
It looks like you sent patch for review to the gdb alias.
I am only in binutils@sourceware.org.
See my comments below.


[PATCH 1/2] gprofng: make cpu identification available to others
Markus Metzger markus.t.metzger@intel.com
Tue Nov 8 05:45:29 GMT 2022
Previous message (by thread): [PATCH] x86: Correct wrong comments in 
vex_w_table
Next message (by thread): [PATCH 2/2] gdb, btrace: use cpuident.h to 
implement btrace_this_cpu
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
I ran gprofng.display/display.exp on Fedora 36 x86-64 and I got

     UNSUPPORTED: jsynprog

It is expected. You built gprofng without java profiling support.


     ERROR: comparison of results in mttest failed
     UNRESOLVED: mttest
     ERROR: comparison of results in mttest failed

     UNRESOLVED: mttest
     PASS: synprog
     PASS: synprog


Usually, when synprog passes and mttest fails, it means that 
pthread_create interposition is broken in libgp-collector.so.
In any case, it is not result of your patch.
I never run gprofng on Fedora 36.
Please create a bug against gprofng and describe how you configure your 
build.



with and without this patch.
---
  gprofng/common/hwcdrv.c                      |  2 +-
  gprofng/common/cpuid.c => include/cpuident.h | 17 +++++++++++++----
  2 files changed, 14 insertions(+), 5 deletions(-)
  rename gprofng/common/cpuid.c => include/cpuident.h (96%)

diff --git a/gprofng/common/hwcdrv.c b/gprofng/common/hwcdrv.c
index 03c3a71c8e2..115638aa623 100644
--- a/gprofng/common/hwcdrv.c
+++ b/gprofng/common/hwcdrv.c
@@ -32,7 +32,7 @@
  /* macros */
  #define IS_GLOBAL /* Mark global symbols */

-#include "cpuid.c" /* ftns for identifying a chip */
+#include "cpuident.h" /* ftns for identifying a chip */

  static hdrv_pcbe_api_t hdrv_pcbe_core_api;
  static hdrv_pcbe_api_t hdrv_pcbe_opteron_api;
diff --git a/gprofng/common/cpuid.c b/include/cpuident.h
similarity index 96%
rename from gprofng/common/cpuid.c
rename to include/cpuident.h
index 211e09aa8ac..7ab6fc1f2b1 100644
--- a/gprofng/common/cpuid.c
+++ b/include/cpuident.h
@@ -18,6 +18,11 @@
     Foundation, 51 Franklin Street - Fifth Floor, Boston,
     MA 02110-1301, USA.  */

+#ifndef _CPUIDENT_H
+#define _CPUIDENT_H
+
+#include <stdint.h>
+
  #if defined(__i386__) || defined(__x86_64)
  #include <cpuid.h>  /* GCC-provided */
  #elif defined(__aarch64__)
@@ -85,7 +90,7 @@ typedef struct


  #if defined(__i386__) || defined(__x86_64)
-static uint_t
+static unsigned int
  cpuid_vendorstr_to_vendorcode (char *vendorstr)
  {
    if (strcmp (vendorstr, X86_VENDORSTR_Intel) == 0)
@@ -101,8 +106,10 @@ my_cpuid (unsigned int op, cpuid_regs_t *regs)
  {
    regs->eax = regs->ebx = regs->ecx = regs->edx = 0;
    int ret = __get_cpuid (op, &regs->eax, &regs->ebx, &regs->ecx, 
&regs->edx);
+#ifdef DBG_LT1
    TprintfT (DBG_LT1, "my_cpuid: __get_cpuid(0x%x, 0x%x, 0x%x, 0x%x, 
0x%x) returns %d\n",
          op, regs->eax, regs->ebx, regs->ecx, regs->edx, ret);
+#endif



I suggest to just remove TprintfT. This debug output is currently not 
used by anyone.

Thanks for fixing this, and please check it in.

-Vladimir



    return ret;
  }
  #endif
@@ -184,20 +191,22 @@ get_cpuid_info ()
    return cpi;
  }

-static inline uint_t
+static inline unsigned int
  cpuid_getvendor ()
  {
    return get_cpuid_info ()->cpi_vendor;
  }

-static inline uint_t
+static inline unsigned int
  cpuid_getfamily ()
  {
    return get_cpuid_info ()->cpi_family;
  }

-static inline uint_t
+static inline unsigned int
  cpuid_getmodel ()
  {
    return get_cpuid_info ()->cpi_model;
  }
+
+#endif /* _CPUIDENT_H */
-- 
2.37.3