Skip to content

Commit 4bd3ea1

Browse files
TIFitisTIFitis
committed
Address reviewer comments. Update test cases with omp.composite checks.
1 parent ba8cf35 commit 4bd3ea1

File tree

4 files changed

+224
-48
lines changed

4 files changed

+224
-48
lines changed

‎flang/lib/Lower/OpenMP/OpenMP.cpp

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2063,14 +2063,12 @@ static void genCompositeDistributeSimd(
20632063
// TODO: Populate entry block arguments with private variables.
20642064
auto distributeOp = genWrapperOp<mlir::omp::DistributeOp>(
20652065
converter, loc, distributeClauseOps, /*blockArgTypes=*/{});
2066-
llvm::cast<mlir::omp::ComposableOpInterface>(distributeOp.getOperation())
2067-
.setComposite(/*val=*/true);
2066+
distributeOp.setComposite(/*val=*/true);
20682067

20692068
// TODO: Populate entry block arguments with reduction and private variables.
20702069
auto simdOp = genWrapperOp<mlir::omp::SimdOp>(converter, loc, simdClauseOps,
20712070
/*blockArgTypes=*/{});
2072-
llvm::cast<mlir::omp::ComposableOpInterface>(simdOp.getOperation())
2073-
.setComposite(/*val=*/true);
2071+
simdOp.setComposite(/*val=*/true);
20742072

20752073
// Construct wrapper entry block list and associated symbols. It is important
20762074
// that the symbol order and the block argument order match, so that the
@@ -2115,14 +2113,12 @@ static void genCompositeDoSimd(lower::AbstractConverter &converter,
21152113
// TODO: Add private variables to entry block arguments.
21162114
auto wsloopOp = genWrapperOp<mlir::omp::WsloopOp>(
21172115
converter, loc, wsloopClauseOps, wsloopReductionTypes);
2118-
llvm::cast<mlir::omp::ComposableOpInterface>(wsloopOp.getOperation())
2119-
.setComposite(/*val=*/true);
2116+
wsloopOp.setComposite(/*val=*/true);
21202117

21212118
// TODO: Populate entry block arguments with reduction and private variables.
21222119
auto simdOp = genWrapperOp<mlir::omp::SimdOp>(converter, loc, simdClauseOps,
21232120
/*blockArgTypes=*/{});
2124-
llvm::cast<mlir::omp::ComposableOpInterface>(simdOp.getOperation())
2125-
.setComposite(/*val=*/true);
2121+
simdOp.setComposite(/*val=*/true);
21262122

21272123
// Construct wrapper entry block list and associated symbols. It is important
21282124
// that the symbol and block argument order match, so that the symbol-value

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

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1546,7 +1546,7 @@ LogicalResult ParallelOp::verify() {
15461546
if (!isWrapper())
15471547
return emitOpError() << "must take a loop wrapper role if nested inside "
15481548
"of 'omp.distribute'";
1549-
if (!llvm::cast<ComposableOpInterface>(getOperation()).isComposite())
1549+
if (!isComposite())
15501550
return emitError()
15511551
<< "'omp.composite' attribute missing from composite wrapper";
15521552

@@ -1558,7 +1558,7 @@ LogicalResult ParallelOp::verify() {
15581558
} else {
15591559
return emitOpError() << "must not wrap an 'omp.loop_nest' directly";
15601560
}
1561-
} else if (llvm::cast<ComposableOpInterface>(getOperation()).isComposite()) {
1561+
} else if (isComposite()) {
15621562
return emitError()
15631563
<< "'omp.composite' attribute present in non-composite wrapper";
15641564
}
@@ -1754,8 +1754,13 @@ LogicalResult WsloopOp::verify() {
17541754
if (!isWrapper())
17551755
return emitOpError() << "must be a loop wrapper";
17561756

1757+
auto wrapper =
1758+
llvm::dyn_cast_if_present<LoopWrapperInterface>((*this)->getParentOp());
1759+
bool isCompositeWrapper = wrapper && wrapper.isWrapper() &&
1760+
(!llvm::isa<ParallelOp>(wrapper) ||
1761+
llvm::isa<DistributeOp>(wrapper->getParentOp()));
17571762
if (LoopWrapperInterface nested = getNestedWrapper()) {
1758-
if (!llvm::cast<ComposableOpInterface>(getOperation()).isComposite())
1763+
if (!isComposite())
17591764
return emitError()
17601765
<< "'omp.composite' attribute missing from composite wrapper";
17611766

@@ -1764,9 +1769,12 @@ LogicalResult WsloopOp::verify() {
17641769
if (!isa<SimdOp>(nested))
17651770
return emitError() << "only supported nested wrapper is 'omp.simd'";
17661771

1767-
} else if (llvm::cast<ComposableOpInterface>(getOperation()).isComposite()) {
1772+
} else if (isComposite() && !isCompositeWrapper) {
17681773
return emitError()
17691774
<< "'omp.composite' attribute present in non-composite wrapper";
1775+
} else if (!isComposite() && isCompositeWrapper) {
1776+
return emitError()
1777+
<< "'omp.composite' attribute missing from composite wrapper";
17701778
}
17711779

17721780
return verifyReductionVarList(*this, getReductionSyms(), getReductionVars(),
@@ -1810,7 +1818,17 @@ LogicalResult SimdOp::verify() {
18101818
if (getNestedWrapper())
18111819
return emitOpError() << "must wrap an 'omp.loop_nest' directly";
18121820

1813-
if (llvm::cast<ComposableOpInterface>(getOperation()).isComposite())
1821+
auto wrapper =
1822+
llvm::dyn_cast_if_present<LoopWrapperInterface>((*this)->getParentOp());
1823+
bool isCompositeWrapper = wrapper && wrapper.isWrapper() &&
1824+
(!llvm::isa<ParallelOp>(wrapper) ||
1825+
llvm::isa<DistributeOp>(wrapper->getParentOp()));
1826+
1827+
if (!isComposite() && isCompositeWrapper)
1828+
return emitError()
1829+
<< "'omp.composite' attribute missing from composite wrapper";
1830+
1831+
if (isComposite() && !isCompositeWrapper)
18141832
return emitError()
18151833
<< "'omp.composite' attribute present in non-composite wrapper";
18161834

@@ -1843,15 +1861,15 @@ LogicalResult DistributeOp::verify() {
18431861
return emitOpError() << "must be a loop wrapper";
18441862

18451863
if (LoopWrapperInterface nested = getNestedWrapper()) {
1846-
if (!llvm::cast<ComposableOpInterface>(getOperation()).isComposite())
1864+
if (!isComposite())
18471865
return emitError()
18481866
<< "'omp.composite' attribute missing from composite wrapper";
18491867
// Check for the allowed leaf constructs that may appear in a composite
18501868
// construct directly after DISTRIBUTE.
18511869
if (!isa<ParallelOp, SimdOp>(nested))
18521870
return emitError() << "only supported nested wrappers are 'omp.parallel' "
18531871
"and 'omp.simd'";
1854-
} else if (llvm::cast<ComposableOpInterface>(getOperation()).isComposite()) {
1872+
} else if (isComposite()) {
18551873
return emitError()
18561874
<< "'omp.composite' attribute present in non-composite wrapper";
18571875
}
@@ -2055,15 +2073,15 @@ LogicalResult TaskloopOp::verify() {
20552073
return emitOpError() << "must be a loop wrapper";
20562074

20572075
if (LoopWrapperInterface nested = getNestedWrapper()) {
2058-
if (!llvm::cast<ComposableOpInterface>(getOperation()).isComposite())
2076+
if (!isComposite())
20592077
return emitError()
20602078
<< "'omp.composite' attribute missing from composite wrapper";
20612079

20622080
// Check for the allowed leaf constructs that may appear in a composite
20632081
// construct directly after TASKLOOP.
20642082
if (!isa<SimdOp>(nested))
20652083
return emitError() << "only supported nested wrapper is 'omp.simd'";
2066-
} else if (llvm::cast<ComposableOpInterface>(getOperation()).isComposite()) {
2084+
} else if (isComposite()) {
20672085
return emitError()
20682086
<< "'omp.composite' attribute present in non-composite wrapper";
20692087
}

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

Lines changed: 169 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,9 @@ func.func @invalid_nested_wrapper(%lb : index, %ub : index, %step : index) {
3636
omp.terminator
3737
}
3838
omp.terminator
39-
}
39+
} {omp.composite}
4040
omp.terminator
41-
}
41+
} {omp.composite}
4242

4343
return
4444
}
@@ -53,9 +53,9 @@ func.func @no_nested_wrapper(%lb : index, %ub : index, %step : index) {
5353
omp.yield
5454
}
5555
omp.terminator
56-
}
56+
} {omp.composite}
5757
omp.terminator
58-
}
58+
} {omp.composite}
5959

