[Mlir-commits] [flang] [mlir] [mlir][func] Refactor FuncToLLVM discardable attributes algorithm (PR #188232)

[Hocky Yudhiono]

================
@@ -59,19 +68,114 @@ static bool shouldUseBarePtrCallConv(Operation *op,
          typeConverter->getOptions().useBarePtrCallConv;
 }
 
+static bool isDiscardableAttr(StringRef name) {
+  return name == linkageAttrName || name == varargsAttrName ||
+         name == LLVM::LLVMDialect::getReadnoneAttrName();
+}
+
 /// Only retain those attributes that are not constructed by
 /// `LLVMFuncOp::build`.
 static void filterFuncAttributes(FunctionOpInterface func,
                                  SmallVectorImpl<NamedAttribute> &result) {
   for (const NamedAttribute &attr : func->getDiscardableAttrs()) {
-    if (attr.getName() == linkageAttrName ||
-        attr.getName() == varargsAttrName ||
-        attr.getName() == LLVM::LLVMDialect::getReadnoneAttrName())
+    if (isDiscardableAttr(attr.getName().strref()))
       continue;
     result.push_back(attr);
   }
 }
 
+/// Add custom lowered funcOp to llvm.func attributes here.
+struct LoweredFuncAttrs {
+  LLVM::Linkage linkage = LLVM::Linkage::External;
+  bool hasReadnone = false;
+  SmallVector<NamedAttribute, 4> attrs;
+};
+
+static LogicalResult lowerLinkageAttr(FunctionOpInterface func,
+                                      Attribute attrValue,
+                                      LoweredFuncAttrs &lowered) {
+  auto linkageAttr = dyn_cast<mlir::LLVM::LinkageAttr>(attrValue);
+  if (!linkageAttr) {
+    func->emitError() << "Contains " << linkageAttrName
+                      << " attribute not of type LLVM::LinkageAttr";
+    return failure();
+  }
+  lowered.linkage = linkageAttr.getLinkage();
+  return success();
+}
+
+static LogicalResult lowerReadnoneAttr(FunctionOpInterface func,
+                                       Attribute attrValue,
+                                       LoweredFuncAttrs &lowered) {
+  StringRef readnoneAttrName = LLVM::LLVMDialect::getReadnoneAttrName();
+  if (!isa<UnitAttr>(attrValue)) {
+    func->emitError() << "Contains " << readnoneAttrName
+                      << " attribute not of type UnitAttr";
+    return failure();
+  }
+  lowered.hasReadnone = true;
+  return success();
+}
+
+/// Lower discardable function attributes on `func.func` to attributes expected
+/// by `llvm.func`.
+static FailureOr<LoweredFuncAttrs>
+lowerFuncAttributes(FunctionOpInterface func) {
+  MLIRContext *ctx = func->getContext();
+  LoweredFuncAttrs lowered;
+  llvm::SmallDenseSet<StringRef> odsAttrNames(
+      LLVM::LLVMFuncOp::getAttributeNames().begin(),
+      LLVM::LLVMFuncOp::getAttributeNames().end());
+
+  // Obtain specific attributes and add them to the lowered attributes.
+  for (const NamedAttribute &attr : func->getDiscardableAttrs()) {
+    StringRef attrName = attr.getName().strref();
+    if (attrName == linkageAttrName) {
+      if (failed(lowerLinkageAttr(func, attr.getValue(), lowered)))
+        return failure();
+      continue;
+    }
+
+    // TODO: discardable attributes should not be used again after this lowering
+    if (attrName == LLVM::LLVMDialect::getEmitCWrapperAttrName()) {
+      lowered.attrs.emplace_back(attr);
+      continue;
+    }
+
+    if (attrName == barePtrAttrName) {
+      lowered.attrs.emplace_back(attr);
+      continue;
+    }
+
+    if (attrName == LLVM::LLVMDialect::getReadnoneAttrName()) {
+      if (failed(lowerReadnoneAttr(func, attr.getValue(), lowered)))
+        return failure();
+      continue;
+    }
+
+    // TODO: this is very breaking, make sure all inherent attributes have
+    // `llvm.*` prefix
+    if (odsAttrNames.contains(attrName)) {
+      continue;
+    }
+
+    // Map llvm.<name> -> inherent <name> when <name> is an LLVMFuncOp ODS attr.
+    StringRef inherent = attrName;
+    LDBG() << "inherent: " << inherent;
+    if (inherent.consume_front("llvm.")) {
+      if (odsAttrNames.contains(inherent)) {
+        LDBG() << "inserting to attrs: " << inherent;
+        lowered.attrs.emplace_back(StringAttr::get(ctx, inherent),
+                                   attr.getValue());
+      }
+    } else {
+      lowered.attrs.push_back(attr);
----------------
hockyy wrote:

> Why are we dropping the llvm.xxxx ones that don't match above?
My plan is to implement this idea from the discourse 

https://discourse.llvm.org/t/confusion-regarding-llvm-linkage-attribute-should-it-be-linkage-or-llvm-linkage/89497/2

> In theory we should reject all discardable attribute that don’t have a dialect prefix, we just never implemented it unfortunately :frowning:


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