SWDEV-281658 - Preserve the callback IDs enum ordering

This change addresses the rocprofiler and HIP backward compatibility
issues. Before this patch, each time the hip_prof_str.h header was
generated, the ordering of the callbacks IDs changed, causing
incompatibilities between tools compiled with the old header and
runtimes compiled with the new headers (or vice versa).

To make the API callback IDs stable, the previous version of the header
is read to extract the enum values so that the same values can be
assigned in the new header.

Also, to make diffing different versions of the hip_prof_str.h easier
to read, all other sections (types, macros, helper functions) are now
alphabetically ordered.

If an update to the checked-in hip_prof_str.h file is required, the
cmake build is aborted and a message printed on stderr. The build will
not be successful until the checked-in hip_prof_str.h and the generated
hip_prof_str.h match.

Change-Id: I38b920e601185f7365a76a6584df91a7e8a11798

hipamd/src/CMakeLists.txt

@@ -251,6 +251,7 @@ endif()
 #############################
 # Generate profiling API macros/structures header
 set(PROF_API_STR "${PROJECT_BINARY_DIR}/include/hip/amd_detail/hip_prof_str.h")
+set(PROF_API_STR_IN "${CMAKE_SOURCE_DIR}/include/hip/amd_detail/hip_prof_str.h")
 set(PROF_API_HDR "${HIP_COMMON_INCLUDE_DIR}/hip/hip_runtime_api.h")
 set(PROF_API_SRC "${CMAKE_CURRENT_SOURCE_DIR}")
 set(PROF_API_GEN "${CMAKE_CURRENT_SOURCE_DIR}/hip_prof_gen.py")
