[MLIR,Flang,OpenMP] Remove usage of getElementType in OpenMPTranslation #69772

kiranchandramohan · 2023-10-20T20:28:34Z

Remove usage of getElementType in OpenMPTranslation to pave way for switching to opaque pointers in MLIR and Flang. The approach chosen stores the elementType in a new field in MapInfo called varType. A similar approach was chosen for AtomicReadOp in
81767f5

Remove usage of getElementType in OpenMPTranslation to pave way for switching to opaque pointers in MLIR and Flang. The approach chosen stores the elementType in a new field in MapInfo called varType. A similar approach was chosen for AtomicReadOp in llvm@81767f5

llvmbot · 2023-10-20T20:33:23Z

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

@llvm/pr-subscribers-mlir-llvm

Author: Kiran Chandramohan (kiranchandramohan)

Changes

Remove usage of getElementType in OpenMPTranslation to pave way for switching to opaque pointers in MLIR and Flang. The approach chosen stores the elementType in a new field in MapInfo called varType. A similar approach was chosen for AtomicReadOp in
81767f5

Patch is 77.27 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/69772.diff

18 Files Affected:

(modified) flang/lib/Lower/OpenMP.cpp (+2)
(modified) flang/test/Fir/convert-to-llvm-openmp-and-fir.fir (+18-18)
(modified) flang/test/Lower/OpenMP/FIR/array-bounds.f90 (+8-8)
(modified) flang/test/Lower/OpenMP/FIR/omp-target-early-outlining.f90 (+1-1)
(modified) flang/test/Lower/OpenMP/FIR/target.f90 (+5-5)
(modified) flang/test/Lower/OpenMP/array-bounds.f90 (+4-4)
(modified) mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td (+3-1)
(modified) mlir/lib/Conversion/OpenMPToLLVM/OpenMPToLLVM.cpp (+31-3)
(modified) mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp (+4-4)
(modified) mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir (+22-22)
(modified) mlir/test/Dialect/OpenMP/invalid.mlir (+4-4)
(modified) mlir/test/Dialect/OpenMP/ops.mlir (+28-28)
(modified) mlir/test/Target/LLVMIR/omptarget-declare-target-llvm-device.mlir (+1-1)
(modified) mlir/test/Target/LLVMIR/omptarget-llvm.mlir (+12-12)
(modified) mlir/test/Target/LLVMIR/omptarget-region-device-llvm.mlir (+3-3)
(modified) mlir/test/Target/LLVMIR/omptarget-region-llvm-target-device.mlir (+2-2)
(modified) mlir/test/Target/LLVMIR/omptarget-region-llvm.mlir (+3-3)
(modified) mlir/test/Target/LLVMIR/omptarget-region-parallel-llvm.mlir (+3-3)

