Fix PR compile error with WinML which does not seem to support dynami…

…c_cast and RTTI. Instead use a custom type system on ISink that is safe for use with static_cast
microsoft · May 2, 2024 · 7fdf2f5 · 7fdf2f5
1 parent 2eec678
commit 7fdf2f5
Show file tree

Hide file tree

Showing 5 changed files with 16 additions and 8 deletions.
diff --git a/include/onnxruntime/core/common/logging/isink.h b/include/onnxruntime/core/common/logging/isink.h
@@ -13,6 +13,11 @@ class ISink {
  public:
   ISink() = default;
 
+  enum SinkType { BaseSink,
+                  CompositeSink,
+                  EtwSink };
+  virtual SinkType GetType() const { return BaseSink; }
+
   /**
      Sends the message to the sink.
      @param timestamp The timestamp.

diff --git a/onnxruntime/core/common/logging/logging.cc b/onnxruntime/core/common/logging/logging.cc
@@ -291,13 +291,11 @@ void LoggingManager::AddEtwSink(logging::Severity etwSeverity) {
     return;  // ETW not enabled, no operation needed
   }
 
-  CompositeSink* current_composite = dynamic_cast<CompositeSink*>(sink_.get());
-  if (!current_composite) {
+  if (sink_->GetType() != ISink::CompositeSink) {
     // Current sink is not a composite, create a new composite sink and add the current sink to it
     auto new_composite = std::make_unique<CompositeSink>();
     new_composite->AddSink(std::move(sink_), default_min_severity_);  // Move the current sink into the new composite
     sink_ = std::move(new_composite);                                 // Now sink_ is pointing to the new composite
-    current_composite = static_cast<CompositeSink*>(sink_.get());     // Update pointer to the new composite sink
   }
 
   // Adjust the default minimum severity level to accommodate ETW logging needs
@@ -306,10 +304,11 @@ void LoggingManager::AddEtwSink(logging::Severity etwSeverity) {
     s_default_logger_->SetSeverity(default_min_severity_);
   }
 
+  CompositeSink* current_composite = static_cast<CompositeSink*>(sink_.get());
   // Check if an EtwSink already exists in the current composite
   const auto& sinks = current_composite->GetSinks();
   if (std::any_of(sinks.begin(), sinks.end(), [](const auto& pair) {
-        return dynamic_cast<EtwSink*>(pair.first.get()) != nullptr;
+        return pair.first->GetType() == ISink::EtwSink;
       })) {
     return;  // EtwSink already exists, do not add another
   }
@@ -320,16 +319,16 @@ void LoggingManager::AddEtwSink(logging::Severity etwSeverity) {
 
 void LoggingManager::RemoveEtwSink() {
   std::lock_guard<OrtMutex> guard(sink_mutex_);
-  auto composite_sink = dynamic_cast<CompositeSink*>(sink_.get());
 
-  if (composite_sink) {
+  if (sink_->GetType() == ISink::CompositeSink) {
+    auto composite_sink = static_cast<CompositeSink*>(sink_.get());
     const auto& sinks_with_severity = composite_sink->GetSinks();
     std::unique_ptr<ISink> remaining_sink;
 
     Severity newSeverity = Severity::kFATAL;
 
     for (const auto& sink_pair : sinks_with_severity) {
-      if (dynamic_cast<EtwSink*>(sink_pair.first.get()) == nullptr) {
+      if (sink_pair.first->GetType() != ISink::EtwSink) {
         if (remaining_sink) {
           // If more than one non-EtwSink is found, we leave the CompositeSink intact
           return;

diff --git a/onnxruntime/core/common/logging/sinks/composite_sink.h b/onnxruntime/core/common/logging/sinks/composite_sink.h
@@ -25,6 +25,8 @@ class CompositeSink : public ISink {
   /// </summary>
   CompositeSink() {}
 
+  SinkType GetType() const override { return ISink::CompositeSink; }
+
   /// <summary>
   /// Adds a sink. Takes ownership of the sink (so pass unique_ptr by value).
   /// </summary>

diff --git a/onnxruntime/core/platform/windows/logging/etw_sink.h b/onnxruntime/core/platform/windows/logging/etw_sink.h
@@ -34,6 +34,8 @@ class EtwSink : public ISink {
   EtwSink() = default;
   ~EtwSink() = default;
 
+  SinkType GetType() const override { return ISink::EtwSink; }
+
   constexpr static const char* kEventName = "ONNXRuntimeLogEvent";
 
  private:

diff --git a/onnxruntime/core/providers/qnn/builder/qnn_backend_manager.cc b/onnxruntime/core/providers/qnn/builder/qnn_backend_manager.cc
@@ -317,7 +317,7 @@ QnnLog_Level_t QnnBackendManager::MapOrtSeverityToQNNLogLevel(logging::Severity
 
 Status QnnBackendManager::ResetQnnLogLevel() {
   auto ort_log_level = logger_->GetSeverity();
-  LOGS(*logger_, INFO) << "Reset Qnn log level to ORT Logger level: " << (UINT)ort_log_level;
+  LOGS(*logger_, INFO) << "Reset Qnn log level to ORT Logger level: " << (unsigned int)ort_log_level;
   return UpdateQnnLogLevel(ort_log_level);
 }