address reviews

Author: bcardosolopes

mlir/include/mlir/Dialect/LLVMIR/LLVMDialect.td

@@ -88,6 +88,11 @@ def LLVM_Dialect : Dialect {
       return "llvm.dependent_libraries";
     }
 
+    /// Names of known llvm module flag keys.
+    static StringRef getModuleFlagKeyCGProfileName() {
+      return "CG Profile";
+    }
+
     /// Returns `true` if the given type is compatible with the LLVM dialect.
     static bool isCompatibleType(Type);
 

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

@@ -380,10 +380,10 @@ LogicalResult
 ModuleFlagAttr::verify(function_ref<InFlightDiagnostic()> emitError,
                        LLVM::ModFlagBehavior flagBehavior, StringAttr key,
                        Attribute value) {
-  if (key == "CG Profile") {
+  if (key == LLVM::LLVMDialect::getModuleFlagKeyCGProfileName()) {
     auto arrayAttr = dyn_cast<ArrayAttr>(value);
-    if ((!arrayAttr) || (!llvm::all_of(arrayAttr, [](Attribute v) {
-          return isa<ModuleFlagCGProfileEntryAttr>(v);
+    if ((!arrayAttr) || (!llvm::all_of(arrayAttr, [](Attribute attr) {
+          return isa<ModuleFlagCGProfileEntryAttr>(attr);
         })))
       return emitError()
              << "'CG Profile' key expects an array of '#llvm.cgprofile_entry'";

mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.cpp

@@ -279,7 +279,7 @@ convertModuleFlagValue(StringRef key, ArrayAttr arrayAttr,
   llvm::MDBuilder mdb(context);
   SmallVector<llvm::Metadata *> nodes;
 
-  if (key == "CG Profile") {
+  if (key == LLVM::LLVMDialect::getModuleFlagKeyCGProfileName()) {
     for (auto entry : arrayAttr.getAsRange<ModuleFlagCGProfileEntryAttr>()) {
       llvm::Function *fromFn =
           moduleTranslation.lookupFunction(entry.getFrom().getValue());

mlir/lib/Target/LLVMIR/ModuleImport.cpp

@@ -531,7 +531,7 @@ static Attribute convertCGProfileModuleFlagValue(ModuleOp mlirModule,
   SmallVector<Attribute> cgProfile;
   for (unsigned i = 0; i < mdTuple->getNumOperands(); i++) {
     const llvm::MDOperand &mdo = mdTuple->getOperand(i);
-    auto *cgEntry = dyn_cast_or_null<llvm::MDNode>(mdo);
+    auto *cgEntry = cast<llvm::MDNode>(mdo);
     llvm::Constant *llvmConstant =
         cast<llvm::ConstantAsMetadata>(cgEntry->getOperand(2))->getValue();
     uint64_t count = cast<llvm::ConstantInt>(llvmConstant)->getZExtValue();
@@ -542,12 +542,12 @@ static Attribute convertCGProfileModuleFlagValue(ModuleOp mlirModule,
   return ArrayAttr::get(mlirModule->getContext(), cgProfile);
 }
 
-// Invoke specific handlers for each known module flag value, returns nullptr if
-// the key is unknown or unimplemented.
+/// Invoke specific handlers for each known module flag value, returns nullptr
+/// if the key is unknown or unimplemented.
 static Attribute convertModuleFlagValueFromMDTuple(ModuleOp mlirModule,
                                                    StringRef key,
                                                    llvm::MDTuple *mdTuple) {
-  if (key == "CG Profile")
+  if (key == LLVM::LLVMDialect::getModuleFlagKeyCGProfileName())
     return convertCGProfileModuleFlagValue(mlirModule, mdTuple);
   return nullptr;
 }

mlir/test/Dialect/LLVMIR/module-roundtrip.mlir

@@ -8,9 +8,9 @@ module {
                      #llvm.mlir.module_flag<max, "frame-pointer", 1 : i32>,
                      #llvm.mlir.module_flag<override, "probe-stack", "inline-asm">,
                      #llvm.mlir.module_flag<append, "CG Profile", [
-                      #llvm.cgprofile_entry<from = @from, to = @to, count = 222>,
-                      #llvm.cgprofile_entry<from = @from, to = @from, count = 222>,
-                      #llvm.cgprofile_entry<from = @to, to = @from, count = 222>
+                       #llvm.cgprofile_entry<from = @from, to = @to, count = 222>,
+                       #llvm.cgprofile_entry<from = @from, to = @from, count = 222>,
+                       #llvm.cgprofile_entry<from = @to, to = @from, count = 222>
                     ]>]
 }
 

mlir/test/Target/LLVMIR/llvmir.mlir

@@ -2838,15 +2838,13 @@ module {
 
 // -----
 
-module {
-  llvm.module_flags [#llvm.mlir.module_flag<append, "CG Profile", [
-    #llvm.cgprofile_entry<from = @from, to = @to, count = 222>,
-    #llvm.cgprofile_entry<from = @from, to = @from, count = 222>,
-    #llvm.cgprofile_entry<from = @to, to = @from, count = 222>
-  ]>]
-  llvm.func @from(i32)
-  llvm.func @to()
-}
+llvm.module_flags [#llvm.mlir.module_flag<append, "CG Profile", [
+  #llvm.cgprofile_entry<from = @from, to = @to, count = 222>,
+  #llvm.cgprofile_entry<from = @from, to = @from, count = 222>,
+  #llvm.cgprofile_entry<from = @to, to = @from, count = 222>
+]>]
+llvm.func @from(i32)
+llvm.func @to()
 
 // CHECK: !llvm.module.flags = !{![[#CGPROF:]], ![[#DBG:]]}