[flang-commits] [flang] [mlir] [MLIR][LLVMIR][DLTI] Add #llvm.target, #llvm.data_layout and TargetAttrInterface (PR #145899)

[Rolf Morel via flang-commits]

================
@@ -404,3 +408,73 @@ ModuleFlagAttr::verify(function_ref<InFlightDiagnostic()> emitError,
                         "supported for unknown key '"
                      << key << "'";
 }
+
+FailureOr<Attribute> TargetFeaturesAttr::query(DataLayoutEntryKey key) {
+  if (auto stringKey = dyn_cast<StringAttr>(key))
+    if (contains(stringKey))
+      return UnitAttr::get(getContext());
+  return failure();
+}
+
+//===----------------------------------------------------------------------===//
+// LLVM_TargetAttr
+//===----------------------------------------------------------------------===//
+
+FailureOr<llvm::TargetMachine *> TargetAttr::getTargetMachine() {
+  if (targetMachine.has_value()) {
+    llvm::TargetMachine *tm = targetMachine.value();
+    if (tm != nullptr)
+      return {tm};
+    return failure();
+  }
+  llvm::InitializeAllTargets();
+  llvm::InitializeAllTargetMCs();
+
+  std::string error;
+  const llvm::Target *target =
+      llvm::TargetRegistry::lookupTarget(getTriple(), error);
+  if (!error.empty()) {
+    LLVM_DEBUG({
+      llvm::dbgs() << "Failed to retrieve the target with: `" << error << "`\n";
+    });
+    targetMachine = {nullptr};
+    return failure();
+  }
+
+  targetMachine = {target->createTargetMachine(
----------------
rolfmorel wrote:

I remembered why I didn't do it before: wrapping `TargetMachine` in a unique_ptr makes ODS-generated code, in particular a printer, scream:
```
In file included from /data/nfs_home/rmorel/llvm-project.git/llvm-target-dlti/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp:20:
/data/nfs_home/rmorel/llvm-project.git/llvm-target-dlti/llvm/include/llvm/ADT/TypeSwitch.h:102:29: error: call to implicitly-deleted copy constructor of 'mlir::LLVM::TargetAttr'
      result.emplace(caseFn(caseValue));
                            ^~~~~~~~~
tools/mlir/include/mlir/Dialect/LLVMIR/LLVMOpsAttrDefs.cpp.inc:372:80: note: in instantiation of function template specialization 'llvm::TypeSwitch<mlir::Attribute, llvm::LogicalResult>::Case<mlir::LLVM::TargetAttr, (lambda at tools/mlir/include/mlir/Dialect/LLVMIR/LLVMOpsAttrDefs.cpp.inc:372:111)>' requested here
  return ::llvm::TypeSwitch<::mlir::Attribute, ::llvm::LogicalResult>(def)    .Case<::mlir::LLVM::TargetAttr>([&](auto t) {
                                                                               ^
tools/mlir/include/mlir/Dialect/LLVMIR/LLVMOpsAttrDefs.h.inc:306:55: note: copy constructor of 'TargetAttr' is implicitly deleted because field 'targetMachine' has a deleted copy constructor
  std::optional<std::unique_ptr<llvm::TargetMachine>> targetMachine = std::nullopt;
                                                      ^
/opt/rh/gcc-toolset-13/root/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/optional:707:7: note: copy constructor of 'optional<std::unique_ptr<llvm::TargetMachine>>' is implicitly deleted because base class '_Enable_copy_move<is_copy_constructible_v<unique_ptr<TargetMachine, default_delete<TargetMachine>>>, __and_v<is_copy_constructible<unique_ptr<TargetMachine, default_delete<TargetMachine>>>, is_copy_assignable<unique_ptr<TargetMachine, default_delete<TargetMachine>>>>, is_move_constructible_v<unique_ptr<TargetMachine, default_delete<TargetMachine>>>, __and_v<is_move_constructible<unique_ptr<TargetMachine, default_delete<TargetMachine>>>, is_move_assignable<unique_ptr<TargetMachine, default_delete<TargetMachine>>>>, optional<unique_ptr<TargetMachine, default_delete<TargetMachine>>>>' has a deleted copy constructor
      private _Enable_copy_move<
      ^
/opt/rh/gcc-toolset-13/root/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/enable_special_members.h:160:15: note: '_Enable_copy_move' has been explicitly marked deleted here
    constexpr _Enable_copy_move(_Enable_copy_move const&) noexcept  = delete;
              ^
tools/mlir/include/mlir/Dialect/LLVMIR/LLVMOpsAttrDefs.cpp.inc:372:120: note: passing argument to parameter 't' here
  return ::llvm::TypeSwitch<::mlir::Attribute, ::llvm::LogicalResult>(def)    .Case<::mlir::LLVM::TargetAttr>([&](auto t) {
                                                                                                                       ^
1 error generated.
```

This is with `std::optional<std::unique_ptr<llvm::TargetMachine>> targetMachine = std::nullopt;` in the `extraClassDeclaration` and 
```
  auto targetMach = std::unique_ptr<llvm::TargetMachine>(target->createTargetMachine(
          llvm::Triple(getTriple().strref()), getChip().strref(),
          getFeatures().getFeaturesString().c_str(), {}, {}));

  targetMachine = { std::move(targetMach) };
```
in the implementation. 

If I am understanding correctly, the issue seems to be that the type switch in the following code is trying to do a copy of the attribute, even though attributes should just be thought of/treated as (unique) pointers (... if my understanding is correct):
```
static ::llvm::LogicalResult generatedAttributePrinter(::mlir::Attribute def, ::mlir::AsmPrinter &printer) {
  return ::llvm::TypeSwitch<::mlir::Attribute, ::llvm::LogicalResult>(def)    .Case<::mlir::LLVM::TargetAttr>([&](auto t) {
      printer << ::mlir::LLVM::TargetAttr::getMnemonic();
t.print(printer);
      return ::mlir::success();
    })
    .Case<::mlir::LLVM::DataLayoutAttr>([&](auto t) {
      printer << ::mlir::LLVM::DataLayoutAttr::getMnemonic();
t.print(printer);
      return ::mlir::success();
    })
    .Case<::mlir::LLVM::CConvAttr>([&](auto t) 
    ...
```

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