6060
return
6161
}
@@ -210,7 +210,7 @@ func.func @invalid_nested_wrapper(%lb : index, %ub : index, %step : index) {
210210
omp.terminator
211211
}
212212
omp.terminator
213-
}
213+
} {omp.composite}
214214
}
215215

216216
// -----
@@ -1965,7 +1965,7 @@ func.func @taskloop(%lb: i32, %ub: i32, %step: i32) {
19651965
omp.terminator
19661966
}
19671967
omp.terminator
1968-
}
1968+
} {omp.composite}
19691969
return
19701970
}
19711971

@@ -2173,7 +2173,7 @@ func.func @omp_distribute_nested_wrapper(%lb: index, %ub: index, %step: index) -
21732173
"omp.terminator"() : () -> ()
21742174
}) : () -> ()
21752175
"omp.terminator"() : () -> ()
2176-
}
2176+
} {omp.composite}
21772177
}
21782178

21792179
// -----
@@ -2383,3 +2383,165 @@ func.func @masked_arg_count_mismatch(%arg0: i32, %arg1: i32) {
23832383
}) : (i32, i32) -> ()
23842384
return
23852385
}
2386+
2387+
// -----
2388+
func.func @omp_parallel_missing_composite(%lb: index, %ub: index, %step: index) -> () {
2389+
omp.distribute {
2390+
// expected-error@+1 {{'omp.composite' attribute missing from composite wrapper}}
2391+
omp.parallel {
2392+
omp.wsloop {
2393+
omp.loop_nest (%iv) : index = (%lb) to (%ub) step (%step) {
2394+
omp.yield
2395+
}
2396+
omp.terminator
2397+
} {omp.composite}
2398+
omp.terminator
2399+
}
2400+
omp.terminator
2401+
} {omp.composite}
2402+
return
2403+
}
2404+
2405+
// -----
2406+
func.func @omp_parallel_invalid_composite(%lb: index, %ub: index, %step: index) -> () {
2407+
// expected-error @below {{'omp.composite' attribute present in non-composite wrapper}}
2408+
omp.parallel {
2409+
omp.wsloop {
2410+
omp.loop_nest (%iv) : index = (%lb) to (%ub) step (%step) {
2411+
omp.yield
2412+
}
2413+
omp.terminator
2414+
}
2415+
omp.terminator
2416+
} {omp.composite}
2417+
return
2418+
}
2419+
2420+
// -----
2421+
func.func @omp_wsloop_missing_composite(%lb: index, %ub: index, %step: index) -> () {
2422+
// expected-error @below {{'omp.composite' attribute missing from composite wrapper}}
2423+
omp.wsloop {
2424+
omp.simd {
2425+
omp.loop_nest (%iv) : index = (%lb) to (%ub) step (%step) {
2426+
omp.yield
2427+
}
2428+
omp.terminator
2429+
} {omp.composite}
2430+
omp.terminator
2431+
}
2432+
return
2433+
}
2434+
2435+
// -----
2436+
func.func @omp_wsloop_invalid_composite(%lb: index, %ub: index, %step: index) -> () {
2437+
// expected-error @below {{'omp.composite' attribute present in non-composite wrapper}}
2438+
omp.wsloop {
2439+
omp.loop_nest (%iv) : index = (%lb) to (%ub) step (%step) {
2440+
omp.yield
2441+
}
2442+
omp.terminator
2443+
} {omp.composite}
2444+
return
2445+
}
2446+
2447+
// -----
2448+
func.func @omp_wsloop_missing_composite_2(%lb: index, %ub: index, %step: index) -> () {
2449+
omp.distribute {
2450+
omp.parallel {
2451+
// expected-error @below {{'omp.composite' attribute missing from composite wrapper}}
2452+
omp.wsloop {
2453+
omp.loop_nest (%iv) : index = (%lb) to (%ub) step (%step) {
2454+
omp.yield
2455+
}
2456+
omp.terminator
2457+
}
2458+
omp.terminator
2459+
} {omp.composite}
2460+
omp.terminator
2461+
} {omp.composite}
2462+
return
2463+
}
2464+
2465+
// -----
2466+
func.func @omp_simd_missing_composite(%lb: index, %ub: index, %step: index) -> () {
2467+
omp.wsloop {
2468+
// expected-error @below {{'omp.composite' attribute missing from composite wrapper}}
2469+
omp.simd {
2470+
omp.loop_nest (%iv) : index = (%lb) to (%ub) step (%step) {
2471+
omp.yield
2472+
}
2473+
omp.terminator
2474+
}
2475+
omp.terminator
2476+
} {omp.composite}
2477+
return
2478+
}
2479+
2480+
// -----
2481+
func.func @omp_simd_invalid_composite(%lb: index, %ub: index, %step: index) -> () {
2482+
// expected-error @below {{'omp.composite' attribute present in non-composite wrapper}}
2483+
omp.simd {
2484+
omp.loop_nest (%iv) : index = (%lb) to (%ub) step (%step) {
2485+
omp.yield
2486+
}
2487+
omp.terminator
2488+
} {omp.composite}
2489+
return
2490+
}
2491+
2492+
// -----
2493+
func.func @omp_distribute_missing_composite(%lb: index, %ub: index, %step: index) -> () {
2494+
// expected-error @below {{'omp.composite' attribute missing from composite wrapper}}
2495+
omp.distribute {
2496+
omp.parallel {
2497+
omp.wsloop {
2498+
omp.loop_nest (%iv) : index = (%lb) to (%ub) step (%step) {
2499+
omp.yield
2500+
}
2501+
omp.terminator
2502+
} {omp.composite}
2503+
omp.terminator
2504+
} {omp.composite}
2505+
omp.terminator
2506+
}
2507+
return
2508+
}
2509+
2510+
// -----
2511+
func.func @omp_distribute_invalid_composite(%lb: index, %ub: index, %step: index) -> () {
2512+
// expected-error @below {{'omp.composite' attribute present in non-composite wrapper}}
2513+
omp.distribute {
2514+
omp.loop_nest (%0) : index = (%lb) to (%ub) step (%step) {
2515+
omp.yield
2516+
}
2517+
omp.terminator
2518+
} {omp.composite}
2519+
return
2520+
}
2521+
2522+
// -----
2523+
func.func @omp_taskloop_missing_composite(%lb: index, %ub: index, %step: index) -> () {
2524+
// expected-error @below {{'omp.composite' attribute missing from composite wrapper}}
2525+
omp.taskloop {
2526+
omp.simd {
2527+
omp.loop_nest (%i) : index = (%lb) to (%ub) step (%step) {
2528+
omp.yield
2529+
}
2530+
omp.terminator
2531+
} {omp.composite}
2532+
omp.terminator
2533+
}
2534+
return
2535+
}
2536+
2537+
// -----
2538+
func.func @omp_taskloop_invalid_composite(%lb: index, %ub: index, %step: index) -> () {
2539+
// expected-error @below {{'omp.composite' attribute present in non-composite wrapper}}
2540+
omp.taskloop {
2541+
omp.loop_nest (%i) : index = (%lb) to (%ub) step (%step) {
2542+
omp.yield
2543+
}
2544+
omp.terminator
2545+
} {omp.composite}
2546+
return
2547+
}

0 commit comments

Comments
 (0)