[Mlir-commits] [llvm] [mlir] [MLIR][OpenMP] Fix recursive mapper emission. (PR #178453)

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir

Author: Akash Banerjee (TIFitis)

<details>
<summary>Changes</summary>

Recursive types can cause re-entrant mapper emission. The mapper function is created by OpenMPIRBuilder before the callbacks run, so it may already exist in the LLVM module even though it is not yet registered in the ModuleTranslation mapping table. Reuse and register it to break the recursion. Added offloading test.

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


2 Files Affected:

- (modified) mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp (+17-1) 
- (added) offload/test/offloading/fortran/recursive-default-mapper.f90 (+40) 


``````````diff
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index 022322502a755..1955d681e4826 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -5333,6 +5333,17 @@ getOrCreateUserDefinedMapperFunc(Operation *op, llvm::IRBuilderBase &builder,
   if (auto *lookupFunc = moduleTranslation.lookupFunction(mapperFuncName))
     return lookupFunc;
 
+  // Recursive types can cause re-entrant mapper emission. The mapper function
+  // is created by OpenMPIRBuilder before the callbacks run, so it may already
+  // exist in the LLVM module even though it is not yet registered in the
+  // ModuleTranslation mapping table. Reuse and register it to break the
+  // recursion.
+  if (llvm::Function *existingFunc =
+          moduleTranslation.getLLVMModule()->getFunction(mapperFuncName)) {
+    moduleTranslation.mapFunction(mapperFuncName, existingFunc);
+    return existingFunc;
+  }
+
   return emitUserDefinedMapper(declMapperOp, builder, moduleTranslation,
                                mapperFuncName, targetDirective);
 }
@@ -5389,7 +5400,12 @@ emitUserDefinedMapper(Operation *op, llvm::IRBuilderBase &builder,
       genMapInfoCB, varType, mapperFuncName, customMapperCB);
   if (!newFn)
     return newFn.takeError();
-  moduleTranslation.mapFunction(mapperFuncName, *newFn);
+  if (llvm::Function *mappedFunc = moduleTranslation.lookupFunction(mapperFuncName)) {
+    assert(mappedFunc == *newFn &&
+           "mapper function mapping disagrees with emitted function");
+  } else {
+    moduleTranslation.mapFunction(mapperFuncName, *newFn);
+  }
   return *newFn;
 }
 
diff --git a/offload/test/offloading/fortran/recursive-default-mapper.f90 b/offload/test/offloading/fortran/recursive-default-mapper.f90
new file mode 100644
index 0000000000000..47b706d108437
--- /dev/null
+++ b/offload/test/offloading/fortran/recursive-default-mapper.f90
@@ -0,0 +1,40 @@
+! Offloading test for recursive default mapper emission
+! REQUIRES: flang, amdgpu
+
+! RUN: %libomptarget-compile-fortran-run-and-check-generic
+
+module recursive_mapper_mod
+  implicit none
+
+  type :: inner
+    integer :: value
+    type(inner), pointer :: next
+  end type inner
+
+  type :: outer
+    integer, allocatable :: arr(:)
+    type(inner), pointer :: head
+  end type outer
+
+contains
+
+end module recursive_mapper_mod
+
+program main
+  use recursive_mapper_mod
+  implicit none
+
+  type(outer) :: o
+
+  allocate(o%arr(2))
+  o%arr = [1, 2]
+
+  !$omp target map(tofrom: o)
+    o%arr(1) = o%arr(1) + 1
+    o%arr(2) = o%arr(2) + 1
+  !$omp end target
+
+  print *, o%arr(1), o%arr(2)
+end program main
+
+! CHECK: 2 3

``````````

</details>


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