Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Flang][MLIR][OpenMP] Remove the early outlining interface #78450

Merged
merged 1 commit into from
Jan 18, 2024

Conversation

skatrak
Copy link
Contributor

@skatrak skatrak commented Jan 17, 2024

After the removal of the OpenMP early outlining MLIR pass in #67319, the EarlyOutliningInterface stopped doing any useful work. It used to be necessary to tie the name of the function from which a target region was outlined to that new function, so it would be used when translating to LLVM IR in place of the outlined function's name.

This is not necessary anymore, so this patch removes all references to this interface and uses of the omp.outline_parent_name discardable attribute in tests.

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 17, 2024

@llvm/pr-subscribers-mlir
@llvm/pr-subscribers-flang-openmp
@llvm/pr-subscribers-mlir-openmp

@llvm/pr-subscribers-flang-fir-hlfir

Author: Sergio Afonso (skatrak)

Changes

After the removal of the OpenMP early outlining MLIR pass in #67319, the EarlyOutliningInterface stopped doing any useful work. It used to be necessary to tie the name of the function from which a target region was outlined to that new function, so it would be used when translating to LLVM IR in place of the outlined function's name.

This is not necessary anymore, so this patch removes all references to this interface and uses of the omp.outline_parent_name discardable attribute in tests.


Full diff: https://github.com/llvm/llvm-project/pull/78450.diff

9 Files Affected:

  • (modified) flang/lib/Optimizer/Transforms/ExternalNameConversion.cpp (-16)
  • (modified) flang/test/Fir/external-mangling.fir (-9)
  • (modified) mlir/include/mlir/Dialect/OpenMP/OpenMPInterfaces.h (-5)
  • (modified) mlir/include/mlir/Dialect/OpenMP/OpenMPOpsInterfaces.td (-40)
  • (modified) mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp (-7)
  • (modified) mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp (-7)
  • (modified) mlir/test/Dialect/OpenMP/attr.mlir (-8)
  • (modified) mlir/test/Target/LLVMIR/omptarget-parallel-llvm.mlir (+2-2)
  • (removed) mlir/test/Target/LLVMIR/omptarget-region-llvm-target-device.mlir (-21)
diff --git a/flang/lib/Optimizer/Transforms/ExternalNameConversion.cpp b/flang/lib/Optimizer/Transforms/ExternalNameConversion.cpp
index bc5be3f196b81a..3a9686418c2eae 100644
--- a/flang/lib/Optimizer/Transforms/ExternalNameConversion.cpp
+++ b/flang/lib/Optimizer/Transforms/ExternalNameConversion.cpp
@@ -44,20 +44,6 @@ mangleExternalName(const std::pair<fir::NameUniquer::NameKind,
                                                   appendUnderscore);
 }
 
-/// Update the early outlining parent name
-void updateEarlyOutliningParentName(mlir::func::FuncOp funcOp,
-                                    bool appendUnderscore) {
-  if (auto earlyOutlineOp = llvm::dyn_cast<mlir::omp::EarlyOutliningInterface>(
-          funcOp.getOperation())) {
-    auto oldName = earlyOutlineOp.getParentName();
-    if (oldName != "") {
-      auto dName = fir::NameUniquer::deconstruct(oldName);
-      std::string newName = mangleExternalName(dName, appendUnderscore);
-      earlyOutlineOp.setParentName(newName);
-    }
-  }
-}
-
 //===----------------------------------------------------------------------===//
 // Rewrite patterns
 //===----------------------------------------------------------------------===//
@@ -93,8 +79,6 @@ struct MangleNameOnFuncOp : public mlir::OpRewritePattern<mlir::func::FuncOp> {
       op->setAttr(fir::getInternalFuncNameAttrName(),
                   mlir::StringAttr::get(op->getContext(), oldName));
     }
-
-    updateEarlyOutliningParentName(op, appendUnderscore);
     rewriter.finalizeOpModification(op);
     return ret;
   }
