[XPTI][NFC] Fix -Wconversion warnings (#18786)

`-Wconversion` is a must-have flag for us in according with internal
guidelines (see `AddSecurityFlags.cmake`), but we have never built with
it + `-Werror` so there are multiple places in `xpti` subproject where
we did potentially incorrect implicit conversions.

Author: AlexeySachkov

sycl/source/detail/graph_impl.cpp

@@ -818,7 +818,7 @@ exec_graph_impl::enqueueNodeDirect(sycl::context Ctx,
 
 #ifdef XPTI_ENABLE_INSTRUMENTATION
   const bool xptiEnabled = xptiTraceEnabled();
-  int32_t StreamID = xpti::invalid_id;
+  int32_t StreamID = xpti::invalid_id<>;
   xpti_td *CmdTraceEvent = nullptr;
   uint64_t InstanceID = 0;
   if (xptiEnabled) {

sycl/source/detail/queue_impl.cpp

@@ -585,7 +585,7 @@ void queue_impl::wait(const detail::code_location &CodeLoc) {
   void *TelemetryEvent = nullptr;
   uint64_t IId;
   std::string Name;
-  int32_t StreamID = xpti::invalid_id;
+  int32_t StreamID = xpti::invalid_id<>;
   if (xptiEnabled) {
     StreamID = xptiRegisterStream(SYCL_STREAM_NAME);
     TelemetryEvent = instrumentationProlog(CodeLoc, Name, StreamID, IId);

sycl/source/handler.cpp

@@ -565,7 +565,7 @@ event handler::finalize() {
 #endif
       auto EnqueueKernel = [&]() {
 #ifdef XPTI_ENABLE_INSTRUMENTATION
-        int32_t StreamID = xpti::invalid_id;
+        int32_t StreamID = xpti::invalid_id<>;
         xpti_td *CmdTraceEvent = nullptr;
         uint64_t InstanceID = 0;
         if (xptiEnabled) {

xpti/include/xpti/xpti_data_types.h

@@ -105,7 +105,7 @@ inline xpti::uid128_t make_uid128(uint64_t FileID, uint64_t FuncID, int Line,
                                   int Col) {
   xpti::uid128_t UID;
   UID.p1 = (FileID << 32) | FuncID;
-  UID.p2 = ((uint64_t)Col << 32) | Line;
+  UID.p2 = ((uint64_t)Col << 32) | (uint64_t)Line;
   UID.instance = 0;
   return UID;
 }
@@ -147,7 +147,15 @@ struct hash_t {
   ///
   /// @param value A 64-bit integer for which the bit count is to be calculated.
   /// @return The number of bits required to represent the input value.
-  int bit_count(uint64_t value) { return (int)log2(value) + 1; }
+  unsigned bit_count(uint64_t value) {
+    // FIXME: using std::log2 is imprecise. There is no guarantee that the
+    // implementation has actual integer overload for log2 and it is allowed to
+    // do a static_cast<double>(value) to compute the result. Not every integer
+    // is represetntable as floating-point, meaning that byte representation of
+    // value as double may not be the same as for integer, resulting in
+    // different result.
+    return static_cast<unsigned>(std::log2(value)) + 1;
+  }
 
   /// @brief Compacts the file ID, function ID, line number, and column number
   /// into a single 64-bit hash value.
@@ -164,7 +172,8 @@ struct hash_t {
   /// @param col An integer that represents the column number.
   /// @return A 64-bit hash value that represents the compacted file ID,
   /// function ID, line number, and column number.
-  uint64_t compact(uint64_t file_id, uint64_t func_id, int line, int col) {
+  uint64_t compact(uint64_t file_id, uint64_t func_id, uint64_t line,
+                   uint64_t col) {
     uint64_t funcB, lineB, colB;
     // Figure out the bit counts necessary to represent the input values
     funcB = bit_count(func_id);
@@ -177,9 +186,9 @@ struct hash_t {
     hash <<= funcB;
     hash = hash | func_id;
     hash <<= lineB;
-    hash = hash | (uint64_t)line;
+    hash = hash | line;
     hash <<= colB;
-    hash = hash | (uint64_t)col;
+    hash = hash | col;
 #ifdef DEBUG
     uint64_t fileB = bit_count(file_id);
     std::cout << "Total bits: " << (fileB + funcB + lineB + colB) << "\n";
@@ -203,7 +212,7 @@ struct hash_t {
   /// @param line An integer that represents the line number.
   /// @return A 64-bit hash value that represents the compacted file ID,
   /// function ID, and line number.
-  uint64_t compact_short(uint64_t file_id, uint64_t func_id, int line) {
+  uint64_t compact_short(uint64_t file_id, uint64_t func_id, uint64_t line) {
     uint64_t funcB, lineB;
     funcB = bit_count(func_id);
     lineB = bit_count(line);
@@ -215,7 +224,7 @@ struct hash_t {
     hash <<= funcB;
     hash = hash | func_id;
     hash <<= lineB;
-    hash = hash | (uint64_t)line;
+    hash = hash | line;
 #ifdef DEBUG
     uint64_t fileB = bit_count(file_id);
     std::cout << "Total bits: " << (fileB + funcB + lineB) << "\n";
@@ -312,7 +321,8 @@ struct uid_t {
 } // namespace xpti
 
 namespace xpti {
-constexpr int invalid_id = -1;
+template <typename T = uint32_t>
+constexpr T invalid_id = std::numeric_limits<T>::max();
 constexpr uint64_t invalid_uid = 0;
 constexpr uint8_t default_vendor = 0;
 
@@ -428,11 +438,11 @@ struct payload_t {
   /// Absolute path of the source file; may have to to be unicode string
   const char *source_file = nullptr;
   /// Line number information to correlate the trace point
-  uint32_t line_no = invalid_id;
+  uint32_t line_no = invalid_id<>;
   /// For a complex statement, column number may be needed to resolve the
   /// trace point; currently none of the compiler builtins return a valid
   /// column no
-  uint32_t column_no = invalid_id;
+  uint32_t column_no = invalid_id<>;
   /// Kernel/lambda/function address
   const void *code_ptr_va = nullptr;
   /// Internal bookkeeping slot - do not change.
@@ -451,10 +461,10 @@ struct payload_t {
   //  indicates a partial but valid payload.
   payload_t(const void *codeptr) {
     code_ptr_va = codeptr;
-    name = nullptr;         ///< Invalid name string pointer
-    source_file = nullptr;  ///< Invalid source file string pointer
-    line_no = invalid_id;   ///< Invalid line number
-    column_no = invalid_id; ///< Invalid column number
+    name = nullptr;           ///< Invalid name string pointer
+    source_file = nullptr;    ///< Invalid source file string pointer
+    line_no = invalid_id<>;   ///< Invalid line number
+    column_no = invalid_id<>; ///< Invalid column number
     if (codeptr) {
       flags = (uint64_t)payload_flag_t::CodePointerAvailable;
     }
@@ -517,8 +527,8 @@ struct payload_t {
     /// Capture the rest of the parameters
     name = kname;
     source_file = sf;
-    line_no = line;
-    column_no = col;
+    line_no = static_cast<uint32_t>(line);
+    column_no = static_cast<uint32_t>(col);
     if (kname && kname[0] != '\0') {
       flags = (uint64_t)payload_flag_t::NameAvailable;
     }

xpti/include/xpti/xpti_trace_framework.h

@@ -1018,7 +1018,7 @@ typedef uint16_t (*xpti_register_user_defined_et_t)(const char *, uint8_t);
 typedef xpti::trace_event_data_t *(*xpti_make_event_t)(
     const char *, xpti::payload_t *, uint16_t, xpti::trace_activity_type_t,
     uint64_t *);
-typedef const xpti::trace_event_data_t *(*xpti_find_event_t)(int64_t);
+typedef const xpti::trace_event_data_t *(*xpti_find_event_t)(uint64_t);
 typedef const xpti::payload_t *(*xpti_query_payload_t)(
     xpti::trace_event_data_t *);
 typedef const xpti::payload_t *(*xpti_query_payload_by_uid_t)(uint64_t uid);

xpti/include/xpti/xpti_trace_framework.hpp

@@ -961,9 +961,10 @@ class tracepoint_scope_t {
     MData = const_cast<xpti_tracepoint_t *>(xptiGetTracepointScopeData());
     if (!MData) {
       if (funcName && fileName)
-        init(funcName, fileName, line, column);
+        init(funcName, fileName, static_cast<uint32_t>(line),
+             static_cast<uint32_t>(column));
       else
-        init(callerFuncName, nullptr, 0, 0);
+        init(callerFuncName, nullptr, 0u, 0u);
     } else {
       MTraceEvent = MData->event_ref();
     }

xpti/src/xpti_proxy.cpp

@@ -13,7 +13,7 @@
 #include <unordered_map>
 #include <vector>
 
-enum functions_t {
+enum functions_t : unsigned {
   XPTI_FRAMEWORK_INITIALIZE,
   XPTI_FRAMEWORK_FINALIZE,
   XPTI_INITIALIZE,
@@ -69,7 +69,7 @@ enum functions_t {
 
 namespace xpti {
 class ProxyLoader {
-  std::unordered_map<int, const char *> m_function_names = {
+  std::unordered_map<unsigned, const char *> m_function_names = {
       {XPTI_FRAMEWORK_INITIALIZE, "xptiFrameworkInitialize"},
       {XPTI_FRAMEWORK_FINALIZE, "xptiFrameworkFinalize"},
       {XPTI_INITIALIZE, "xptiInitialize"},
@@ -150,8 +150,8 @@ class ProxyLoader {
 
   inline bool noErrors() { return m_loaded; }
 
-  void *functionByIndex(int index) {
-    if (index >= XPTI_FRAMEWORK_INITIALIZE && index < XPTI_FW_API_COUNT) {
+  void *functionByIndex(unsigned index) {
+    if (index < XPTI_FW_API_COUNT) {
       return reinterpret_cast<void *>(m_dispatch_table[index]);
     }
     return nullptr;
@@ -233,7 +233,7 @@ XPTI_EXPORT_API uint16_t xptiRegisterUserDefinedTracePoint(
       return (*(xpti_register_user_defined_tp_t)f)(tool_name, user_defined_tp);
     }
   }
-  return xpti::invalid_id;
+  return xpti::invalid_id<uint16_t>;
 }
 
 XPTI_EXPORT_API uint16_t xptiRegisterUserDefinedEventType(
@@ -246,7 +246,7 @@ XPTI_EXPORT_API uint16_t xptiRegisterUserDefinedEventType(
                                                    user_defined_event);
     }
   }
-  return xpti::invalid_id;
+  return xpti::invalid_id<uint16_t>;
 }
 
 XPTI_EXPORT_API xpti::result_t xptiInitialize(const char *stream, uint32_t maj,
@@ -278,7 +278,7 @@ XPTI_EXPORT_API uint64_t xptiGetUniversalId() {
       return (*reinterpret_cast<xpti_get_universal_id_t>(f))();
     }
   }
-  return xpti::invalid_id;
+  return xpti::invalid_id<uint64_t>;
 }
 
 XPTI_EXPORT_API void xptiSetUniversalId(uint64_t uid) {
@@ -329,7 +329,7 @@ XPTI_EXPORT_API uint64_t xptiGetUniqueId() {
       return (*(xpti_get_unique_id_t)f)();
     }
   }
-  return xpti::invalid_id;
+  return xpti::invalid_id<uint64_t>;
 }
 
 XPTI_EXPORT_API xpti::string_id_t xptiRegisterString(const char *string,
@@ -341,7 +341,7 @@ XPTI_EXPORT_API xpti::string_id_t xptiRegisterString(const char *string,
       return (*(xpti_register_string_t)f)(string, table_string);
     }
   }
-  return xpti::invalid_id;
+  return xpti::invalid_id<xpti::string_id_t>;
 }
 
 XPTI_EXPORT_API const char *xptiLookupString(xpti::string_id_t id) {
@@ -363,7 +363,7 @@ xptiRegisterObject(const char *data, size_t size, uint8_t type) {
       return (*(xpti_register_object_t)f)(data, size, type);
     }
   }
-  return xpti::invalid_id;
+  return xpti::invalid_id<xpti::object_id_t>;
 }
 
 XPTI_EXPORT_API xpti::object_data_t xptiLookupObject(xpti::object_id_t id) {
@@ -395,7 +395,7 @@ XPTI_EXPORT_API uint8_t xptiRegisterStream(const char *stream_name) {
       return (*(xpti_register_stream_t)f)(stream_name);
     }
   }
-  return xpti::invalid_id;
+  return xpti::invalid_id<uint8_t>;
 }
 
 XPTI_EXPORT_API xpti::result_t xptiUnregisterStream(const char *stream_name) {
@@ -689,7 +689,7 @@ XPTI_EXPORT_API uint8_t xptiGetDefaultStreamID() {
       return (*(xpti_get_default_stream_id_t)f)();
     }
   }
-  return xpti::invalid_id;
+  return xpti::invalid_id<uint8_t>;
 }
 
 XPTI_EXPORT_API xpti::result_t xptiSetDefaultStreamID(uint8_t stream_id) {
@@ -747,4 +747,4 @@ xptiSetDefaultTraceType(xpti::trace_point_type_t trace_type) {
     }
   }
   return xpti::result_t::XPTI_RESULT_FAIL;
-}
+}

xptifw/include/xpti_int64_hash_table.hpp

@@ -63,7 +63,7 @@ class Hash64x64Table {
 #endif
       return KeyLoc->second; // We found it, so we return the Value
     } else
-      return xpti::invalid_id;
+      return xpti::invalid_id<int64_t>;
   }
 
   //  Add a <Key, Value> pair to the hash table. If the Key already exists, this
@@ -121,7 +121,7 @@ class Hash64x64Table {
 #endif
       return ValLoc->second;
     } else
-      return xpti::invalid_id;
+      return xpti::invalid_id<int64_t>;
   }
 
   void printStatistics() {

xptifw/include/xpti_string_table.hpp

@@ -61,15 +61,15 @@ class StringTable {
   // in the string table is returned through the default argument
   xpti::string_id_t add(const char *str, const char **ref_str = nullptr) {
     if (!str)
-      return xpti::invalid_id;
+      return xpti::invalid_id<xpti::string_id_t>;
 
     std::string LocalStr = str;
     return add(LocalStr, ref_str);
   }
 
   xpti::string_id_t add(std::string str, const char **ref_str = nullptr) {
     if (str.empty())
-      return xpti::invalid_id;
+      return xpti::invalid_id<xpti::string_id_t>;
 
     //  Lock-free lookup to see if the string exists in the table; XPTI has
     //  always had this as lock-free, but if instability occurs, we can use a
@@ -126,7 +126,7 @@ class StringTable {
             if (ref_str)
               *ref_str = nullptr;
 
-            return xpti::invalid_id;
+            return xpti::invalid_id<xpti::string_id_t>;
           }
         }
 
@@ -142,7 +142,7 @@ class StringTable {
       }
       // The MMutex will be released here!
     }
-    return xpti::invalid_id;
+    return xpti::invalid_id<xpti::string_id_t>;
   }
 
   //  The reverse query allows one to get the string from the string_id_t that

xptifw/src/xpti_trace_framework.cpp

@@ -204,9 +204,9 @@ struct TracePointImpl : xpti_payload_t,
   /// @brief Event data for the trace point.
   xpti::trace_event_data_t MEvent;
   /// @brief Cached Function string ID for the trace point.
-  int32_t MFuncID = xpti::invalid_id;
+  int32_t MFuncID = xpti::invalid_id<>;
   /// @brief Cached File string ID for the trace point.
-  int32_t MFileID = xpti::invalid_id;
+  int32_t MFileID = xpti::invalid_id<>;
   /// @brief Iterator for the metadata associated with the trace point.
   xpti::metadata_t::iterator MCurr;
 
@@ -944,7 +944,7 @@ class Tracepoints {
       return xpti::result_t::XPTI_RESULT_INVALIDARG;
 
     string_id_t KeyID = MStringTableRef.add(Key);
-    if (KeyID == xpti::invalid_id) {
+    if (KeyID == xpti::invalid_id<string_id_t>) {
       return xpti::result_t::XPTI_RESULT_INVALIDARG;
     }
 
@@ -2115,7 +2115,7 @@ class Framework {
 
   string_id_t registerString(const char *String, char **TableString) {
     if (!TableString || !String)
-      return xpti::invalid_id;
+      return xpti::invalid_id<string_id_t>;
 
     *TableString = 0;
 
@@ -2134,7 +2134,7 @@ class Framework {
 
   object_id_t registerObject(const char *Object, size_t Size, uint8_t Type) {
     if (!Object)
-      return xpti::invalid_id;
+      return xpti::invalid_id<object_id_t>;
 
     return MObjectTable.insert(std::string_view(Object, Size), Type);
   }