[flang-commits] [flang] [flang][openmp] fix OMPFunctionFiltering pass after #87796 (PR #89776)

[via flang-commits]

https://github.com/jeanPerier created https://github.com/llvm/llvm-project/pull/89776

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.

>From c71c89fc0cfad6a65746e272f8a3c5f35ea24df6 Mon Sep 17 00:00:00 2001
From: Jean Perier <jperier at nvidia.com>
Date: Tue, 23 Apr 2024 08:06:26 -0700
Subject: [PATCH] [flang][openmp] fix OMPFunctionFiltering pass after #87796

---
 .../Transforms/OMPFunctionFiltering.cpp       |  7 ++++
 .../Lower/OpenMP/function-filtering-3.f90     | 37 +++++++++++++++++++
 2 files changed, 44 insertions(+)
 create mode 100644 flang/test/Lower/OpenMP/function-filtering-3.f90

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