Skip to content

Commit 88bb252

Browse files
shraiyshshraiysh
committed
[mlir][OpenMP] Add checks and tests for hint clause and fix empty hint
This patch handles empty hint value for critical and atomic constructs. This also adds checks and tests for hint clause on atomic constructs. Reviewed By: peixin, kiranchandramohan, NimishMishra Differential Revision: https://reviews.llvm.org/D123186
1 parent 7895c87 commit 88bb252

File tree

6 files changed

+476
-30
lines changed

6 files changed

+476
-30
lines changed

‎flang/test/Lower/OpenMP/critical.f90

Lines changed: 19 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,41 +1,36 @@
1-
!RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s --check-prefix="FIRDialect"
2-
!RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | fir-opt --fir-to-llvm-ir | FileCheck %s --check-prefix="LLVMDialect"
1+
!RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s --check-prefixes="OMPDialect"
2+
!RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | fir-opt --fir-to-llvm-ir | FileCheck %s --check-prefix="OMPDialect"
33
!RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | fir-opt --fir-to-llvm-ir | tco | FileCheck %s --check-prefix="LLVMIR"
44

5+
!OMPDialect: omp.critical.declare @help2 hint(none)
6+
!OMPDialect: omp.critical.declare @help1 hint(contended)
7+
58
subroutine omp_critical()
69
use omp_lib
710
integer :: x, y
8-
!FIRDialect: omp.critical.declare @help hint(contended)
9-
!LLVMDialect: omp.critical.declare @help hint(contended)
10-
!FIRDialect: omp.critical(@help)
11-
!LLVMDialect: omp.critical(@help)
12-
!LLVMIR: call void @__kmpc_critical_with_hint({{.*}}, {{.*}}, {{.*}} @{{.*}}help.var, i32 2)
13-
!$OMP CRITICAL(help) HINT(omp_lock_hint_contended)
11+
!OMPDialect: omp.critical(@help1)
12+
!LLVMIR: call void @__kmpc_critical_with_hint({{.*}}, {{.*}}, {{.*}} @{{.*}}help1.var, i32 2)
13+
!$OMP CRITICAL(help1) HINT(omp_lock_hint_contended)
1414
x = x + y
15-
!FIRDialect: omp.terminator
16-
!LLVMDialect: omp.terminator
17-
!LLVMIR: call void @__kmpc_end_critical({{.*}}, {{.*}}, {{.*}} @{{.*}}help.var)
18-
!$OMP END CRITICAL(help)
15+
!OMPDialect: omp.terminator
16+
!LLVMIR: call void @__kmpc_end_critical({{.*}}, {{.*}}, {{.*}} @{{.*}}help1.var)
17+
!$OMP END CRITICAL(help1)
1918

2019
! Test that the same name can be used again
2120
! Also test with the zero hint expression
22-
!FIRDialect: omp.critical(@help)
23-
!LLVMDialect: omp.critical(@help)
24-
!LLVMIR: call void @__kmpc_critical_with_hint({{.*}}, {{.*}}, {{.*}} @{{.*}}help.var, i32 2)
25-
!$OMP CRITICAL(help) HINT(omp_lock_hint_none)
21+
!OMPDialect: omp.critical(@help2)
22+
!LLVMIR: call void @__kmpc_critical_with_hint({{.*}}, {{.*}}, {{.*}} @{{.*}}help2.var, i32 0)
23+
!$OMP CRITICAL(help2) HINT(omp_lock_hint_none)
2624
x = x - y
27-
!FIRDialect: omp.terminator
28-
!LLVMDialect: omp.terminator
29-
!LLVMIR: call void @__kmpc_end_critical({{.*}}, {{.*}}, {{.*}} @{{.*}}help.var)
30-
!$OMP END CRITICAL(help)
25+
!OMPDialect: omp.terminator
26+
!LLVMIR: call void @__kmpc_end_critical({{.*}}, {{.*}}, {{.*}} @{{.*}}help2.var)
27+
!$OMP END CRITICAL(help2)
3128

