address reviews

Author: bcardosolopes

mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp

@@ -378,9 +378,9 @@ TargetFeaturesAttr TargetFeaturesAttr::featuresAt(Operation *op) {
 
 LogicalResult
 ModuleFlagAttr::verify(function_ref<InFlightDiagnostic()> emitError,
-                       mlir::LLVM::ModFlagBehavior flagBehavior,
-                       mlir::StringAttr key, mlir::Attribute value) {
-  if (!isa<mlir::IntegerAttr, mlir::StringAttr>(value))
+                       LLVM::ModFlagBehavior flagBehavior, StringAttr key,
+                       Attribute value) {
+  if (!isa<IntegerAttr, StringAttr>(value))
     return emitError()
            << "only integer and string values are currently supported";
   return success();

mlir/lib/Target/LLVMIR/ModuleImport.cpp

@@ -533,10 +533,8 @@ LogicalResult ModuleImport::convertModuleFlagsMetadata() {
     } else {
       emitWarning(mlirModule.getLoc())
           << "unsupported module flag value: " << diagMD(val, llvmModule.get());
-    }
-
-    if (!valAttr)
       continue;
+    }
 
     moduleFlags.push_back(builder.getAttr<ModuleFlagAttr>(
         convertModFlagBehaviorFromLLVM(behavior),

mlir/test/Dialect/LLVMIR/invalid.mlir

@@ -1778,7 +1778,7 @@ llvm.mlir.alias external @y5 : i32 {
 module {
   llvm.func @foo()
 
-  // expected-error@+1 {{only integer and string values are currently supported}}
+  // expected-error@below {{only integer and string values are currently supported}}
   llvm.module_flags [#llvm.mlir.module_flag<error, "yolo", @foo>]
 }
 

mlir/test/Target/LLVMIR/Import/module-flags.ll

@@ -19,9 +19,9 @@
 ; CHECK-SAME: #llvm.mlir.module_flag<override, "probe-stack", "inline-asm">]
 
 ; // -----
-; expected-warning@-2{{unsupported module flag value: !4 = !{!"foo", i32 1}}}
-!0 = !{ i32 1, !"foo", i32 1 }
-!1 = !{ i32 4, !"bar", i32 37 }
-!2 = !{ i32 2, !"qux", i32 42 }
-!3 = !{ i32 3, !"qux", !{ !"foo", i32 1 }}
-!llvm.module.flags = !{ !0, !1, !2, !3 }
+; expected-warning@-2 {{unsupported module flag value: !4 = !{!"foo", i32 1}}}
+!10 = !{ i32 1, !"foo", i32 1 }
+!11 = !{ i32 4, !"bar", i32 37 }
+!12 = !{ i32 2, !"qux", i32 42 }
+!13 = !{ i32 3, !"qux", !{ !"foo", i32 1 }}
+!llvm.module.flags = !{ !10, !11, !12, !13 }