diff --git a/flang/test/Fir/external-mangling.fir b/flang/test/Fir/external-mangling.fir
index 06e1e2e412295c..68e3ca0b098411 100644
--- a/flang/test/Fir/external-mangling.fir
+++ b/flang/test/Fir/external-mangling.fir
@@ -89,12 +89,3 @@ func.func @_QPcaller() {
 
 // LLVMIR-NOUNDER: llvm.call @callee() : () -> ()
 // LLVMIR-NOUNDER: llvm.call @callee() : () -> ()
-
-// -----
-
-func.func @_QPwriteindex_omp_outline_0() attributes {omp.outline_parent_name = "_QPwriteindex"} {
-   return
-}
-
-// CHECK-UNDER: attributes {fir.internal_name = "_QPwriteindex_omp_outline_0", omp.outline_parent_name = "writeindex_"}
-// CHECK-NOUNDER: attributes {fir.internal_name = "_QPwriteindex_omp_outline_0", omp.outline_parent_name = "writeindex"}
diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPInterfaces.h b/mlir/include/mlir/Dialect/OpenMP/OpenMPInterfaces.h
index d78c541252a98d..b3184db8852161 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPInterfaces.h
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPInterfaces.h
@@ -36,11 +36,6 @@ struct DeclareTargetDefaultModel
     : public DeclareTargetInterface::ExternalModel<DeclareTargetDefaultModel<T>,
                                                    T> {};
 
-template <typename T>
-struct EarlyOutliningDefaultModel
-    : public EarlyOutliningInterface::ExternalModel<
-          EarlyOutliningDefaultModel<T>, T> {};
-
 } // namespace mlir::omp
 
 #endif // MLIR_DIALECT_OPENMP_OPENMPINTERFACES_H_
diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPOpsInterfaces.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPOpsInterfaces.td
index 89d04af64766fc..18c5335c63fb9b 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOpsInterfaces.td
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOpsInterfaces.td
@@ -288,44 +288,4 @@ def OffloadModuleInterface : OpInterface<"OffloadModuleInterface"> {
   ];
 }
 
-def EarlyOutliningInterface : OpInterface<"EarlyOutliningInterface"> {
-  let description = [{
-    FuncOps that are a result of early outlining should have this interface.
-  }];
-
-  let cppNamespace = "::mlir::omp";
-
-  let methods = [
-    InterfaceMethod<
-      /*description=*/[{
-        Set a StringAttr on an outlined target region function containing the
-        name of the parent function where the target region was outlined
-        from. The parent name is used to construct the kernel names for target
-        regions.
-      }],
-      /*retTy=*/"void",
-      /*methodName=*/"setParentName",
-      (ins "std::string":$parentName), [{}], [{
-        $_op->setAttr(
-          mlir::StringAttr::get($_op->getContext(),
-                                llvm::Twine{"omp.outline_parent_name"}),
-          mlir::StringAttr::get($_op->getContext(), parentName));
-       }]>,
-
-    InterfaceMethod<
-      /*description=*/[{
-        Returns the parent function name from where the target op was outlined
-        from. If it doesn't exist it returns an empty string.
-      }],
-      /*retTy=*/"llvm::StringRef",
-      /*methodName=*/"getParentName",
-      (ins), [{}], [{
-        if (Attribute parentName = $_op->getAttr("omp.outline_parent_name"))
-          if (::llvm::isa<mlir::StringAttr>(parentName))
-            return ::llvm::dyn_cast<mlir::StringAttr>(parentName).getValue();
-        return {};
-      }]>
-  ];
-}
-
 #endif // OpenMP_OPS_INTERFACES
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index 6e69cd0d386bd2..2d3be76c65e817 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -98,13 +98,6 @@ void OpenMPDialect::initialize() {
       *getContext());
   mlir::func::FuncOp::attachInterface<
       mlir::omp::DeclareTargetDefaultModel<mlir::func::FuncOp>>(*getContext());