32-
!FIRDialect: omp.critical
33-
!LLVMDialect: omp.critical
29+
!OMPDialect: omp.critical
3430
!LLVMIR: call void @__kmpc_critical({{.*}}, {{.*}}, {{.*}} @{{.*}}_.var)
3531
!$OMP CRITICAL
3632
y = x + y
37-
!FIRDialect: omp.terminator
38-
!LLVMDialect: omp.terminator
33+
!OMPDialect: omp.terminator
3934
!LLVMIR: call void @__kmpc_end_critical({{.*}}, {{.*}}, {{.*}} @{{.*}}_.var)
4035
!$OMP END CRITICAL
4136
end subroutine omp_critical

‎mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -946,6 +946,7 @@ def AtomicCaptureOp : OpenMP_Op<"atomic.capture",
946946
$region attr-dict
947947
}];
948948
let hasRegionVerifier = 1;
949+
let hasVerifier = 1;
949950
let extraClassDeclaration = [{
950951
/// Returns the first operation in atomic capture region
951952
Operation* getFirstOp();

‎mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -382,6 +382,10 @@ static ParseResult parseSynchronizationHint(OpAsmParser &parser,
382382
IntegerAttr &hintAttr) {
383383
StringRef hintKeyword;
384384
int64_t hint = 0;
385+
if (succeeded(parser.parseOptionalKeyword("none"))) {
386+
hintAttr = IntegerAttr::get(parser.getBuilder().getI64Type(), 0);
387+
return success();
388+
}
385389
do {
386390
if (failed(parser.parseKeyword(&hintKeyword)))
387391
return failure();
@@ -406,8 +410,10 @@ static void printSynchronizationHint(OpAsmPrinter &p, Operation *op,
406410
IntegerAttr hintAttr) {
407411
int64_t hint = hintAttr.getInt();
408412

409-
if (hint == 0)
413+
if (hint == 0) {
414+
p << "none";
410415
return;
416+
}
411417

412418
// Helper function to get n-th bit from the right end of `value`
413419
auto bitn = [](int value, int n) -> bool { return value & (1 << n); };
@@ -864,7 +870,7 @@ LogicalResult AtomicUpdateOp::verify() {
864870
"element type is the same as that of the region argument");
865871
}
866872

867-
return success();
873+
return verifySynchronizationHint(*this, hint_val());
868874
}
869875

870876
LogicalResult AtomicUpdateOp::verifyRegions() {
@@ -915,6 +921,10 @@ AtomicUpdateOp AtomicCaptureOp::getAtomicUpdateOp() {
915921
return dyn_cast<AtomicUpdateOp>(getSecondOp());
916922
}
917923

924+
LogicalResult AtomicCaptureOp::verify() {
925+
return verifySynchronizationHint(*this, hint_val());
926+
}
927+
918928
LogicalResult AtomicCaptureOp::verifyRegions() {
919929
Block::OpListType &ops = region().front().getOperations();
920930
if (ops.size() != 3)
@@ -949,6 +959,10 @@ LogicalResult AtomicCaptureOp::verifyRegions() {
949959
return firstReadStmt.emitError()
950960
<< "captured variable in omp.atomic.read must be updated in "
951961
"second operation";
962+
963+
if (getFirstOp()->getAttr("hint_val") || getSecondOp()->getAttr("hint_val"))
964+
return emitOpError(
965+
"operations inside capture region must not have hint clause");
952966
return success();
953967
}
954968

‎mlir/test/Dialect/OpenMP/invalid.mlir

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -708,6 +708,42 @@ func @omp_atomic_update9(%x: memref<i32>, %expr: i32) {
708708

709709
// -----
710710

711+
func @omp_atomic_update(%x: memref<i32>, %expr: i32) {
712+
// expected-error @below {{the hints omp_sync_hint_uncontended and omp_sync_hint_contended cannot be combined}}
713+
omp.atomic.update hint(uncontended, contended) %x : memref<i32> {
714+
^bb0(%xval: i32):
715+
%newval = llvm.add %xval, %expr : i32
716+
omp.yield (%newval : i32)
717+
}
718+
return
719+
}
720+
721+
// -----
722+
723+
func @omp_atomic_update(%x: memref<i32>, %expr: i32) {
724+
// expected-error @below {{the hints omp_sync_hint_nonspeculative and omp_sync_hint_speculative cannot be combined}}
725+
omp.atomic.update hint(nonspeculative, speculative) %x : memref<i32> {
726+
^bb0(%xval: i32):
727+
%newval = llvm.add %xval, %expr : i32
728+
omp.yield (%newval : i32)
729+
}
730+
return
731+
}
732+
733+
// -----
734+
735+
func @omp_atomic_update(%x: memref<i32>, %expr: i32) {
736+
// expected-error @below {{invalid_hint is not a valid hint}}
737+
omp.atomic.update hint(invalid_hint) %x : memref<i32> {
738+
^bb0(%xval: i32):
739+
%newval = llvm.add %xval, %expr : i32
740+
omp.yield (%newval : i32)
741+
}
742+
return
743+
}
744+
745+
// -----
746+
711747
func @omp_atomic_capture(%x: memref<i32>, %v: memref<i32>, %expr: i32) {
712748
// expected-error @below {{expected three operations in omp.atomic.capture region}}
713749
omp.atomic.capture {
@@ -848,6 +884,66 @@ func @omp_atomic_capture(%x: memref<i32>, %y: memref<i32>, %v: memref<i32>, %exp
848884

849885
// -----
850886

887+
func @omp_atomic_capture(%x: memref<i32>, %v: memref<i32>, %expr: i32) {
888+
// expected-error @below {{the hints omp_sync_hint_uncontended and omp_sync_hint_contended cannot be combined}}
889+
omp.atomic.capture hint(contended, uncontended) {
890+
omp.atomic.update %x : memref<i32> {
891+
^bb0(%xval: i32):
892+
%newval = llvm.add %xval, %expr : i32
893+
omp.yield(%newval : i32)
894+
}
895+
omp.atomic.read %v = %x : memref<i32>
896+
}
897+
return
898+
}
899+
900+
// -----
901+
902+
func @omp_atomic_capture(%x: memref<i32>, %v: memref<i32>, %expr: i32) {
903+
// expected-error @below {{the hints omp_sync_hint_nonspeculative and omp_sync_hint_speculative cannot be combined}}
904+
omp.atomic.capture hint(nonspeculative, speculative) {
905+
omp.atomic.update %x : memref<i32> {
906+
^bb0(%xval: i32):
907+
%newval = llvm.add %xval, %expr : i32
908+
omp.yield(%newval : i32)
909+
}
910+
omp.atomic.read %v = %x : memref<i32>
911+
}
912+
return
913+
}
914+
915+
// -----
916+
917+
func @omp_atomic_capture(%x: memref<i32>, %v: memref<i32>, %expr: i32) {
918+
// expected-error @below {{invalid_hint is not a valid hint}}
919+
omp.atomic.capture hint(invalid_hint) {
920+
omp.atomic.update %x : memref<i32> {
921+
^bb0(%xval: i32):
922+
%newval = llvm.add %xval, %expr : i32
923+
omp.yield(%newval : i32)
924+
}
925+
omp.atomic.read %v = %x : memref<i32>
926+
}
927+
return
928+
}
929+
930+
// -----
931+
932+
func @omp_atomic_capture(%x: memref<i32>, %v: memref<i32>, %expr: i32) {
933+
// expected-error @below {{operations inside capture region must not have hint clause}}
934+
omp.atomic.capture {
935+
omp.atomic.update hint(uncontended) %x : memref<i32> {
936+
^bb0(%xval: i32):
937+
%newval = llvm.add %xval, %expr : i32
938+
omp.yield(%newval : i32)
939+
}
940+
omp.atomic.read %v = %x : memref<i32>
941+
}
942+
return
943+
}
944+
945+
// -----
946+
851947
func @omp_sections(%data_var : memref<i32>) -> () {
852948
// expected-error @below {{expected equal sizes for allocate and allocator variables}}
853949
"omp.sections" (%data_var) ({

0 commit comments

Comments
 (0)