@@ -258,8 +259,8 @@ set(PROF_API_LOG "${PROJECT_BINARY_DIR}/hip_prof_gen.log.txt")
 
 find_package(PythonInterp REQUIRED)
 add_custom_command(OUTPUT ${PROF_API_STR}
-  COMMAND ${PYTHON_EXECUTABLE} ${PROF_API_GEN} -v -t --priv ${PROF_API_HDR} ${PROF_API_SRC} ${PROF_API_STR}
+  COMMAND ${PYTHON_EXECUTABLE} ${PROF_API_GEN} -v -t --priv ${PROF_API_HDR} ${PROF_API_SRC} ${PROF_API_STR_IN} ${PROF_API_STR}
-  DEPENDS ${PROF_API_HDR} ${PROF_API_GEN}
+  DEPENDS ${PROF_API_STR_IN} ${PROF_API_HDR} ${PROF_API_GEN}
   COMMENT "Generating profiling primitives: ${PROF_API_STR}")
 
 add_custom_target(gen-prof-api-str-header ALL

hipamd/src/hip_prof_api.h

@@ -65,7 +65,7 @@ class api_callbacks_table_t {
   };
 
   struct hip_cb_table_t {
-    hip_cb_table_entry_t arr[HIP_API_ID_NUMBER] = {};
+    hip_cb_table_entry_t arr[HIP_API_ID_LAST + 1] = {};
   };
 
   api_callbacks_table_t() = default;
@@ -74,7 +74,7 @@ class api_callbacks_table_t {
     std::lock_guard<mutex_t> lock(mutex_);
     bool ret = true;
 
-    if (id < HIP_API_ID_NUMBER) {
+    if (id >= HIP_API_ID_FIRST && id <= HIP_API_ID_LAST) {
       cb_sync(id);
       /*
       'fun != nullptr' indicates it is activity register call,
@@ -112,7 +112,7 @@ class api_callbacks_table_t {
     std::lock_guard<mutex_t> lock(mutex_);
     bool ret = true;
 
-    if (id < HIP_API_ID_NUMBER) {
+    if (id >= HIP_API_ID_FIRST && id <= HIP_API_ID_LAST) {
       cb_sync(id);
       callbacks_table_.arr[id].fun = fun;
       callbacks_table_.arr[id].arg = arg;
@@ -192,7 +192,7 @@ class api_callbacks_spawner_t {
   {
     if (!was_enabled_on_construction_) return;
 
-    if (cid_ >= HIP_API_ID_NUMBER) {
+    if (cid_ < HIP_API_ID_FIRST || cid_ > HIP_API_ID_LAST) {
       fprintf(stderr, "HIP %s bad id %d\n", __FUNCTION__, cid_);
       abort();
     }
@@ -244,7 +244,7 @@ class api_callbacks_spawner_t {
 };
 
 template <>
-class api_callbacks_spawner_t<HIP_API_ID_NUMBER> {
+class api_callbacks_spawner_t<HIP_API_ID_NONE> {
  public:
   api_callbacks_spawner_t() {}
   void call() {}

hipamd/src/hip_prof_gen.py

@@ -21,6 +21,8 @@
 # THE SOFTWARE.
 
 import os, sys, re
+import CppHeaderParser
+import filecmp
 
 PROF_HEADER = "hip_prof_str.h"
 OUTPUT = PROF_HEADER
@@ -375,49 +377,61 @@ def parse_src(api_map, src_path, src_patt, out):
 #############################################################
 # Generating profiling primitives header
 # api_map - public API map [<api name>] => [(type, name), ...]
+# callback_ids - public API callback IDs list (name, callback_id)
 # opts_map - opts map  [<api name>] => [opt0, opt1, ...]
-def generate_prof_header(f, api_map, opts_map):
+def generate_prof_header(f, api_map, callback_ids, opts_map):
   # Private API list
   priv_lst = []
 
-  f.write('// automatically generated sources\n')
+  f.write('// Generated file. DO NOT EDIT.\n')
+  f.write('//\n')
+  f.write('// This file is automatically generated by the ' + os.path.basename(__file__) + ' script.\n')
+  f.write('// If changes are required, run the script and commit the updated file.\n\n')
   f.write('#ifndef _HIP_PROF_STR_H\n');
   f.write('#define _HIP_PROF_STR_H\n');
   f.write('#define HIP_PROF_VER 1\n')
 
-  # Generating dummy macro for non-public API
+  # Check for non-public API
-  f.write('\n// Dummy API primitives\n')
+  for name in sorted(opts_map.keys()):
-  f.write('#define INIT_NONE_CB_ARGS_DATA(cb_data) {};\n')
-  for name in opts_map:
     if not name in api_map:
       opts_lst = opts_map[name]
       if len(opts_lst) != 0:
         fatal("bad dummy API \"" + name + "\", args: " + str(opts_lst))
-      f.write('#define INIT_'+ name + '_CB_ARGS_DATA(cb_data) {};\n')
       priv_lst.append(name)
-
+      message("Private: " + name)
-  for name in priv_lst:
-    message("Private: " + name)
 
   # Generating the callbacks ID enumaration
-  f.write('\n// HIP API callbacks ID enumaration\n')
+  f.write('\n// HIP API callbacks ID enumeration\n')
   f.write('enum hip_api_id_t {\n')
-  cb_id = 0
+  f.write('  HIP_API_ID_NONE = 0,\n')
-  for name in api_map.keys():
+  f.write('  HIP_API_ID_FIRST = 1,\n')
-    f.write('  HIP_API_ID_' + name + ' = ' + str(cb_id) + ',\n')
+
-    cb_id += 1
+  cb_id_map = {}
-  f.write('  HIP_API_ID_NUMBER = ' + str(cb_id) + ',\n')
+  last_cb_id = 0
+  for name, cb_id in callback_ids:
+    if not name in api_map:
+      f.write('  HIP_API_ID_RESERVED_' + str(cb_id) + ' = ' + str(cb_id) + ',\n')
+    else:
+      f.write('  HIP_API_ID_' + name + ' = ' + str(cb_id) + ',\n')
+    cb_id_map[name] = cb_id
+    if cb_id > last_cb_id: last_cb_id = cb_id
+
+  for name in sorted(api_map.keys()):
+    if not name in cb_id_map:
+      last_cb_id += 1
+      f.write('  HIP_API_ID_' + name + ' = ' + str(last_cb_id) + ',\n')
+
+  f.write('  HIP_API_ID_LAST = ' + str(last_cb_id) + ',\n')
   f.write('\n')
-  f.write('  HIP_API_ID_NONE = HIP_API_ID_NUMBER,\n')
+  for name in sorted(priv_lst):
-  for name in priv_lst:
+    f.write('  HIP_API_ID_' + name + ' = HIP_API_ID_NONE,\n')
-    f.write('  HIP_API_ID_' + name + ' = HIP_API_ID_NUMBER,\n')
   f.write('};\n')
 
   # Generating the method to return API name by ID
-  f.write('\n// Return HIP API string by given ID\n')
+  f.write('\n// Return the HIP API string for a given callback ID\n')
   f.write('static inline const char* hip_api_name(const uint32_t id) {\n')
   f.write('  switch(id) {\n')
-  for name in api_map.keys():
+  for name in sorted(api_map.keys()):
     f.write('    case HIP_API_ID_' + name + ': return "' +  name + '";\n')
   f.write('  };\n')
   f.write('  return "unknown";\n')
@@ -426,22 +440,23 @@ def generate_prof_header(f, api_map, opts_map):
   # Generating the method for querying API ID by name
   f.write('\n')
   f.write('#include <string.h>\n');
-  f.write('// Return HIP API ID by given name\n')
+  f.write('// Return the HIP API callback ID for a given name\n')
   f.write('static inline uint32_t hipApiIdByName(const char* name) {\n')
-  for name, args in api_map.items():
+  for name in sorted(api_map.keys()):
     f.write('  if (strcmp("' + name + '", name) == 0) return HIP_API_ID_' + name + ';\n')
-  f.write('  return HIP_API_ID_NUMBER;\n')
+  f.write('  return HIP_API_ID_NONE;\n')
   f.write('}\n')
 
   # Generating the callbacks data structure
-  f.write('\n// HIP API callbacks data structure\n')
+  f.write('\n// HIP API callbacks data structures\n')
   f.write(
   'typedef struct hip_api_data_s {\n' +
   '  uint64_t correlation_id;\n' +
   '  uint32_t phase;\n' +
   '  union {\n'
   )
-  for name, args in api_map.items():
+  for name in sorted(api_map.keys()):
+    args = api_map[name]
     if len(args) != 0:
       f.write('    struct {\n')
       for arg_tuple in args:
@@ -462,7 +477,8 @@ def generate_prof_header(f, api_map, opts_map):
 
   # Generating the callbacks args data filling macros
   f.write('\n// HIP API callbacks args data filling macros\n')
-  for name, args in api_map.items():
+  for name in sorted(api_map.keys()):
+    args = api_map[name]
     f.write('// ' + name + str(args) + '\n')
     f.write('#define INIT_' + name + '_CB_ARGS_DATA(cb_data) { \\\n')
     if name in opts_map:
@@ -485,13 +501,20 @@ def generate_prof_header(f, api_map, opts_map):
     f.write('};\n')
   f.write('#define INIT_CB_ARGS_DATA(cb_id, cb_data) INIT_##cb_id##_CB_ARGS_DATA(cb_data)\n')
 
-  f.write('#if HIP_PROF_HIP_API_STRING\n')
+  # Generating macro for non-public API
+  f.write('\n// Macros for non-public API primitives\n')
+  for name in sorted(priv_lst):
+    f.write('// ' + name + '()\n')
+    f.write('#define INIT_'+ name + '_CB_ARGS_DATA(cb_data) {};\n')
+  f.write('\n#define INIT_NONE_CB_ARGS_DATA(cb_data) {};\n')
+
+  f.write('\n#if HIP_PROF_HIP_API_STRING\n')
   # Generating the method for the API args filling
-  f.write('\n')
+  f.write('// HIP API args filling helper\n')
-  f.write('// HIP API args filling method\n')
   f.write('static inline void hipApiArgsInit(hip_api_id_t id, hip_api_data_t* data) {\n')
   f.write('  switch (id) {\n')
-  for name, args in api_map.items():
+  for name in sorted(api_map.keys()):
+    args = api_map[name]
     f.write('// ' + name + str(args) + '\n')
     f.write('    case HIP_API_ID_' + name + ':\n')
     for ind in range(0, len(args)):
@@ -518,7 +541,8 @@ def generate_prof_header(f, api_map, opts_map):
   f.write('static inline const char* hipApiString(hip_api_id_t id, const hip_api_data_t* data) {\n')
   f.write('  std::ostringstream oss;\n')
   f.write('  switch (id) {\n')
-  for name, args in api_map.items():
+  for name in sorted(api_map.keys()):
+    args = api_map[name]
     f.write('    case HIP_API_ID_' + name + ':\n')
     f.write('      oss << "' + name + '(";\n')
     for ind in range(0, len(args)):
@@ -579,8 +603,8 @@ while len(sys.argv) > 1:
     sys.argv.pop(1)
 
 # Usage
-if (len(sys.argv) < 3):
+if (len(sys.argv) < 4):
-  fatal ("Usage: " + sys.argv[0] + " [-v] <input HIP API .h file> <patched srcs path> [<output>]\n" +
+  fatal ("Usage: " + sys.argv[0] + " [-v] <input HIP API .h file> <patched srcs path> <previous output> [<output>]\n" +
          "  -v - verbose messages\n" +
          "  -r - process source directory recursively\n" +
          "  -t - API types matching check\n" +
@@ -589,7 +613,8 @@ if (len(sys.argv) < 3):
          "  -p - HIP_INIT_API macro patching mode\n" +
          "\n" +
          "  Example:\n" +
-         "  $ " + sys.argv[0] + " -v -p -t --priv ./api/hip/include/hip/amd_detail/hip_runtime_api.h ./api/hip ./api/hip/include/hip/amd_detail/hip_prof_str.h");
+         "  $ " + sys.argv[0] + " -v -p -t --priv ./include/hip/amd_detail/hip_runtime_api.h" +
+         " ./src ./include/hip/amd_detail/hip_prof_str.h ./include/hip/amd_detail/hip_prof_str.h.new");
 
 # API header file given as an argument
 src_pat = "\.cpp$"
@@ -602,7 +627,12 @@ src_dir = sys.argv[2]
 if not os.path.isdir(src_dir):
   fatal("src directory " + src_dir + "' not found")
 
-if len(sys.argv) > 3: OUTPUT = sys.argv[3]
+# Current hip_prof_str include
+INPUT = sys.argv[3]
+if not os.path.isfile(INPUT):
+  fatal("input file '" + INPUT + "' not found")
+
+if len(sys.argv) > 4: OUTPUT = sys.argv[4]
 
 # API declaration map
 api_map = {
@@ -639,6 +669,27 @@ parse_api(api_hfile, api_map)
 # Parsing sources
 parse_src(api_map, src_dir, src_pat, opts_map)
 
+try:
+  cppHeader = CppHeaderParser.CppHeader(INPUT)
+except CppHeaderParser.CppParseError as e:
+  print(e)
+  sys.exit(1)
+
+# Callback IDs
+api_callback_ids = []
+
+for enum in cppHeader.enums:
+  if enum['name'] == 'hip_api_id_t':
+    for value in enum['values']:
+      if value['name'] == 'HIP_API_ID_NONE' or value['name'] == 'HIP_API_ID_FIRST':
+        continue
+      if value['name'] == 'HIP_API_ID_LAST':
+        break
+      m = re.match(r'HIP_API_ID_(\S*)', value['name'])
+      if m:
+        api_callback_ids.append((m.group(1), value['value']))
+    break
+
 # Checking for non-conformant APIs with missing HIP_INIT macro
 for name in list(opts_map.keys()):
   m = re.match(r'\.(\S*)', name)
@@ -667,7 +718,10 @@ if not os.path.exists(output_dir):
 
 # Generating output header file
 with open(OUTPUT, 'w') as f:
-  generate_prof_header(f, api_map, opts_map)
+  generate_prof_header(f, api_map, api_callback_ids, opts_map)
+
+if not filecmp.cmp(INPUT, OUTPUT):
+  fatal("\"" + INPUT + "\" needs to be re-generated and checked-in with the current changes")
 
 # Successfull exit
 sys.exit(0)