[Mlir-commits] [mlir] [MLIR][LLVM] Allow strings in module flag value (PR #136793)

[Tobias Gysi]

================
@@ -273,10 +273,18 @@ static void convertLinkerOptionsOp(ArrayAttr options,
 static void convertModuleFlagsOp(ArrayAttr flags, llvm::IRBuilderBase &builder,
                                  LLVM::ModuleTranslation &moduleTranslation) {
   llvm::Module *llvmModule = moduleTranslation.getLLVMModule();
-  for (auto flagAttr : flags.getAsRange<ModuleFlagAttr>())
-    llvmModule->addModuleFlag(
-        convertModFlagBehaviorToLLVM(flagAttr.getBehavior()),
-        flagAttr.getKey().getValue(), flagAttr.getValue());
+  for (auto flagAttr : flags.getAsRange<ModuleFlagAttr>()) {
+    if (auto intAttr = dyn_cast<mlir::IntegerAttr>(flagAttr.getValue()))
+      llvmModule->addModuleFlag(
+          convertModFlagBehaviorToLLVM(flagAttr.getBehavior()),
+          flagAttr.getKey().getValue(), intAttr.getUInt());
+    else if (auto strAttr = dyn_cast<mlir::StringAttr>(flagAttr.getValue())) {
----------------
gysit wrote:

```suggestion
    if (auto intAttr = dyn_cast<IntegerAttr>(flagAttr.getValue())) {
      llvmModule->addModuleFlag(
          convertModFlagBehaviorToLLVM(flagAttr.getBehavior()),
          flagAttr.getKey().getValue(), intAttr.getUInt());
    } else if (auto strAttr = dyn_cast<StringAttr>(flagAttr.getValue())) {
```
nit: braces around the if block seem to be missing. I wonder if it would make sense to use a type switch here and/or cast to a string attribute in the else block. We should never see something else than an integer or string here?

https://github.com/llvm/llvm-project/pull/136793