[NFC][OpenMP][MLIR] Add MLIR test for lowering parallel if #71788

DominikAdamski · 2023-11-09T09:36:33Z

Add test for clause omp target parallel if (parallel : cond )

Test checks if corresponding MLIR construct is correctly lowered to LLVM IR.

llvmbot · 2023-11-09T09:37:06Z

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-llvm

Author: Dominik Adamski (DominikAdamski)

Changes

Add test for clause omp target parallel if (parallel : cond )

Test checks if corresponding MLIR construct is correctly lowered to LLVM IR.

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

1 Files Affected:

(modified) mlir/test/Target/LLVMIR/omptarget-parallel-llvm.mlir (+30)

diff --git a/mlir/test/Target/LLVMIR/omptarget-parallel-llvm.mlir b/mlir/test/Target/LLVMIR/omptarget-parallel-llvm.mlir
index 4457d7f4275260d..160b56f42edf440 100644
--- a/mlir/test/Target/LLVMIR/omptarget-parallel-llvm.mlir
+++ b/mlir/test/Target/LLVMIR/omptarget-parallel-llvm.mlir
@@ -17,6 +17,26 @@ module attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<"dlti.alloca_memo
     }
   llvm.return
   }
+
+  llvm.func @parallel_if(%arg0: !llvm.ptr {fir.bindc_name = "ifcond"}) {
+    %0 = llvm.mlir.constant(1 : i64) : i64
+    %1 = llvm.alloca %0 x i32 {bindc_name = "d"} : (i64) -> !llvm.ptr
+    %2 = omp.map_info var_ptr(%1 : !llvm.ptr, i32) map_clauses(from) capture(ByRef) -> !llvm.ptr {name = "d"}
+    %3 = omp.map_info var_ptr(%arg0 : !llvm.ptr, i32) map_clauses(implicit, exit_release_or_enter_alloc) capture(ByCopy) -> !llvm.ptr {name = "ifcond"}
+    omp.target map_entries(%2 -> %arg1, %3 -> %arg2 : !llvm.ptr, !llvm.ptr) {
+    ^bb0(%arg1: !llvm.ptr, %arg2: !llvm.ptr):
+      %4 = llvm.mlir.constant(10 : i32) : i32
+      %5 = llvm.load %arg2 : !llvm.ptr -> i32
+      %6 = llvm.mlir.constant(0 : i64) : i32
+      %7 = llvm.icmp "ne" %5, %6 : i32
+      omp.parallel if(%7 : i1) {
+        llvm.store %4, %arg1 : i32, !llvm.ptr
+        omp.terminator
+      }
+      omp.terminator
+    }
+    llvm.return
+  }
 }
 
 // CHECK: define weak_odr protected amdgpu_kernel void [[FUNC0:@.*]](
@@ -43,3 +63,13 @@ module attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<"dlti.alloca_memo
 // CHECK: define internal void [[FUNC1]](
 // CHECK-SAME: ptr noalias noundef [[TID_ADDR_ASCAST:%.*]], ptr noalias noundef [[ZERO_ADDR_ASCAST:%.*]], ptr [[TMP0:%.*]]) #[[ATTR0:[0-9]+]] {
 
+// CHECK: define weak_odr protected amdgpu_kernel void [[FUNC2:@.*]](
+// CHECK-SAME: ptr [[IFCOND_ARG0:%.*]], ptr [[IFCOND_ARG1:.*]], ptr [[IFCOND_ARG2:.*]]) {
+// CHECK:         store ptr [[IFCOND_ARG2]], ptr [[IFCOND_TMP1:%.]], align 8
+// CHECK:         [[IFCOND_TMP2:%.*]] = load i32, ptr [[IFCOND_TMP1]], align 4
+// CHECK:         [[IFCOND_TMP3:%.*]] = icmp ne i32 [[IFCOND_TMP2]], 0
+// CHECK:         [[IFCOND_TMP4:%.*]] = sext i1 [[IFCOND_TMP3]] to i32
+// CHECK:         call void @__kmpc_parallel_51(ptr addrspacecast (
+// CHECK-SAME:  ptr addrspace(1) @[[IFCOND_GLOB:[0-9]+]] to ptr),
+// CHECK-SAME:  i32 [[IFCOND_THREAD_NUM:%.*]], i32 [[IFCOND_TMP4]], i32 -1,
+// CHECK-SAME:  i32 -1,  ptr [[FUNC1:@.*]], ptr null, ptr [[IFCOND_TMP5:%.*]], i64 1)