-
-  // Attach default early outlining interface to func ops.
-  mlir::func::FuncOp::attachInterface<
-      mlir::omp::EarlyOutliningDefaultModel<mlir::func::FuncOp>>(*getContext());
-  mlir::LLVM::LLVMFuncOp::attachInterface<
-      mlir::omp::EarlyOutliningDefaultModel<mlir::LLVM::LLVMFuncOp>>(
-      *getContext());
 }
 
 //===----------------------------------------------------------------------===//
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index e7aebc3ce4be56..a7e320771189f3 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -2362,13 +2362,6 @@ convertOmpTarget(Operation &opInst, llvm::IRBuilderBase &builder,
   llvm::OpenMPIRBuilder::LocationDescription ompLoc(builder);
   StringRef parentName = opInst.getParentOfType<LLVM::LLVMFuncOp>().getName();
 
-  // Override parent name if early outlining function
-  if (auto earlyOutlineOp = llvm::dyn_cast<mlir::omp::EarlyOutliningInterface>(
-          opInst.getParentOfType<LLVM::LLVMFuncOp>().getOperation())) {
-    llvm::StringRef outlineParentName = earlyOutlineOp.getParentName();
-    parentName = outlineParentName.empty() ? parentName : outlineParentName;
-  }
-
   llvm::TargetRegionEntryInfo entryInfo;
 
   if (!getTargetEntryUniqueInfo(entryInfo, targetOp, parentName))
diff --git a/mlir/test/Dialect/OpenMP/attr.mlir b/mlir/test/Dialect/OpenMP/attr.mlir
index a9e4c82fe34aaa..61a4e0df2f1ff6 100644
--- a/mlir/test/Dialect/OpenMP/attr.mlir
+++ b/mlir/test/Dialect/OpenMP/attr.mlir
@@ -181,11 +181,3 @@ llvm.mlir.global external @omp_decl_tar_data_any_enter() {omp.declare_target = #
   %0 = llvm.mlir.constant(1 : i32) : i32
   llvm.return %0 : i32
 }
-
-// ----
-
-// CHECK-LABEL: func @_QPwriteindex_omp_outline_0
-// CHECK-SAME: {{.*}} attributes {omp.outline_parent_name = "QPwriteindex"} {
-func.func @_QPwriteindex_omp_outline_0() attributes {omp.outline_parent_name = "QPwriteindex"} {
-   return
-}
diff --git a/mlir/test/Target/LLVMIR/omptarget-parallel-llvm.mlir b/mlir/test/Target/LLVMIR/omptarget-parallel-llvm.mlir
index a21e6d61a56185..1fccb441f4d596 100644
--- a/mlir/test/Target/LLVMIR/omptarget-parallel-llvm.mlir
+++ b/mlir/test/Target/LLVMIR/omptarget-parallel-llvm.mlir
@@ -4,7 +4,7 @@
 // for omp target parallel construct
 
 module attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<"dlti.alloca_memory_space", 5 : ui32>>, llvm.data_layout = "e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32-p7:160:256:256:32-p8:128:128-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5-G1-ni:7:8:9", llvm.target_triple = "amdgcn-amd-amdhsa", omp.is_gpu = true, omp.is_target_device = true, omp.target = #omp.target<target_cpu = "gfx90a", target_features = "">} {
-  llvm.func @_QQmain_omp_outline_1(%arg0: !llvm.ptr) attributes {omp.declare_target = #omp.declaretarget<device_type = (host), capture_clause = (to)>, omp.outline_parent_name = "_QQmain"} {
+  llvm.func @_QQmain_omp_outline_1(%arg0: !llvm.ptr) attributes {omp.declare_target = #omp.declaretarget<device_type = (host), capture_clause = (to)>} {
     %0 = omp.map_info var_ptr(%arg0 : !llvm.ptr, i32) map_clauses(from) capture(ByRef) -> !llvm.ptr {name = "d"}
     omp.target map_entries(%0 -> %arg2 : !llvm.ptr) {
     ^bb0(%arg2: !llvm.ptr):
@@ -18,7 +18,7 @@ module attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<"dlti.alloca_memo
   llvm.return
   }
 
-  llvm.func @_test_num_threads(%arg0: !llvm.ptr) attributes {omp.declare_target = #omp.declaretarget<device_type = (host), capture_clause = (to)>, omp.outline_parent_name = "_QQmain"} {
+  llvm.func @_test_num_threads(%arg0: !llvm.ptr) attributes {omp.declare_target = #omp.declaretarget<device_type = (host), capture_clause = (to)>} {
     %0 = omp.map_info var_ptr(%arg0 : !llvm.ptr, i32) map_clauses(from) capture(ByRef) -> !llvm.ptr {name = "d"}
     omp.target map_entries(%0 -> %arg2 : !llvm.ptr) {
     ^bb0(%arg2: !llvm.ptr):
diff --git a/mlir/test/Target/LLVMIR/omptarget-region-llvm-target-device.mlir b/mlir/test/Target/LLVMIR/omptarget-region-llvm-target-device.mlir
deleted file mode 100644
index 6fa039f522e206..00000000000000
--- a/mlir/test/Target/LLVMIR/omptarget-region-llvm-target-device.mlir
+++ /dev/null
@@ -1,21 +0,0 @@
-// This test checks that the name of the generated kernel function is using the
-// name stored in the omp.outline_parent_name attribute.
-// RUN: mlir-translate -mlir-to-llvmir %s | FileCheck %s
-
-module attributes {omp.is_target_device = true} {
-  llvm.func @writeindex_omp_outline_0_(%val0: !llvm.ptr, %val1: !llvm.ptr) attributes {omp.outline_parent_name = "writeindex_"} {
-    %0 = omp.map_info var_ptr(%val0 : !llvm.ptr, i32)   map_clauses(tofrom) capture(ByRef) -> !llvm.ptr {name = ""}
-    %1 = omp.map_info var_ptr(%val1 : !llvm.ptr, i32)   map_clauses(tofrom) capture(ByRef) -> !llvm.ptr {name = ""}
-    omp.target   map_entries(%0 -> %arg0, %1 -> %arg1 : !llvm.ptr, !llvm.ptr) {
-    ^bb0(%arg0: !llvm.ptr, %arg1: !llvm.ptr):
-      %2 = llvm.mlir.constant(20 : i32) : i32
-      %3 = llvm.mlir.constant(10 : i32) : i32
-      llvm.store %3, %arg0 : i32, !llvm.ptr
-      llvm.store %2, %arg1 : i32, !llvm.ptr
-      omp.terminator
-    }
-    llvm.return
-  }
-}
-
-// CHECK: define {{.*}} void @__omp_offloading_{{.*}}_{{.*}}_writeindex__l{{.*}}(ptr {{.*}}, ptr {{.*}}) {

Copy link
Contributor

@kiranchandramohan kiranchandramohan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK with me. Please wait for @agozillon or @TIFitis.

Copy link
Contributor

@agozillon agozillon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change LGTM.

Copy link
Member

@TIFitis TIFitis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :)

After the removal of the OpenMP early outlining MLIR pass in llvm#67319, the
`EarlyOutliningInterface` stopped doing any useful work. It used to be
necessary to tie the name of the function from which a target region was
outlined to that new function, so it would be used when translating to LLVM IR
in place of the outlined function's name.

This is not necessary anymore, so this patch removes all references to this
interface and uses of the `omp.outline_parent_name` discardable attribute in
tests.
@skatrak
Copy link
Contributor Author

skatrak commented Jan 18, 2024

Thank you for giving this patch a look. Merging now, since premerge test failures look unrelated.

@skatrak skatrak merged commit 2747193 into llvm:main Jan 18, 2024
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants