The current code will crash on the MAP clause with OpenMP version >= 6.0
when the clause does not explicitly list any modifiers. The proper fix
is to update the handling of assumed-size arrays for OpenMP 6.0+, but in
the short term keep the behavior from 5.2, just avoid the crash.
The function ResolveOmpObject had a lot of highly-indented code in two
variant visitors. Extract the visitors into their own functions, and
reformat the code. Replace !(||) with !&&! in a couple of places to make
the formatting a bit nicer. Use llvm::enumerate instead of manually
maintaining iteration index.
They were inserted in the current scope.
OpenMP spec (all versions):
The names of critical constructs are global entities of the program. If
a name conflicts with any other entity, the behavior of the program is
unspecified.
Currently, we indicate to the runtime that implicit scalar captures are
firstprivate (via map and
capture types), enough for the runtime trace to treat it as such, but we
do not CodeGen the IR
in such a way that we can take full advantage of this aspect of the
OpenMP specification.
This patch seeks to change that by applying the correct symbol flags
(firstprivate/implicit) to the
implicitly captured scalars within target regions, which then triggers
the delayed privitization code
generation for these symbols, bringing the code generation in-line with
the explicit firstpriviate
clause. Currently, similarly to the delayed privitization I have
sheltered this segment of code
behind the EnabledDelayedPrivitization flag, as without it, we'll
trigger an compiler error for
firstprivate not being supported any time we implicitly capture a scalar
and try to firstprivitize
it, in future when this flag is removed it can also be removed here. So,
for now, you need to
enable this via providing the compiler the flag on compilation of any
programs.
The structure is
- OmpBeginDirective (aka OmpDirectiveSpecification)
- Block
- optional<OmpEndDirective> (aka optional<OmpDirectiveSpecification>)
The OmpBeginDirective and OmpEndDirective are effectively different
names for OmpDirectiveSpecification. They exist to allow the semantic
analyses to distinguish between the beginning and the ending of a block
construct without maintaining additional context.
The actual changes are in the parser: parse-tree.h and openmp-parser.cpp
in particular. The rest is simply changing the way the directive/clause
information is accessed (typically for the simpler).
All standalone and block constructs now use OmpDirectiveSpecification to
store the directive/clause information.
OpenMP 6.0 has changed the modifiers on the MAP clause. Previous patch
has introduced parsing support for them. This patch introduces
processing of the new forms in semantic checks and in lowering. This
only applies to existing modifiers, which were updated in the 6.0 spec.
Any of the newly introduced modifiers (SELF and REF) are ignored.
OpenMP 6.0 has changed the modifiers on the MAP clause:
- map-type-modifier has been split into individual modifiers,
- map-type "delete" has become a modifier,
- new modifiers have been added.
This patch adds parsing support for all of the OpenMP 6.0 modifiers. The
old "map-type-modifier" is retained, but is no longer created in
parsing. It will remain to take advantage of the preexisting modifier
validation for older versions: when the OpenMP version is < 6.0, the
modifiers will be rewritten back as map-type-modifiers (or map- type in
case of "delete").
In this patch the modifiers will always be rewritten in the older format
to isolate these changes to parsing as much as possible.
A report of the following code not generating an error led to fixing two bugs in directive checking.
- We should treat CombinedConstructs as OpenACC Constructs
- We should treat DoConstruct index variables as private.
```fortran
subroutine sub(nn)
integer :: nn, ii
!$acc serial loop default(none)
do ii = 1, nn
end do
!$acc end serial loop
end subroutine
```
Here `nn` should be flagged as needing a data clause while `ii` should
still get one implicitly.
The ALLOCATORS construct is one of the few constructs that require a
special form of the associated block.
Convert the AST node to use OmpDirectiveSpecification for the directive
and the optional end directive, and to use parser::Block as the body:
the form of the block is checked in the semantic checks (with a more
meaningful message).
Basic parsing and semantics support for Declare Variant was added in
#130578. However, this did not include variant of `Pre` and `Post`
within `OmpAttributeVisitor`. This meant that when a function in the
class tried to get the context using `GetContext`, Flang would crash as
the context was empty. To ensure this is possible, such as when
resolving names as part of the `uniform` clause in the `simd` directive,
the context is now pushed within `OmpAttributeVisitor` when parsing a
`DECLARE VARIANT` directive.
Fixes#145222
In OpenMP Version 5.1, the tile and unroll directives were added. When
using these directives, it is possible to nest them within other OpenMP
Loop Constructs. This patch enables the semantics to allow for this
behaviour on these specific directives. Any nested loops will be stored
within the initial Loop Construct until reaching the DoConstruct itself.
Relevant tests have been added, and previous behaviour has been retained
with no changes.
See also, #110008
There has been a number of deprecation warnings that have been added to
Flang, however these features are only deprecated when the OpenMP
Version being used is 5.2 or later. Previously, flang did not consider
the version with the warnings so would always be emitted.
Flang now ensures warnings are emitted for the appropriate version of
OpenMP, and tests are updated to reflect this change.
I don't think DO CONCURRENT fits the definition of a Canonical Loop Nest
(OpenMP 6.0 section 6.4.1).
It is however explicitly allowed for the LOOP construct (6.0 section
13.8).
There's some obscure language in OpenMP 6.0 for the LOOP construct:
> If the collapsed loop is a DO CONCURRENT loop, neither the
> data-sharing attribute clauses nor the collapse clause may be
specified.
From the surrounding context, I think "collapsed loop" just means the
loop that the LOOP construct applies to. So I will interpret this to
mean that DO CONCURRENT can only be used with the LOOP construct if it
does not contain the COLLAPSE clause.
This also fixes a bug where the associated clause was never cleared
after it was set.
Fixes#144178
Previously we didn't push any context for SECTION and they are not
modelled with differing scopes and so goto detection couldn't tell that
GOTOs between two SECTIONs were between constructs rather than just
staying inside of the parent SECTIONS construct.
Fixes#143231
Symbols that have a pre-existing DSA set in the enclosing context should
not be made shared based on them being static duration variables.
Suggested-by: Leandro Lupori <leandro.lupori@linaro.org>
---------
Signed-off-by: Kajetan Puchalski <kajetan.puchalski@arm.com>
According to the OpenMP standard, variables with static storage duration
are predetermined as shared.
Add a check when creating implicit symbols for OpenMP to fix them
erroneously getting set to firstprivate.
Fixes llvm#140732.
---------
Signed-off-by: Kajetan Puchalski <kajetan.puchalski@arm.com>
As part of OpenMP 5.2, the allocate directive has been deprecated in
favour of the allocators construct for Fortran when an ALLOCATE
statement follows the OpenMP allocate directive.
To enable this in flang, a warning has been added informing the user of
this. Tests to ensure this behaviour is continued are also included.
See also: #110008
Before this change, OmpShared was not always set in shared symbols.
Instead, absence of private flags was interpreted as shared DSA.
The problem was that symbols with no flags, with only a host
association, could also mean "has same DSA as in the enclosing
context". Now shared symbols behave the same as private and can be
treated the same way.
Because of the host association symbols with no flags mentioned
above, it was also incorrect to simply test the flags of a given
symbol to find out if it was private or shared. The function
GetSymbolDSA() was added to fix this. It would be better to avoid
the need of these special symbols, but this would require changes
to how symbols are collected in lowering.
Besides that, some semantic checks need to know if a DSA clause
was used or not. To avoid confusing implicit symbols with DSA
clauses a new flag was added: OmpExplicit. It is now set for all
symbols with explicitly determined data-sharing attributes.
With the changes above, AddToContextObjectWithDSA() and the symbol
to DSA map could probably be removed and the DSA could be obtained
directly from the symbol, but this was not attempted.
Some debug messages were also added, with the "omp" DEBUG_TYPE, to
make it easier to debug the creation of implicit symbols and to
visualize all associations of a given symbol.
Fixes#130533Fixes#140882
Add a visitor for OmpClause::Uniform to resolve its parameter names.
Add Symbol::Flag::OmpUniform to attach it to the resolved symbols.
Fixes issue #140741.
---------
Signed-off-by: Kajetan Puchalski <kajetan.puchalski@arm.com>
Fixes#136583
Normally the flush argument list would contain a DataRef to some
variable. All DataRefs are handled generically in resolve-names and so
the problem wasn't observed. But when a common block name is specified,
this is not parsed as a DataRef. There was already handling in
resolve-directives for OmpObjectList but not for argument lists. I've
added a visitor for FLUSH which ensures all of the arguments have been
resolved.
The test is there to make sure the compiler doesn't crashed encountering
the unresolved symbol. It shows that we currently deny flushing a common
block. I'm not sure that it is right to restrict common blocks from
flush argument lists, but fixing that can come in a different patch.
This one is fixing an ICE.
OpenACC routines annotations in separate compilation units currently get
ignored, which leads to errors in compilation. There are two reason for
currently ignoring open acc routine information and this PR is
addressing both.
- The module file reader doesn't read back in openacc directives from
module files.
- Simple fix in `flang/lib/Semantics/mod-file.cpp`
- The lowering to HLFIR doesn't generate routine directives for symbols
imported from other modules that are openacc routines.
- This is the majority of this diff, and is address by the changes that
start in `flang/lib/Lower/CallInterface.cpp`.
This patch,
- Added support for lowering of task_reduction to MLIR
- Added support for lowering of in_reduction to MLIR
- Fixed incorrect DSA handling for variables in the presence of 'in_reduction' clause.
Add "Acquire" and "Release", and rename it to OmpMemoryOrderType, since
memory order type is a concept extending beyond the
ATOMIC_DEFAULT_MEM_ORDER clause.
When processing a REQUIRES directive (in rewrite-directives.cpp), do not
add Acquire or Release to ATOMIC constructs, because handling of those
types depends on the OpenMP version, which is not available in that
file. This issue will be addressed later.
OpenACC declare statements are restricted from having having clauses
that reference assumed size arrays. It should be the case that we can
implement `deviceptr` and `present` clauses for assumed-size arrays.
This is a first step towards relaxing this restriction.
Note running flang on the following example results in an error in
lowering.
```
$ cat t.f90
subroutine vadd (a, b, c, n)
real(8) :: a(*), b(*), c(*)
!$acc declare deviceptr(a, b, c)
!$acc parallel loop
do i = 1,n
c(i) = a(i) + b(i)
enddo
end subroutine
$ flang -fopenacc -c t.f90
error: loc("/home/akuhlenschmi/work/p4/ta/tests/openacc/src/t.f90":3:7): expect declare attribute on variable in declare operation
error: Lowering to LLVM IR failed
error: loc("/home/akuhlenschmi/work/p4/ta/tests/openacc/src/t.f90":4:7): unsupported OpenACC operation: acc.private.recipe
error: loc("/home/akuhlenschmi/work/p4/ta/tests/openacc/src/t.f90":4:7): LLVM Translation failed for operation: acc.private.recipe
error: failed to create the LLVM module
```
I would like to to share this code, because others are currently working
on the implementation of `deviceptr`, but it is obviously not running
end-to-end. I think the cleanest approach to this would be to put this
exception to the rule behind some feature flag, but I am not certain
what the precedence for that is.
When a host associated `threadprivate` variable was used in a parallel
region with `default(none)` in an internal subroutine was failing,
because the compiler did not properly determine that the variable was
pre-determined `threadprivate` and thus should not have been reported as
missing a DSA.
Consider:
```
function foo()
!$omp declare target(foo) ! This `foo` was a function-result symbol
...
end
```
When resolving symbols, for this case use the symbol corresponding to
the function instead of the symbol corresponding to the function result.
Currently, this will result in an error:
```
error: A variable that appears in a DECLARE TARGET directive must be
declared in the scope of a module or have the SAVE attribute, either
explicitly or implicitly
```
Issue: Cray Pointer is not associated to Cray Pointee, leading to
Segmentation fault
Fix: GetUltimate, retrieves the base symbol in the current scope, which
gets passed all the references and returns the original symbol
---------
Co-authored-by: Michael Klemm <michael.klemm@amd.com>
The `OmpDirectiveSpecification` contains directive name, the list of
arguments, and the list of clauses. It was introduced to store the
directive specification in METADIRECTIVE, and could be reused everywhere
a directive representation is needed.
In the long term this would unify the handling of common directive
properties, as well as creating actual constructs from METADIRECTIVE by
linking the contained directive specification with any associated user
code.
The standard prohibits privatising namelist variables. We also decided
in #110671 to prohibit reductions of namelist variables.
This commit prevents this rule from being circumvented through the use
of equivalence statements.
Fixes#122824
Part of the DECLARE REDUCTION was already supported by the parser, but
the semantics to add the reduction identifier wasn't implemented.
The semantics would not accept the name given by the reduction, so a few
lines added to support that.
Some tests were in place but not quite working, so fixed those up too.
Adding new tests for unparsing and parse-tree, as well as checking the
symbolic name being generated.
Lowering of DECLARE REDUCTION is not supported in this patch, and a test
that it hits the relevant TODO is in this patch (most of this was
already existing, but not actually testing the TODO message).
Issue: Compilation abnormally terminates in parallel default(private)
Documentation reference:
A threadprivate variable must not appear as the base variable of a list
item in any clause except for the copyin and copyprivate clauses
Explanation:
From the reference, the threadprivate symbols cannot be used in the DSA
clauses, which in turn means, the symbol can be skipped for default
privatization
Fixes#123535
Parse METADIRECTIVE as a standalone executable directive at the moment.
This will allow testing the parser code.
There is no lowering, not even clause conversion yet. There is also no
verification of the allowed values for trait sets, trait properties.
Follow-up PR to fix the failure caused here:
https://github.com/llvm/llvm-project/pull/121028
Failure:
https://lab.llvm.org/buildbot/#/builders/89/builds/14474
Problems:
- Cray pointee cannot be used in the DSA list (If used results in segmentation fault)
- Cray pointer has to be in the DSA list when Cray pointee is used in the default (none) region
Fix: Added required semantic checks along the tests
Reference from the documentation (OpenMP 5.0: 2.19.1):
- Cray pointees have the same data-sharing attribute as the storage with
which their Cray pointers are associated.
This allows the Flang parser to accept the !$OMP DISPATCH and related
clauses.
Lowering is currently not implemented. Tests for unparse and parse-tree
dump is provided, and one for checking that the lowering ends in a "not
yet implemented"
---------
Co-authored-by: Kiran Chandramohan <kiran.chandramohan@arm.com>
Problems:
- Cray pointee cannot be used in the DSA list (If used results in
segmentation fault)
- Cray pointer has to be in the DSA list when Cray pointee is used in
the default (none) region
Fix: Added required semantic checks along the tests
Reference from the documentation (OpenMP 5.0: 2.19.1):
- Cray pointees have the same data-sharing attribute as the storage with
which their Cray pointers are associated.
The OmpLinearClause class was a variant of two classes, one for when the
linear modifier was present, and one for when it was absent. These two
classes did not follow the conventions for parse tree nodes, (i.e.
tuple/wrapper/union formats), which necessitated specialization of the
parse tree visitor.
The new form of OmpLinearClause is the standard tuple with a list of
modifiers and an object list. The specialization of parse tree visitor
for it has been removed.
Parsing and unparsing of the new form bears additional complexity due to
syntactical differences between OpenMP 5.2 and prior versions: in OpenMP
5.2 the argument list is post-modified, while in the prior versions, the
step modifier was a post-modifier while the linear modifier had an
unusual syntax of `modifier(list)`.
With this change the LINEAR clause is no different from any other
clauses in terms of its structure and use of modifiers. Modifier
validation and all other checks work the same as with other clauses.
The intent is to keep names in sync with the terminology from the OpenMP
spec:
```
OmpBindClause::Type -> Binding
OmpDefaultClause::Type -> DataSharingAttribute
OmpDeviceTypeClause::Type -> DeviceTypeDescription
OmpProcBindClause::Type -> AffinityPolicy
```
Add more comments with references to the OpenMP specs.