diff --git a/flang/lib/Lower/OpenMP.cpp b/flang/lib/Lower/OpenMP.cpp
index 5f5e968eaaa6414..1c4b2055769ec62 100644
--- a/flang/lib/Lower/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP.cpp
@@ -1675,6 +1675,8 @@ createMapInfoOp(fir::FirOpBuilder &builder, mlir::Location loc,
 
   mlir::omp::MapInfoOp op =
       builder.create<mlir::omp::MapInfoOp>(loc, retTy, baseAddr);
+  op.setVarTypeAttr(mlir::TypeAttr::get(
+      llvm::dyn_cast<mlir::omp::PointerLikeType>(retTy).getElementType()));
   op.setNameAttr(builder.getStringAttr(name.str()));
   op.setImplicit(implicit);
   op.setMapType(mapType);
diff --git a/flang/test/Fir/convert-to-llvm-openmp-and-fir.fir b/flang/test/Fir/convert-to-llvm-openmp-and-fir.fir
index ba1a15bf773d2ef..27097e743a9f950 100644
--- a/flang/test/Fir/convert-to-llvm-openmp-and-fir.fir
+++ b/flang/test/Fir/convert-to-llvm-openmp-and-fir.fir
@@ -232,38 +232,38 @@ func.func @_QPomp_target_data() {
   %c0 = arith.constant 0 : index
   %4 = arith.subi %c1024, %c1 : index
   %5 = omp.bounds   lower_bound(%c0 : index) upper_bound(%4 : index) extent(%c1024 : index) stride(%c1 : index) start_idx(%c1 : index)
-  %6 = omp.map_info var_ptr(%0 : !fir.ref<!fir.array<1024xi32>>)   map_clauses(to) capture(ByRef) bounds(%5) -> !fir.ref<!fir.array<1024xi32>> {name = "a"}
+  %6 = omp.map_info var_ptr(%0 : !fir.ref<!fir.array<1024xi32>>, !fir.array<1024xi32>)   map_clauses(to) capture(ByRef) bounds(%5) -> !fir.ref<!fir.array<1024xi32>> {name = "a"}
   %c1_3 = arith.constant 1 : index
   %c0_4 = arith.constant 0 : index
   %7 = arith.subi %c1024_0, %c1_3 : index
   %8 = omp.bounds   lower_bound(%c0_4 : index) upper_bound(%7 : index) extent(%c1024_0 : index) stride(%c1_3 : index) start_idx(%c1_3 : index)
-  %9 = omp.map_info var_ptr(%1 : !fir.ref<!fir.array<1024xi32>>)   map_clauses(to) capture(ByRef) bounds(%8) -> !fir.ref<!fir.array<1024xi32>> {name = "b"}
+  %9 = omp.map_info var_ptr(%1 : !fir.ref<!fir.array<1024xi32>>, !fir.array<1024xi32>)   map_clauses(to) capture(ByRef) bounds(%8) -> !fir.ref<!fir.array<1024xi32>> {name = "b"}
   %c1_5 = arith.constant 1 : index
   %c0_6 = arith.constant 0 : index
   %10 = arith.subi %c1024_1, %c1_5 : index
   %11 = omp.bounds   lower_bound(%c0_6 : index) upper_bound(%10 : index) extent(%c1024_1 : index) stride(%c1_5 : index) start_idx(%c1_5 : index)
-  %12 = omp.map_info var_ptr(%2 : !fir.ref<!fir.array<1024xi32>>)   map_clauses(always, exit_release_or_enter_alloc) capture(ByRef) bounds(%11) -> !fir.ref<!fir.array<1024xi32>> {name = "c"}
+  %12 = omp.map_info var_ptr(%2 : !fir.ref<!fir.array<1024xi32>>, !fir.array<1024xi32>)   map_clauses(always, exit_release_or_enter_alloc) capture(ByRef) bounds(%11) -> !fir.ref<!fir.array<1024xi32>> {name = "c"}
   omp.target_enter_data   map_entries(%6, %9, %12 : !fir.ref<!fir.array<1024xi32>>, !fir.ref<!fir.array<1024xi32>>, !fir.ref<!fir.array<1024xi32>>)
   %c1_7 = arith.constant 1 : index
   %c0_8 = arith.constant 0 : index
   %13 = arith.subi %c1024, %c1_7 : index
   %14 = omp.bounds   lower_bound(%c0_8 : index) upper_bound(%13 : index) extent(%c1024 : index) stride(%c1_7 : index) start_idx(%c1_7 : index)
-  %15 = omp.map_info var_ptr(%0 : !fir.ref<!fir.array<1024xi32>>)   map_clauses(from) capture(ByRef) bounds(%14) -> !fir.ref<!fir.array<1024xi32>> {name = "a"}
+  %15 = omp.map_info var_ptr(%0 : !fir.ref<!fir.array<1024xi32>>, !fir.array<1024xi32>)   map_clauses(from) capture(ByRef) bounds(%14) -> !fir.ref<!fir.array<1024xi32>> {name = "a"}
   %c1_9 = arith.constant 1 : index
   %c0_10 = arith.constant 0 : index
   %16 = arith.subi %c1024_0, %c1_9 : index
   %17 = omp.bounds   lower_bound(%c0_10 : index) upper_bound(%16 : index) extent(%c1024_0 : index) stride(%c1_9 : index) start_idx(%c1_9 : index)
-  %18 = omp.map_info var_ptr(%1 : !fir.ref<!fir.array<1024xi32>>)   map_clauses(from) capture(ByRef) bounds(%17) -> !fir.ref<!fir.array<1024xi32>> {name = "b"}
+  %18 = omp.map_info var_ptr(%1 : !fir.ref<!fir.array<1024xi32>>, !fir.array<1024xi32>)   map_clauses(from) capture(ByRef) bounds(%17) -> !fir.ref<!fir.array<1024xi32>> {name = "b"}
   %c1_11 = arith.constant 1 : index
   %c0_12 = arith.constant 0 : index
   %19 = arith.subi %c1024_1, %c1_11 : index
   %20 = omp.bounds   lower_bound(%c0_12 : index) upper_bound(%19 : index) extent(%c1024_1 : index) stride(%c1_11 : index) start_idx(%c1_11 : index)
-  %21 = omp.map_info var_ptr(%2 : !fir.ref<!fir.array<1024xi32>>)   map_clauses(exit_release_or_enter_alloc) capture(ByRef) bounds(%20) -> !fir.ref<!fir.array<1024xi32>> {name = "c"}
+  %21 = omp.map_info var_ptr(%2 : !fir.ref<!fir.array<1024xi32>>, !fir.array<1024xi32>)   map_clauses(exit_release_or_enter_alloc) capture(ByRef) bounds(%20) -> !fir.ref<!fir.array<1024xi32>> {name = "c"}
   %c1_13 = arith.constant 1 : index
   %c0_14 = arith.constant 0 : index
   %22 = arith.subi %c1024_2, %c1_13 : index
   %23 = omp.bounds   lower_bound(%c0_14 : index) upper_bound(%22 : index) extent(%c1024_2 : index) stride(%c1_13 : index) start_idx(%c1_13 : index)
-  %24 = omp.map_info var_ptr(%3 : !fir.ref<!fir.array<1024xi32>>)   map_clauses(always, delete) capture(ByRef) bounds(%23) -> !fir.ref<!fir.array<1024xi32>> {name = "d"}
+  %24 = omp.map_info var_ptr(%3 : !fir.ref<!fir.array<1024xi32>>, !fir.array<1024xi32>)   map_clauses(always, delete) capture(ByRef) bounds(%23) -> !fir.ref<!fir.array<1024xi32>> {name = "d"}
   omp.target_exit_data   map_entries(%15, %18, %21, %24 : !fir.ref<!fir.array<1024xi32>>, !fir.ref<!fir.array<1024xi32>>, !fir.ref<!fir.array<1024xi32>>, !fir.ref<!fir.array<1024xi32>>)
   return
 }
@@ -285,38 +285,38 @@ func.func @_QPomp_target_data() {
  // CHECK:    %13 = llvm.mlir.constant(0 : index) : i64
  // CHECK:    %14 = llvm.mlir.constant(1023 : index) : i64
  // CHECK:    %15 = omp.bounds   lower_bound(%13 : i64) upper_bound(%14 : i64) extent(%0 : i64) stride(%12 : i64) start_idx(%12 : i64)
- // CHECK:    %16 = omp.map_info var_ptr(%[[VAL_1]] : !llvm.ptr<array<1024 x i32>>)   map_clauses(to) capture(ByRef) bounds(%15) -> !llvm.ptr<array<1024 x i32>> {name = "a"}
+ // CHECK:    %16 = omp.map_info var_ptr(%[[VAL_1]] : !llvm.ptr<array<1024 x i32>>, !llvm.array<1024 x i32>)   map_clauses(to) capture(ByRef) bounds(%15) -> !llvm.ptr<array<1024 x i32>> {name = "a"}
  // CHECK:    %17 = llvm.mlir.constant(1 : index) : i64
  // CHECK:    %18 = llvm.mlir.constant(0 : index) : i64
  // CHECK:    %19 = llvm.mlir.constant(1023 : index) : i64
  // CHECK:    %20 = omp.bounds   lower_bound(%18 : i64) upper_bound(%19 : i64) extent(%3 : i64) stride(%17 : i64) start_idx(%17 : i64)
- // CHECK:    %21 = omp.map_info var_ptr(%[[VAL_3]] : !llvm.ptr<array<1024 x i32>>)   map_clauses(to) capture(ByRef) bounds(%20) -> !llvm.ptr<array<1024 x i32>> {name = "b"}
+ // CHECK:    %21 = omp.map_info var_ptr(%[[VAL_3]] : !llvm.ptr<array<1024 x i32>>, !llvm.array<1024 x i32>)   map_clauses(to) capture(ByRef) bounds(%20) -> !llvm.ptr<array<1024 x i32>> {name = "b"}
  // CHECK:    %22 = llvm.mlir.constant(1 : index) : i64
  // CHECK:    %23 = llvm.mlir.constant(0 : index) : i64
  // CHECK:    %24 = llvm.mlir.constant(1023 : index) : i64
  // CHECK:    %25 = omp.bounds   lower_bound(%23 : i64) upper_bound(%24 : i64) extent(%6 : i64) stride(%22 : i64) start_idx(%22 : i64)
- // CHECK:    %26 = omp.map_info var_ptr(%[[VAL_5]] : !llvm.ptr<array<1024 x i32>>)   map_clauses(always, exit_release_or_enter_alloc) capture(ByRef) bounds(%25) -> !llvm.ptr<array<1024 x i32>> {name = "c"}
+ // CHECK:    %26 = omp.map_info var_ptr(%[[VAL_5]] : !llvm.ptr<array<1024 x i32>>, !llvm.array<1024 x i32>)   map_clauses(always, exit_release_or_enter_alloc) capture(ByRef) bounds(%25) -> !llvm.ptr<array<1024 x i32>> {name = "c"}
  // CHECK:    omp.target_enter_data   map_entries(%16, %21, %26 : !llvm.ptr<array<1024 x i32>>, !llvm.ptr<array<1024 x i32>>, !llvm.ptr<array<1024 x i32>>)
  // CHECK:    %27 = llvm.mlir.constant(1 : index) : i64
  // CHECK:    %28 = llvm.mlir.constant(0 : index) : i64
  // CHECK:    %29 = llvm.mlir.constant(1023 : index) : i64
  // CHECK:    %30 = omp.bounds   lower_bound(%28 : i64) upper_bound(%29 : i64) extent(%0 : i64) stride(%27 : i64) start_idx(%27 : i64)
- // CHECK:    %31 = omp.map_info var_ptr(%[[VAL_1]] : !llvm.ptr<array<1024 x i32>>)   map_clauses(from) capture(ByRef) bounds(%30) -> !llvm.ptr<array<1024 x i32>> {name = "a"}
+ // CHECK:    %31 = omp.map_info var_ptr(%[[VAL_1]] : !llvm.ptr<array<1024 x i32>>, !llvm.array<1024 x i32>)   map_clauses(from) capture(ByRef) bounds(%30) -> !llvm.ptr<array<1024 x i32>> {name = "a"}
  // CHECK:    %32 = llvm.mlir.constant(1 : index) : i64
  // CHECK:    %33 = llvm.mlir.constant(0 : index) : i64
  // CHECK:    %34 = llvm.mlir.constant(1023 : index) : i64
  // CHECK:    %35 = omp.bounds   lower_bound(%33 : i64) upper_bound(%34 : i64) extent(%3 : i64) stride(%32 : i64) start_idx(%32 : i64)
- // CHECK:    %36 = omp.map_info var_ptr(%[[VAL_3]] : !llvm.ptr<array<1024 x i32>>)   map_clauses(from) capture(ByRef) bounds(%35) -> !llvm.ptr<array<1024 x i32>> {name = "b"}
+ // CHECK:    %36 = omp.map_info var_ptr(%[[VAL_3]] : !llvm.ptr<array<1024 x i32>>, !llvm.array<1024 x i32>)   map_clauses(from) capture(ByRef) bounds(%35) -> !llvm.ptr<array<1024 x i32>> {name = "b"}
  // CHECK:    %37 = llvm.mlir.constant(1 : index) : i64
  // CHECK:    %38 = llvm.mlir.constant(0 : index) : i64
  // CHECK:    %39 = llvm.mlir.constant(1023 : index) : i64
  // CHECK:    %40 = omp.bounds   lower_bound(%38 : i64) upper_bound(%39 : i64) extent(%6 : i64) stride(%37 : i64) start_idx(%37 : i64)
- // CHECK:    %41 = omp.map_info var_ptr(%[[VAL_5]] : !llvm.ptr<array<1024 x i32>>)   map_clauses(exit_release_or_enter_alloc) capture(ByRef) bounds(%40) -> !llvm.ptr<array<1024 x i32>> {name = "c"}
+ // CHECK:    %41 = omp.map_info var_ptr(%[[VAL_5]] : !llvm.ptr<array<1024 x i32>>, !llvm.array<1024 x i32>)   map_clauses(exit_release_or_enter_alloc) capture(ByRef) bounds(%40) -> !llvm.ptr<array<1024 x i32>> {name = "c"}
  // CHECK:    %42 = llvm.mlir.constant(1 : index) : i64
  // CHECK:    %43 = llvm.mlir.constant(0 : index) : i64
  // CHECK:    %44 = llvm.mlir.constant(1023 : index) : i64
  // CHECK:    %45 = omp.bounds   lower_bound(%43 : i64) upper_bound(%44 : i64) extent(%9 : i64) stride(%42 : i64) start_idx(%42 : i64)
- // CHECK:    %46 = omp.map_info var_ptr(%[[VAL_7]] : !llvm.ptr<array<1024 x i32>>)   map_clauses(always, delete) capture(ByRef) bounds(%45) -> !llvm.ptr<array<1024 x i32>> {name = "d"}
+ // CHECK:    %46 = omp.map_info var_ptr(%[[VAL_7]] : !llvm.ptr<array<1024 x i32>>, !llvm.array<1024 x i32>)   map_clauses(always, delete) capture(ByRef) bounds(%45) -> !llvm.ptr<array<1024 x i32>> {name = "d"}
  // CHECK:    omp.target_exit_data   map_entries(%31, %36, %41, %46 : !llvm.ptr<array<1024 x i32>>, !llvm.ptr<array<1024 x i32>>, !llvm.ptr<array<1024 x i32>>, !llvm.ptr<array<1024 x i32>>)
  // CHECK:    llvm.return
  // CHECK: }
@@ -331,7 +331,7 @@ func.func @_QPopenmp_target_data_region() {
   %c0 = arith.constant 0 : index
   %c2 = arith.subi %c1024, %c3 : index
   %bound = omp.bounds   lower_bound(%c0 : index) upper_bound(%c2 : index) extent(%c1024 : index) stride(%c3 : index) start_idx(%c3 : index)
-  %entry = omp.map_info var_ptr(%0 : !fir.ref<!fir.array<1024xi32>>)   map_clauses(tofrom) capture(ByRef) bounds(%bound) -> !fir.ref<!fir.array<1024xi32>> {name = "a"}
+  %entry = omp.map_info var_ptr(%0 : !fir.ref<!fir.array<1024xi32>>, !fir.array<1024xi32>)   map_clauses(tofrom) capture(ByRef) bounds(%bound) -> !fir.ref<!fir.array<1024xi32>> {name = "a"}
   omp.target_data   map_entries(%entry : !fir.ref<!fir.array<1024xi32>>) {
     %c1_i32 = arith.constant 1 : i32
     %2 = fir.convert %c1_i32 : (i32) -> index
@@ -370,7 +370,7 @@ func.func @_QPopenmp_target_data_region() {
 // CHECK:           %[[VAL_ZERO:.*]] = llvm.mlir.constant(0 : index) : i64
 // CHECK:           %[[VAL_UPPER:.*]] = llvm.mlir.constant(1023 : index) : i64
 // CHECK:           %[[VAL_BOUNDS:.*]] = omp.bounds   lower_bound(%[[VAL_ZERO]] : i64) upper_bound(%[[VAL_UPPER]] : i64) extent(%[[VAL_MAX]] : i64) stride(%[[VAL_ONE]] : i64) start_idx(%[[VAL_ONE]] : i64)
-// CHECK:           %[[VAL_MAP:.*]] = omp.map_info var_ptr(%[[VAL_1]] : !llvm.ptr<array<1024 x i32>>)  map_clauses(tofrom) capture(ByRef) bounds(%[[VAL_BOUNDS]]) -> !llvm.ptr<array<1024 x i32>> {name = "a"}
+// CHECK:           %[[VAL_MAP:.*]] = omp.map_info var_ptr(%[[VAL_1]] : !llvm.ptr<array<1024 x i32>>, !llvm.array<1024 x i32>)  map_clauses(tofrom) capture(ByRef) bounds(%[[VAL_BOUNDS]]) -> !llvm.ptr<array<1024 x i32>> {name = "a"}
 // CHECK:           omp.target_data   map_entries(%[[VAL_MAP]] : !llvm.ptr<array<1024 x i32>>) {
 // CHECK:             %[[VAL_4:.*]] = llvm.mlir.constant(1 : i32) : i32
 // CHECK:             %[[VAL_5:.*]] = llvm.sext %[[VAL_4]] : i32 to i64
@@ -432,7 +432,7 @@ func.func @_QPomp_target() {
   %c0 = arith.constant 0 : index
   %1 = arith.subi %c512, %c1 : index
   %2 = omp.bounds   lower_bound(%c0 : index) upper_bound(%1 : index) extent(%c512 : index) stride(%c1 : index) start_idx(%c1 : index)
-  %3 = omp.map_info var_ptr(%0 : !fir.ref<!fir.array<512xi32>>)   map_clauses(tofrom) capture(ByRef) bounds(%2) -> !fir.ref<!fir.array<512xi32>> {name = "a"}
+  %3 = omp.map_info var_ptr(%0 : !fir.ref<!fir.array<512xi32>>, !fir.array<512xi32>)   map_clauses(tofrom) capture(ByRef) bounds(%2) -> !fir.ref<!fir.array<512xi32>> {name = "a"}
   omp.target   thread_limit(%c64_i32 : i32) map_entries(%3 : !fir.ref<!fir.array<512xi32>>) {
     %c10_i32 = arith.constant 10 : i32
     %c1_i64 = arith.constant 1 : i64
@@ -454,7 +454,7 @@ func.func @_QPomp_target() {
 // CHECK:           %[[LOWER:.*]] = llvm.mlir.constant(0 : index) : i64
 // CHECK:           %[[UPPER:.*]] = llvm.mlir.constant(511 : index) : i64
 // CHECK:           %[[BOUNDS:.*]] = omp.bounds   lower_bound(%[[LOWER]] : i64) upper_bound(%[[UPPER]] : i64) extent(%[[EXTENT]] : i64) stride(%[[STRIDE]] : i64) start_idx(%[[STRIDE]] : i64)
-// CHECK:           %[[MAP:.*]] = omp.map_info var_ptr(%2 : !llvm.ptr<array<512 x i32>>)   map_clauses(tofrom) capture(ByRef) bounds(%[[BOUNDS]]) -> !llvm.ptr<array<512 x i32>> {name = "a"}
+// CHECK:           %[[MAP:.*]] = omp.map_info var_ptr(%2 : !llvm.ptr<array<512 x i32>>, !llvm.array<512 x i32>)   map_clauses(tofrom) capture(ByRef) bounds(%[[BOUNDS]]) -> !llvm.ptr<array<512 x i32>> {name = "a"}
 // CHECK:           omp.target   thread_limit(%[[VAL_2]] : i32) map_entries(%[[MAP]] : !llvm.ptr<array<512 x i32>>) {
 // CHECK:             %[[VAL_3:.*]] = llvm.mlir.constant(10 : i32) : i32
 // CHECK:             %[[VAL_4:.*]] = llvm.mlir.constant(1 : i64) : i64
diff --git a/flang/test/Lower/OpenMP/FIR/array-bounds.f90 b/flang/test/Lower/OpenMP/FIR/array-bounds.f90
index 697ff44176b3f10..a24e8d294346748 100644
--- a/flang/test/Lower/OpenMP/FIR/array-bounds.f90
+++ b/flang/test/Lower/OpenMP/FIR/array-bounds.f90
@@ -8,13 +8,13 @@
 !DEVICE:  %[[C3:.*]] = arith.constant 1 : index
 !DEVICE:  %[[C4:.*]] = arith.constant 1 : index
 !DEVICE:  %[[BOUNDS0:.*]] = omp.bounds   lower_bound(%[[C1]] : index) upper_bound(%[[C2]] : index) stride(%[[C4]] : index) start_idx(%[[C4]] : index)
-!DEVICE:  %[[MAP0:.*]] = omp.map_info var_ptr(%[[ARG1]] : !fir.ref<!fir.array<10xi32>>)   map_clauses(tofrom) capture(ByRef) bounds(%[[BOUNDS0]]) -> !fir.ref<!fir.array<10xi32>> {name = "sp_read(2:5)"}
+!DEVICE:  %[[MAP0:.*]] = omp.map_info var_ptr(%[[ARG1]] : !fir.ref<!fir.array<10xi32>>, !fir.array<10xi32>)   map_clauses(tofrom) capture(ByRef) bounds(%[[BOUNDS0]]) -> !fir.ref<!fir.array<10xi32>> {name = "sp_read(2:5)"}
 !DEVICE:  %[[C5:.*]] = arith.constant 1 : index
 !DEVICE:  %[[C6:.*]] = arith.constant 4 : index
 !DEVICE:  %[[C7:.*]] = arith.constant 1 : index
 !DEVICE:  %[[C8:.*]] = arith.constant 1 : index
 !DEVICE:  %[[BOUNDS1:.*]] = omp.bounds   lower_bound(%[[C5]] : index) upper_bound(%[[C6]] : index) stride(%[[C8]] : index) start_idx(%[[C8]] : index)
-!DEVICE:  %[[MAP1:.*]] = omp.map_info var_ptr(%[[ARG2]] : !fir.ref<!fir.array<10xi32>>)   map_clauses(tofrom) capture(ByRef) bounds(%[[BOUNDS1]]) -> !fir.ref<!fir.array<10xi32>> {name = "sp_write(2:5)"}
+!DEVICE:  %[[MAP1:.*]] = omp.map_info var_ptr(%[[ARG2]] : !fir.ref<!fir.array<10xi32>>, !fir.array<10xi32>)   map_clauses(tofrom) capture(ByRef) bounds(%[[BOUNDS1]]) -> !fir.ref<!fir.array<10xi32>> {name = "sp_write(2:5)"}
 !DEVICE:  omp.target   map_entries(%[[MAP0]], %[[MAP1]] : !fir.ref<!fir.array<10xi32>>, !fir.ref<!fir.array<10xi32>>) {
 
 !HOST-LABEL:  func.func @_QPread_write_section() {
@@ -25,12 +25,12 @@
 !HOST:  %[[C2:.*]] = arith.constant 1 : index
 !HOST:  %[[C3:.*]] = arith.constant 4 : index
 !HOST:  %[[BOUNDS0:.*]] = omp.bounds   lower_bound(%[[C2]] : index) upper_bound(%[[C3]] : index) stride(%[[C1]] : index) start_idx(%[[C1]] : index)
-!HOST:  %[[MAP0:.*]] = omp.map_info var_ptr(%[[READ]] : !fir.ref<!fir.array<10xi32>>)   map_clauses(tofrom) capture(ByRef) bounds(%[[BOUNDS0]]) -> !fir.ref<!fir.array<10xi32>> {name = "sp_read(2:5)"}
+!HOST:  %[[MAP0:.*]] = omp.map_info var_ptr(%[[READ]] : !fir.ref<!fir.array<10xi32>>, !fir.array<10xi32>)   map_clauses(tofrom) capture(ByRef) bounds(%[[BOUNDS0]]) -> !fir.ref<!fir.array<10xi32>> {name = "sp_read(2:5)"}
 !HOST:  %[[C4:.*]] = arith.constant 1 : index
 !HOST:  %[[C5:.*]] = arith.constant 1 : index
 !HOST:  %[[C6:.*]] = arith.constant 4 : index
 !HOST:  %[[BOUNDS1:.*]] = omp.bounds   lower_bound(%[[C5]] : index) upper_bound(%[[C6]] : index) stride(%[[C4]] : index) start_idx(%[[C4]] : index)
-!HOST:  %[[MAP1:.*]] = omp.map_info var_ptr(%[[WRITE]] : !fir.ref<!fir.array<10xi32>>)   map_clauses(tofrom) capture(ByRef) bounds(%[[BOUNDS1]]) -> !fir.ref<!fir.array<10xi32>> {name = "sp_write(2:5)"}
+!HOST:  %[[MAP1:.*]] = omp.map_info var_ptr(%[[WRITE]] : !fir.ref<!fir.array<10xi32>>, !fir.array<10xi32>)   map_clauses(tofrom) capture(ByRef) bounds(%[[BOUNDS1]]) -> !fir.ref<!fir.array<10xi32>> {name = "sp_write(2:5)"}
 !HOST:  omp.target   map_entries(%[[MAP0]], %[[MAP1]] : !fir.ref<!fir.array<10xi32>>, !fir.ref<!fir.array<10xi32>>) {
 
 subroutine read_write_section()
@@ -56,7 +56,7 @@ module assumed_array_routines
 !DEVICE: %[[C4:.*]] = arith.constant 1 : index
 !DEVICE: %[[BOUNDS:.*]] = omp.bounds   lower_bound(%[[C0]] : index) upper_bound(%[[C1]] : index) stride(%[[C3]]#2 : index) start_idx(%[[C4]] : index) {stride_in_bytes = true}
 !DEVICE: %[[ARGADDR:.*]] = fir.box_addr %[[ARG1]] : (!fir.box<!fir.array<?xi32>>) -> !fir.ref<!fir.array<?xi32>>
-!DEVICE: %[[MAP:.*]] = omp.map_info var_ptr(%[[ARGADDR]] : !fir.ref<!fir.array<?xi32>>)   map_clauses(tofrom) capture(ByRef) bounds(%[[BOUNDS]]) -> !fir.ref<!fir.array<?xi32>> {name = "arr_read_write(2:5)"}
+!DEVICE: %[[MAP:.*]] = omp.map_info var_ptr(%[[ARGADDR]] : !fir.ref<!fir.array<?xi32>>, !fir.array<?xi32>)   map_clauses(tofrom) capture(ByRef) bounds(%[[BOUNDS]]) -> !fir.ref<!fir.array<?xi32>> {name = "arr_read_write(2:5)"}
 !DEVICE: omp.target   map_entries(%[[MAP]] : !fir.ref<!fir.array<?xi32>>) {
 
 !HOST-LABEL: func.func @_QMassumed_array_routinesPassumed_shape_array(
@@ -69,7 +69,7 @@ module assumed_array_routines
 !HOST: %[[C4:.*]] = arith.constant 4 : index
 !HOST: %[[BOUNDS:.*]] = omp.bounds   lower_bound(%[[C3]] : index) upper_bound(%[[C4]] : index) stride(%[[C2]]#2 : index) start_idx(%[[C0]] : index) {stride_in_bytes = true}
 !HOST: %[[ADDROF:.*]] = fir.box_addr %arg0 : (!fir.box<!fir.array<?xi32>>) -> !fir.ref<!fir.array<?xi32>>
-!HOST: %[[MAP:.*]] = omp.map_info var_ptr(%[[ADDROF]] : !fir.ref<!fir.array<?xi32>>)   map_clauses(tofrom) capture(ByRef) bounds(%[[BOUNDS]]) -> !fir.ref<!fir.array<?xi32>> {name = "arr_read_write(2:5)"}
+!HOST: %[[MAP:.*]] = omp.map_info var_ptr(%[[ADDROF]] : !fir.ref<!fir.array<?xi32>>, !fir.array<?xi32>)   map_clauses(tofrom) capture(ByRef) bounds(%[[BOUNDS]]) -> !fir.ref<!fir.array<?xi32>> {name = "arr_read_write(2:5)"}
 !HOST: omp.target   map_entries(%[[MAP]] : !fir.ref<!fir.array<?xi32>>) {
     subroutine assumed_shape_array(arr_read_write)
             integer, intent(inout) :: arr_read_write(:)
@@ -88,7 +88,7 @@ end subroutine assumed_shape_array
 !DEVICE: %[[C2:.*]] = arith.constant 1 : index
 !DEVICE: %[[C3:.*]] = arith.constant 1 : index
 !DEVICE: %[[BOUNDS:.*]] = omp.bounds   lower_bound(%[[C0]] : index) upper_bound(%[[C1]] : index) stride(%[[C3]] : index) start_idx(%[[C3]] : index)
-!DEVICE: %[[MAP:.*]] = omp.map_info var_ptr(%[[ARG1]] : !fir.ref<!fir.array<?xi32>>)   map_clauses(tofrom) capture(ByRe...
[truncated]

zero9178

LGTM!

zero9178 · 2023-10-21T10:32:39Z

mlir/lib/Conversion/OpenMPToLLVM/OpenMPToLLVM.cpp

+    // Copy attributes of the curOp except for the typeAttr which should
+    // be converted
+    SmallVector<NamedAttribute> newAttrs;
+    for (auto attr : curOp->getAttrs()) {


Suggested change

for (auto attr : curOp->getAttrs()) {

for (NamedAttribute attr : curOp->getAttrs()) {

nit: expand the auto here

Dinistro

Thanks for working on this.
Would it already be possible to turn all the pointers into opaque ones in tests? This would ensure that no place accidentally relies on the element type.
It might also make sense to do this in a followup revision, though.

If we decide perform this change in a second step, this LGTM!

razvanlupusoru

Nice! MapInfo changes look good to me.

jeanPerier

Thanks for the fast update @kiranchandramohan.

agozillon · 2023-10-23T12:24:49Z

mlir/lib/Conversion/OpenMPToLLVM/OpenMPToLLVM.cpp

+    SmallVector<NamedAttribute> newAttrs;
+    for (auto attr : curOp->getAttrs()) {
+      if (auto typeAttr = dyn_cast<TypeAttr>(attr.getValue())) {
+        auto newAttr = converter->convertType(typeAttr.getValue());


Nit: could also expand the auto here but not important

agozillon

LGTM! Might want to wait on @TIFitis review (and whoever else wants to feed in on it).

Provided it doesn't break any of the tests that explicitly check for the sizes allocated to the OpenMP kernel argument structure after lowering to LLVM-IR then everything should continue to work as before, thank you for looking into this @kiranchandramohan

TIFitis

LGTM 👍🏽

kiranchandramohan · 2023-10-23T15:53:52Z

Would it already be possible to turn all the pointers into opaque ones in tests? This would ensure that no place accidentally relies on the element type.
It might also make sense to do this in a followup revision, though.

If we decide perform this change in a second step, this LGTM!

I will do this in a follow up PR.

kiranchandramohan · 2023-10-23T21:25:44Z

Thanks everyone for the reviews. Will merge this now.

…70057) First half of the tests switching to opaque pointers. Rest of the tests are omptarget-*.mlir and a specific test for typed pointers. Patch created as requested in #69772

kiranchandramohan requested review from clementval, zero9178, gysit, agozillon, jeanPerier, Dinistro and TIFitis October 20, 2023 20:28

llvmbot added mlir:llvm mlir flang Flang issues not falling into any other category mlir:openmp flang:fir-hlfir flang:openmp labels Oct 20, 2023

zero9178 approved these changes Oct 21, 2023

View reviewed changes

Dinistro approved these changes Oct 21, 2023

View reviewed changes

razvanlupusoru approved these changes Oct 23, 2023

View reviewed changes

jeanPerier approved these changes Oct 23, 2023

View reviewed changes

agozillon reviewed Oct 23, 2023

View reviewed changes

agozillon approved these changes Oct 23, 2023

View reviewed changes

TIFitis approved these changes Oct 23, 2023

View reviewed changes

Remove auto usage

39fc2a8

kiranchandramohan merged commit 0235cd7 into llvm:main Oct 23, 2023

jeanPerier mentioned this pull request Oct 24, 2023

[flang][codegen] Update FIR codegen to use mlir.llvm opaque pointers #69692

Merged

kiranchandramohan mentioned this pull request Oct 24, 2023

[OpenMP][MLIR] Use opaque pointers in OpenMP translation tests 1/2 #70057

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[MLIR,Flang,OpenMP] Remove usage of getElementType in OpenMPTranslation #69772

[MLIR,Flang,OpenMP] Remove usage of getElementType in OpenMPTranslation #69772

Uh oh!

kiranchandramohan commented Oct 20, 2023

Uh oh!

llvmbot commented Oct 20, 2023 •

edited

Loading

Uh oh!

zero9178 left a comment

Uh oh!

zero9178 Oct 21, 2023

Uh oh!

kiranchandramohan Oct 23, 2023

Uh oh!

Dinistro left a comment •

edited

Loading

Uh oh!

razvanlupusoru left a comment

Uh oh!

jeanPerier left a comment

Uh oh!

agozillon Oct 23, 2023

Uh oh!

kiranchandramohan Oct 23, 2023

Uh oh!

agozillon left a comment

Uh oh!

TIFitis left a comment

Uh oh!

kiranchandramohan commented Oct 23, 2023

Uh oh!

kiranchandramohan commented Oct 23, 2023

Uh oh!

Uh oh!

	for (auto attr : curOp->getAttrs()) {
	for (NamedAttribute attr : curOp->getAttrs()) {

[MLIR,Flang,OpenMP] Remove usage of getElementType in OpenMPTranslation #69772

[MLIR,Flang,OpenMP] Remove usage of getElementType in OpenMPTranslation #69772

Uh oh!

Conversation

kiranchandramohan commented Oct 20, 2023

Uh oh!

llvmbot commented Oct 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zero9178 left a comment

Choose a reason for hiding this comment

Uh oh!

zero9178 Oct 21, 2023

Choose a reason for hiding this comment

Uh oh!

kiranchandramohan Oct 23, 2023

Choose a reason for hiding this comment

Uh oh!

Dinistro left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

razvanlupusoru left a comment

Choose a reason for hiding this comment

Uh oh!

jeanPerier left a comment

Choose a reason for hiding this comment

Uh oh!

agozillon Oct 23, 2023

Choose a reason for hiding this comment

Uh oh!

kiranchandramohan Oct 23, 2023

Choose a reason for hiding this comment

Uh oh!

agozillon left a comment

Choose a reason for hiding this comment

Uh oh!

TIFitis left a comment

Choose a reason for hiding this comment

Uh oh!

kiranchandramohan commented Oct 23, 2023

Uh oh!

kiranchandramohan commented Oct 23, 2023

Uh oh!

Uh oh!

llvmbot commented Oct 20, 2023 •

edited

Loading

Dinistro left a comment •

edited

Loading