You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
[MLIR][OpenMP] Normalize representation of entry block arg-defining clauses
This patch updates printing and parsing of operations including clauses that
define entry block arguments to the operation's region. This impacts
`in_reduction`, `map`, `private`, `reduction` and `task_reduction`.
The proposed representation to be used by all such clauses is the following:
```
<clause_name>([byref] [@<sym>] %value -> %block_arg [, ...] : <type>[, ...]) {
...
}
```
The `byref` tag is only allowed for reduction-like clauses and the `@<sym>` is
required and only allowed for the `private` and reduction-like clauses. The
`map` clause does not accept any of these two.
This change fixes some currently broken op representations, like `omp.teams` or
`omp.sections` reduction:
```
omp.teams reduction([byref] @<sym> -> %value : <type>) {
^bb0(%block_arg : <type>):
...
}
```
Additionally, it addresses some redundancy in the representation of the
previously mentioned cases, as well as e.g. `map` in `omp.target`. The problem
is that the block argument name after the arrow is not checked in any way,
which makes some misleading representations legal:
```mlir
omp.target map_entries(%x -> %arg1, %y -> %arg0, %z -> %doesnt_exist : !llvm.ptr, !llvm.ptr, !llvm.ptr) {
^bb0(%arg0 : !llvm.ptr, %arg1 : !llvm.ptr, %arg2 : !llvm.ptr):
...
}
```
In that case, `%x` maps to `%arg0`, contrary to what the representation states,
and `%z` maps to `%arg2`. `%doesnt_exist` is not resolved, so it would likely
cause issues if used anywhere inside of the operation's region.
The solution implemented in this patch makes it so that values introduced after
the arrow on the representation of these clauses implicitly define the
corresponding entry block arguments, removing the potential for these
problematic representations. This is what is already implemented for the
`private` and `reduction` clauses of `omp.parallel`.
There are a couple of consequences of this change:
- Entry block argument-defining clauses must come at the end of the operation's
representation and in alphabetical order. This is because they are
printed/parsed as part of the region and a standardized ordering is needed to
reliably match op arguments with their corresponding entry block arguments via
the `BlockArgOpenMPOpInterface`.
- We can no longer define per-clause assembly formats to be reused by all
operations that take these clauses, since they must be passed to a custom
printer including the region and arguments of all other entry block
argument-defining clauses. Code duplication and potential for introducing
issues is minimized by providing the generic `{print,parse}BlockArgRegion`
helpers and associated structures.
MLIR and Flang lowering unit tests are updated due to changes in the order and
formatting of impacted operations.
0 commit comments