Add test for clause omp target parallel if (parallel : cond ) Test checks if corresponding MLIR construct is correctly lowered to LLVM IR.

kiranchandramohan · 2023-11-16T12:59:03Z

Can you add some more comments about parallel if and what is difference expected in the test?

DominikAdamski · 2023-11-17T13:31:41Z

I added comments in test file. They explain the purpose of given test.

kiranchandramohan

I think, generally, the % in the mlir SSA value is not captured usually. Only the name or number is captured.

// CHECK:         %[[VAL_0:.*]] = alloca [1 x ptr], align 8

Also, only capture the values that are required for the checks. Others can be skipped using {{.*}}.

kiranchandramohan · 2023-11-18T11:20:20Z

mlir/test/Target/LLVMIR/omptarget-parallel-llvm.mlir

 // CHECK: define weak_odr protected amdgpu_kernel void [[FUNC_NUM_THREADS0:@.*]](
 // CHECK-NOT:     call void @__kmpc_push_num_threads(
 // CHECK:         call void @__kmpc_parallel_51(ptr addrspacecast (
 // CHECK-SAME:  ptr addrspace(1) @[[NUM_THREADS_GLOB:[0-9]+]] to ptr),
 // CHECK-SAME:  i32 [[NUM_THREADS_TMP0:%.*]], i32 1, i32 156,
 // CHECK-SAME:  i32 -1,  ptr [[FUNC_NUM_THREADS1:@.*]], ptr null, ptr [[NUM_THREADS_TMP1:%.*]], i64 1)
+
+// The one of arguments of  kmpc_parallel_51 function is responsible for handling if clause


Suggested change

// The one of arguments of kmpc_parallel_51 function is responsible for handling if clause

// One of the arguments of kmpc_parallel_51 function is responsible for handling if clause

kiranchandramohan · 2023-11-18T11:21:23Z

mlir/test/Target/LLVMIR/omptarget-parallel-llvm.mlir

+// is correctly lowered to LLVM IR code and the if condition variable
+// is passed as a param to kmpc_parallel_51 function
+
+// CHECK: define weak_odr protected amdgpu_kernel void [[FUNC2:@.*]](


How did this amdgpu_kernel name appear? Is that from the target triple?

Yes, please see function void OpenMPIRBuilder::setOutlinedTargetRegionFunctionAttributes: https://github.com/llvm/llvm-project/blob/main/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp#L4489

kiranchandramohan · 2023-11-18T11:23:31Z

mlir/test/Target/LLVMIR/omptarget-parallel-llvm.mlir

+
+// CHECK: define weak_odr protected amdgpu_kernel void [[FUNC2:@.*]](
+// CHECK-SAME: ptr [[IFCOND_ARG0:%.*]], ptr [[IFCOND_ARG1:.*]], ptr [[IFCOND_ARG2:.*]]) {
+// CHECK:         store ptr [[IFCOND_ARG2]], ptr [[IFCOND_TMP1:%.]], align 8


Suggested change

// CHECK: store ptr [[IFCOND_ARG2]], ptr [[IFCOND_TMP1:%.]], align 8

// CHECK: store ptr [[IFCOND_ARG2]], ptr [[IFCOND_TMP1:%.*]], align 8

kiranchandramohan · 2023-11-18T11:27:35Z

mlir/test/Target/LLVMIR/omptarget-parallel-llvm.mlir

