[flang][openmp] fix OMPFunctionFiltering pass after #87796 #89776
+45
−0
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-openmp Author: None (jeanPerier) ChangesThe pass assumed that all fun.func symbol usages could be safely replaced by undef, that is not true after #87796 that added a back link from internal procedure back to the parent procedure. This caused the internal procedures to be erased and then processed (segfault). Note that I noticed that the LLVM output for the device is basically empty. I think this is because the internal procedure is removed by MLIR on the device because its usages have been removed since the parent procedure, that contains the only call to it, is host only and was deleted by the filtering pass. This is unrelated to #87796 (was already the case before), so I did not want to change this here, but maybe the filtering should remove the private visibility aspects of internal procedures during the filtering. Full diff: https://github.com/llvm/llvm-project/pull/89776.diff 2 Files Affected:
diff --git a/flang/lib/Optimizer/Transforms/OMPFunctionFiltering.cpp b/flang/lib/Optimizer/Transforms/OMPFunctionFiltering.cpp
index 959099d039a5e6..ad3560946f4135 100644
--- a/flang/lib/Optimizer/Transforms/OMPFunctionFiltering.cpp
+++ b/flang/lib/Optimizer/Transforms/OMPFunctionFiltering.cpp
@@ -12,6 +12,7 @@
//===----------------------------------------------------------------------===//
#include "flang/Optimizer/Dialect/FIRDialect.h"
+#include "flang/Optimizer/Dialect/FIROpsSupport.h"
#include "flang/Optimizer/Transforms/Passes.h"
#include "mlir/Dialect/Func/IR/FuncOps.h"
@@ -66,6 +67,12 @@ class OMPFunctionFilteringPass
SymbolTable::UseRange funcUses = *funcOp.getSymbolUses(op);
for (SymbolTable::SymbolUse use : funcUses) {
Operation *callOp = use.getUser();
+ if (mlir::isa<func::FuncOp>(callOp)) {
+ // Do not delete internal procedures holding the symbol of their
+ // Fortran host procedure as attribute.
+ callOp->removeAttr(fir::getHostSymbolAttrName());
+ continue;
+ }
// If the callOp has users then replace them with Undef values.
if (!callOp->use_empty()) {
SmallVector<Value> undefResults;
diff --git a/flang/test/Lower/OpenMP/function-filtering-3.f90 b/flang/test/Lower/OpenMP/function-filtering-3.f90
new file mode 100644
index 00000000000000..32cc6b6caf7189
--- /dev/null
+++ b/flang/test/Lower/OpenMP/function-filtering-3.f90
@@ -0,0 +1,37 @@
+! RUN: %flang_fc1 -fopenmp -flang-experimental-hlfir -emit-llvm %s -o - | FileCheck --check-prefixes=LLVM-HOST %s
+! RUN: %flang_fc1 -fopenmp -emit-hlfir %s -o - | FileCheck --check-prefixes=MLIR-HOST,MLIR-ALL %s
+! RUN: %flang_fc1 -fopenmp -fopenmp-is-target-device -flang-experimental-hlfir -emit-llvm %s -o - | FileCheck --check-prefixes=LLVM-DEVICE %s
+! RUN: %flang_fc1 -fopenmp -fopenmp-is-target-device -emit-hlfir %s -o - | FileCheck --check-prefixes=MLIR-DEVICE,MLIR-ALL %s
+! RUN: bbc -fopenmp -emit-hlfir %s -o - | FileCheck --check-prefixes=MLIR-HOST,MLIR-ALL %s
+! RUN: bbc -fopenmp -fopenmp-is-target-device -emit-hlfir %s -o - | FileCheck --check-prefixes=MLIR-DEVICE,MLIR-ALL %s
+
+! Check that the correct LLVM IR functions are kept for the host and device
+! after running the whole set of translation and transformation passes from
+! Fortran.
+
+! MLIR-HOST: func.func @{{.*}}host_parent_procedure(
+! MLIR-HOST: return
+! MLIR-DEVICE-NOT: func.func {{.*}}host_parent_procedure(
+
+! LLVM-HOST: define {{.*}} @host_parent_procedure{{.*}}(
+! LLVM-DEVICE-NOT: {{.*}} @{{.*}}_host_parent_procedure{{.*}}(
+subroutine host_parent_procedure(x)
+ integer, intent(out) :: x
+ call target_internal_proc(x)
+contains
+! MLIR-ALL: func.func private @_QFhost_parent_procedurePtarget_internal_proc(
+
+! LLVM-HOST: define {{.*}} @_QFhost_parent_procedurePtarget_internal_proc(
+! LLVM-HOST: define {{.*}} @__omp_offloading_{{.*}}QFhost_parent_procedurePtarget_internal_proc{{.*}}(
+
+! FIXME: the last check above should also work on the host, but the offload function
+! is deleted because it is private and all its usages have been removed in the
+! device code. Maybe the private attribute should be removed on internal
+! functions while filtering?
+subroutine target_internal_proc(x)
+ integer, intent(out) :: x
+ !$omp target map(from:x)
+ x = 10
+ !$omp end target
+end subroutine
+end subroutine
|
|
@llvm/pr-subscribers-flang-fir-hlfir Author: None (jeanPerier) ChangesThe pass assumed that all fun.func symbol usages could be safely replaced by undef, that is not true after #87796 that added a back link from internal procedure back to the parent procedure. This caused the internal procedures to be erased and then processed (segfault). Note that I noticed that the LLVM output for the device is basically empty. I think this is because the internal procedure is removed by MLIR on the device because its usages have been removed since the parent procedure, that contains the only call to it, is host only and was deleted by the filtering pass. This is unrelated to #87796 (was already the case before), so I did not want to change this here, but maybe the filtering should remove the private visibility aspects of internal procedures during the filtering. Full diff: https://github.com/llvm/llvm-project/pull/89776.diff 2 Files Affected:
diff --git a/flang/lib/Optimizer/Transforms/OMPFunctionFiltering.cpp b/flang/lib/Optimizer/Transforms/OMPFunctionFiltering.cpp
index 959099d039a5e6..ad3560946f4135 100644
--- a/flang/lib/Optimizer/Transforms/OMPFunctionFiltering.cpp
+++ b/flang/lib/Optimizer/Transforms/OMPFunctionFiltering.cpp
@@ -12,6 +12,7 @@
//===----------------------------------------------------------------------===//
#include "flang/Optimizer/Dialect/FIRDialect.h"
+#include "flang/Optimizer/Dialect/FIROpsSupport.h"
#include "flang/Optimizer/Transforms/Passes.h"
#include "mlir/Dialect/Func/IR/FuncOps.h"
@@ -66,6 +67,12 @@ class OMPFunctionFilteringPass
SymbolTable::UseRange funcUses = *funcOp.getSymbolUses(op);
for (SymbolTable::SymbolUse use : funcUses) {
Operation *callOp = use.getUser();
+ if (mlir::isa<func::FuncOp>(callOp)) {
+ // Do not delete internal procedures holding the symbol of their
+ // Fortran host procedure as attribute.
+ callOp->removeAttr(fir::getHostSymbolAttrName());
+ continue;
+ }
// If the callOp has users then replace them with Undef values.
if (!callOp->use_empty()) {
SmallVector<Value> undefResults;
diff --git a/flang/test/Lower/OpenMP/function-filtering-3.f90 b/flang/test/Lower/OpenMP/function-filtering-3.f90
new file mode 100644
index 00000000000000..32cc6b6caf7189
--- /dev/null
+++ b/flang/test/Lower/OpenMP/function-filtering-3.f90
@@ -0,0 +1,37 @@
+! RUN: %flang_fc1 -fopenmp -flang-experimental-hlfir -emit-llvm %s -o - | FileCheck --check-prefixes=LLVM-HOST %s
+! RUN: %flang_fc1 -fopenmp -emit-hlfir %s -o - | FileCheck --check-prefixes=MLIR-HOST,MLIR-ALL %s
+! RUN: %flang_fc1 -fopenmp -fopenmp-is-target-device -flang-experimental-hlfir -emit-llvm %s -o - | FileCheck --check-prefixes=LLVM-DEVICE %s
+! RUN: %flang_fc1 -fopenmp -fopenmp-is-target-device -emit-hlfir %s -o - | FileCheck --check-prefixes=MLIR-DEVICE,MLIR-ALL %s
+! RUN: bbc -fopenmp -emit-hlfir %s -o - | FileCheck --check-prefixes=MLIR-HOST,MLIR-ALL %s
+! RUN: bbc -fopenmp -fopenmp-is-target-device -emit-hlfir %s -o - | FileCheck --check-prefixes=MLIR-DEVICE,MLIR-ALL %s
+
+! Check that the correct LLVM IR functions are kept for the host and device
+! after running the whole set of translation and transformation passes from
+! Fortran.
+
+! MLIR-HOST: func.func @{{.*}}host_parent_procedure(
+! MLIR-HOST: return
+! MLIR-DEVICE-NOT: func.func {{.*}}host_parent_procedure(
+
+! LLVM-HOST: define {{.*}} @host_parent_procedure{{.*}}(
+! LLVM-DEVICE-NOT: {{.*}} @{{.*}}_host_parent_procedure{{.*}}(
+subroutine host_parent_procedure(x)
+ integer, intent(out) :: x
+ call target_internal_proc(x)
+contains
+! MLIR-ALL: func.func private @_QFhost_parent_procedurePtarget_internal_proc(
+
+! LLVM-HOST: define {{.*}} @_QFhost_parent_procedurePtarget_internal_proc(
+! LLVM-HOST: define {{.*}} @__omp_offloading_{{.*}}QFhost_parent_procedurePtarget_internal_proc{{.*}}(
+
+! FIXME: the last check above should also work on the host, but the offload function
+! is deleted because it is private and all its usages have been removed in the
+! device code. Maybe the private attribute should be removed on internal
+! functions while filtering?
+subroutine target_internal_proc(x)
+ integer, intent(out) :: x
+ !$omp target map(from:x)
+ x = 10
+ !$omp end target
+end subroutine
+end subroutine
|
DominikAdamski
approved these changes
Apr 24, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The pass assumed that all fun.func symbol usages could be safely replaced by undef, that is not true after #87796 that added a back link from internal procedure back to the parent procedure. This caused the internal procedures to be erased and then processed (segfault).
Note that I noticed that the LLVM output for the device is basically empty. I think this is because the internal procedure is removed by MLIR on the device because its usages have been removed since the parent procedure, that contains the only call to it, is host only and was deleted by the filtering pass.
This is unrelated to #87796 (was already the case before), so I did not want to change this here, but maybe the filtering should remove the private visibility aspects of internal procedures during the filtering.