diff options
author | Kareem Ergawy <kareem.ergawy@amd.com> | 2024-06-23 12:16:34 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-06-23 12:16:34 +0200 |
commit | f372bb45e39b7eae94930fc419724e12596a8361 (patch) | |
tree | 3acd3a19189560e295f2b02ad4e8d1e14b88bcf9 | |
parent | f7fc72e281e26082d98b26c95c7ffc93393b3920 (diff) | |
download | llvm-f372bb45e39b7eae94930fc419724e12596a8361.zip llvm-f372bb45e39b7eae94930fc419724e12596a8361.tar.gz llvm-f372bb45e39b7eae94930fc419724e12596a8361.tar.bz2 |
[OpenMP][LLVM] Clone `omp.private` op in the parent module (#96024)
Fixes a crash uncovered by test 0007_0019.f90 in the Fujitsu test suite.
Previously, in the `PrivCB`, we cloned the `omp.private` op without
inserting it in the parent module of the original op. This causes issues
whenever there is an op that needs to lookup the parent module (e.g. for
symbol lookup). This PR fixes the issue by cloning in the parent module
instead of creating an orphaned op.
-rw-r--r-- | mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp | 21 | ||||
-rw-r--r-- | mlir/test/Target/LLVMIR/openmp-private.mlir | 44 |
2 files changed, 64 insertions, 1 deletions
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp index 7793d5d..52c780f 100644 --- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp +++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp @@ -1304,7 +1304,26 @@ convertOmpParallel(omp::ParallelOp opInst, llvm::IRBuilderBase &builder, // region. The privatizer is processed in-place (see below) before it // gets inlined in the parallel region and therefore processing the // original op is dangerous. - return {privVar, privatizer.clone()}; + + MLIRContext &context = moduleTranslation.getContext(); + mlir::IRRewriter opCloner(&context); + opCloner.setInsertionPoint(privatizer); + auto clone = llvm::cast<mlir::omp::PrivateClauseOp>( + opCloner.clone(*privatizer)); + + // Unique the clone name to avoid clashes in the symbol table. + unsigned counter = 0; + SmallString<256> cloneName = SymbolTable::generateSymbolName<256>( + privatizer.getSymName(), + [&](llvm::StringRef candidate) { + return SymbolTable::lookupNearestSymbolFrom( + opInst, StringAttr::get(&context, candidate)) != + nullptr; + }, + counter); + + clone.setSymName(cloneName); + return {privVar, clone}; } } diff --git a/mlir/test/Target/LLVMIR/openmp-private.mlir b/mlir/test/Target/LLVMIR/openmp-private.mlir index 58bda87..141724e 100644 --- a/mlir/test/Target/LLVMIR/openmp-private.mlir +++ b/mlir/test/Target/LLVMIR/openmp-private.mlir @@ -140,3 +140,47 @@ omp.private {type = private} @multi_block.privatizer : !llvm.ptr alloc { llvm.store %1, %arg2 : f32, !llvm.ptr omp.yield(%arg2 : !llvm.ptr) } + +// Tests fix for Fujitsu test suite test: 0007_0019.f90: the +// `llvm.mlir.addressof` op needs access to the parent module when lowering +// from the LLVM dialect to LLVM IR. If such op is used inside an `omp.private` +// op instance that was not created/cloned inside the module, we would get a +// seg fault due to trying to access a null pointer. + +// CHECK-LABEL: define internal void @lower_region_with_addressof..omp_par +// CHECK: omp.par.region: +// CHECK: br label %[[PAR_REG_BEG:.*]] +// CHECK: [[PAR_REG_BEG]]: +// CHECK: %[[PRIVATIZER_GEP:.*]] = getelementptr double, ptr @_QQfoo, i64 111 +// CHECK: call void @bar(ptr %[[PRIVATIZER_GEP]]) +// CHECK: call void @bar(ptr getelementptr (double, ptr @_QQfoo, i64 222)) +llvm.func @lower_region_with_addressof() { + %0 = llvm.mlir.constant(1 : i64) : i64 + %1 = llvm.alloca %0 x f64 {bindc_name = "u1"} : (i64) -> !llvm.ptr + omp.parallel private(@_QFlower_region_with_addressof_privatizer %1 -> %arg0 : !llvm.ptr) { + %c0 = llvm.mlir.constant(111 : i64) : i64 + %2 = llvm.getelementptr %arg0[%c0] : (!llvm.ptr, i64) -> !llvm.ptr, f64 + llvm.call @bar(%2) : (!llvm.ptr) -> () + + %c1 = llvm.mlir.constant(222 : i64) : i64 + %3 = llvm.mlir.addressof @_QQfoo: !llvm.ptr + %4 = llvm.getelementptr %3[%c1] : (!llvm.ptr, i64) -> !llvm.ptr, f64 + llvm.call @bar(%4) : (!llvm.ptr) -> () + omp.terminator + } + + llvm.return +} + +omp.private {type = private} @_QFlower_region_with_addressof_privatizer : !llvm.ptr alloc { +^bb0(%arg0: !llvm.ptr): + %0 = llvm.mlir.addressof @_QQfoo: !llvm.ptr + omp.yield(%0 : !llvm.ptr) +} + +llvm.mlir.global linkonce constant @_QQfoo() {addr_space = 0 : i32} : !llvm.array<3 x i8> { + %0 = llvm.mlir.constant("foo") : !llvm.array<3 x i8> + llvm.return %0 : !llvm.array<3 x i8> +} + +llvm.func @bar(!llvm.ptr) |