+
+// CHECK: define weak_odr protected amdgpu_kernel void [[FUNC2:@.*]](
+// CHECK-SAME: ptr [[IFCOND_ARG0:%.*]], ptr [[IFCOND_ARG1:.*]], ptr [[IFCOND_ARG2:.*]]) {
+// CHECK:         store ptr [[IFCOND_ARG2]], ptr [[IFCOND_TMP1:%.]], align 8


Looks like IFCOND_ARG2 is the important argument here. If others are not used then no need to capture them.

DominikAdamski · 2023-11-23T09:39:20Z

I think, generally, the % in the mlir SSA value is not captured usually. Only the name or number is captured.
// CHECK:         %[[VAL_0:.*]] = alloca [1 x ptr], align 8
Also, only capture the values that are required for the checks. Others can be skipped using {{.*}}.

Done

kiranchandramohan

LG.

DominikAdamski requested review from kiranchandramohan, agozillon, dpalermo and kiranktp November 9, 2023 09:36

llvmbot added mlir:llvm mlir labels Nov 9, 2023

DominikAdamski requested a review from skatrak November 13, 2023 13:12

[NFC][OpenMP][MLIR] Add MLIR test for lowering parallel if

6682a44

Add test for clause omp target parallel if (parallel : cond ) Test checks if corresponding MLIR construct is correctly lowered to LLVM IR.

DominikAdamski force-pushed the target_parallel_if_mlir_test branch from f397391 to 6682a44 Compare November 13, 2023 13:27

Added comments

1116ab8

kiranchandramohan reviewed Nov 18, 2023

View reviewed changes

Applied comments

15da88c

kiranchandramohan approved these changes Nov 23, 2023

View reviewed changes

DominikAdamski merged commit bda723f into llvm:main Nov 23, 2023

DominikAdamski deleted the target_parallel_if_mlir_test branch November 23, 2023 12:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[NFC][OpenMP][MLIR] Add MLIR test for lowering parallel if #71788

[NFC][OpenMP][MLIR] Add MLIR test for lowering parallel if #71788

Uh oh!

DominikAdamski commented Nov 9, 2023

Uh oh!

llvmbot commented Nov 9, 2023 •

edited

Loading

Uh oh!

kiranchandramohan commented Nov 16, 2023

Uh oh!

DominikAdamski commented Nov 17, 2023

Uh oh!

kiranchandramohan left a comment

Uh oh!

kiranchandramohan Nov 18, 2023

Uh oh!

DominikAdamski Nov 23, 2023

Uh oh!

kiranchandramohan Nov 18, 2023

Uh oh!

DominikAdamski Nov 23, 2023

Uh oh!

kiranchandramohan Nov 18, 2023

Uh oh!

DominikAdamski Nov 23, 2023

Uh oh!

kiranchandramohan Nov 18, 2023

Uh oh!

DominikAdamski Nov 23, 2023

Uh oh!

DominikAdamski commented Nov 23, 2023

Uh oh!

kiranchandramohan left a comment

Uh oh!

Uh oh!

	// The one of arguments of kmpc_parallel_51 function is responsible for handling if clause
	// One of the arguments of kmpc_parallel_51 function is responsible for handling if clause

	// CHECK: store ptr [[IFCOND_ARG2]], ptr [[IFCOND_TMP1:%.]], align 8
	// CHECK: store ptr [[IFCOND_ARG2]], ptr [[IFCOND_TMP1:%.*]], align 8

[NFC][OpenMP][MLIR] Add MLIR test for lowering parallel if #71788

[NFC][OpenMP][MLIR] Add MLIR test for lowering parallel if #71788

Uh oh!

Conversation

DominikAdamski commented Nov 9, 2023

Uh oh!

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

Uh oh!

Uh oh!

kiranchandramohan commented Nov 16, 2023

Uh oh!

DominikAdamski commented Nov 17, 2023

Uh oh!

kiranchandramohan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DominikAdamski commented Nov 23, 2023

Uh oh!

kiranchandramohan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

llvmbot commented Nov 9, 2023 •

edited

Loading