Re-enable diagnostic error/warning printing (#3020)

Signed-off-by: Alexandre Eichenberger <[email protected]>
Co-authored-by: Tung D. Le <[email protected]>

Author: AlexandreEichenberger

src/Compiler/CompilerUtils.cpp

@@ -62,6 +62,7 @@ namespace onnx_mlir {
 // Values to report the current phase of compilation.
 uint64_t CURRENT_COMPILE_PHASE = 1;
 uint64_t TOTAL_COMPILE_PHASE = 0;
+static DiagnosticEngine::HandlerID diagnosticHandlerID = 0;
 
 // Make a function that forces preserving all files using the runtime arguments
 // and/or the overridePreserveFiles enum.
@@ -420,6 +421,12 @@ static int genLLVMBitcode(const mlir::OwningOpRef<ModuleOp> &module,
     return InvalidTemporaryFileAccess;
   }
 
+  // In the LLVM translation, we get some warnings, so disable in non-verbose
+  // mode.
+  if (diagnosticHandlerID && !VerboseOutput) {
+    module.get().getContext()->getDiagEngine().eraseHandler(
+        diagnosticHandlerID);
+  }
   llvm::LLVMContext llvmContext;
   mlir::registerBuiltinDialectTranslation(*(module.get().getContext()));
   mlir::registerLLVMDialectTranslation(*(module.get().getContext()));
@@ -995,6 +1002,17 @@ int compileModule(mlir::OwningOpRef<ModuleOp> &module,
 
   configurePasses();
 
+  // Enable printing for error handler on llvm error stream. Save ID if we want
+  // to disable it later. We currently disable for the llvm lowering, as
+  // otherwise we currently get an unrecognized warning for the "onnx.name"
+  // attribute in function operations. In Verbose mode, we keep the error
+  // handling all the way to the end.
+  diagnosticHandlerID =
+      context.getDiagEngine().registerHandler([](Diagnostic &diag) {
+        llvm::errs() << diag << "\n";
+        return mlir::LogicalResult::success();
+      });
+
   mlir::PassManager pm(
       module.get()->getName(), mlir::OpPassManager::Nesting::Implicit);
   // TODO(tung): Revise adding passes. The current mechanism does not work if

src/Support/Diagnostic.cpp

@@ -19,19 +19,22 @@ using namespace mlir;
 namespace onnx_mlir {
 
 template <typename T>
-LogicalResult Diagnostic::emitAttributeOutOfRangeError(Operation &op,
-    const llvm::Twine &attrName, T attrVal, Range<T> validRange) {
+LogicalResult Diagnostic::emitAttributeOutOfRangeError(
+    Operation &op, const llvm::Twine &attrName, T attrVal, Range<T> range) {
   static_assert(std::is_arithmetic<T>::value, "Expecting an arithmetic type");
 
   llvm::Twine msg(op.getName().getStringRef() + ": ");
+  std::string rangeMessage =
+      range.isValid() ? "" : " <<Warning, ill-formed range>>";
   return emitError(op.getLoc(), msg.concat("'" + attrName + "'")
                                     .concat(" value is ")
                                     .concat(std::to_string(attrVal))
                                     .concat(", accepted range is [")
-                                    .concat(std::to_string(validRange.min))
+                                    .concat(std::to_string(range.min))
                                     .concat(", ")
-                                    .concat(std::to_string(validRange.max))
-                                    .concat("]"));
+                                    .concat(std::to_string(range.max))
+                                    .concat("]")
+                                    .concat(rangeMessage));
 }
 
 template <typename T>

src/Support/Diagnostic.hpp

@@ -36,15 +36,22 @@ class Diagnostic {
     T max;
 
   public:
+    // Range is used in error situations, so having an assert is not very useful
+    // as that assert may crash the program instead of reporting the error
+    // condition. New approach is to report the error with an additional
+    // warning.
     Range(T min, T max) : min(min), max(max) {
-      assert(min <= max && "Illegal range");
+      if (!isValid())
+        llvm::errs() << "Warning: badly formed range(min=" << min
+                     << ", max=" << max << ")\n";
     }
+    bool isValid() { return min <= max; }
   };
 
   /// Diagnostic message for attribute value outside of a supplied range.
   template <typename T>
   static mlir::LogicalResult emitAttributeOutOfRangeError(mlir::Operation &op,
-      const llvm::Twine &attrName, T attrVal, Range<T> validRange);
+      const llvm::Twine &attrName, T attrVal, Range<T> range);
 
   /// Verifies whether 2 inputs have the same rank.
   template <typename T>