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.
Again, this simplifies the semantic checks and lowering quite a bit.
Update the check for positive alignment to use a more informative
message, and to highlight the modifier itsef, not the whole clause.
Remove the checks for the allocator expression itself being positive:
there is nothing in the spec that says that it should be positive.
Remove the "simple" modifier from the AllocateT template, since both
simple and complex modifiers are the same thing, only differing in
syntax.
This removes the specialized parsers and helper classes for these
clauses, namely ConcatSeparated, MapModifiers, and MotionModifiers. Map
and the motion clauses are now handled in the same way as all other
clauses with modifiers, with one exception: the commas separating their
modifiers are optional. This syntax is deprecated in OpenMP 5.2.
Implement version checks for modifiers: for a given modifier on a given
clause, check if that modifier is allowed on this clause in the
specified OpenMP version. This replaced several individual checks.
Add a testcase for handling map modifiers in a different order, and for
diagnosing an ultimate modifier out of position.
Two PRs were merged at the same time: one that modified `maybeApplyToV`
function, and shortly afterwards, this (the reverted) one that had the
old definition.
During the merge both definitions were retained leading to compilation
errors.
Reapply the reverted PR (1a08b15589) with the duplicate removed.
Also, define helper macros in parse-tree.h.
Apply the new modifier representation to the DEFAULTMAP and REDUCTION
clauses, with testcases utilizing the new modifier validation.
OpenMP modifier overhaul: #3/3
This is the first part of the effort to make parsing of clause modifiers
more uniform and robust. Currently, when multiple modifiers are allowed,
the parser will expect them to appear in a hard-coded order.
Additionally, modifier properties (such as "ultimate") are checked
separately for each case.
The overall plan is
1. Extract all modifiers into their own top-level classes, and then
equip them with sets of common properties that will allow performing the
property checks generically, without refering to the specific kind of
the modifier.
2. Define a parser (as a separate class) for each modifier.
3. For each clause define a union (std::variant) of all allowable
modifiers, and parse the modifiers as a list of these unions.
The intent is also to isolate parts of the code that could eventually be
auto-generated.
OpenMP modifier overhaul: #1/3
Keep track of loop constructs and OpenMP loop constructs that have been
entered. Use the information to validate the variables in the SINK loop
iteration vector.
---------
Co-authored-by: Tom Eccles <tom.eccles@arm.com>
Extract the SINK/SOURCE parse tree elements into a separate class
`OmpDoacross`, share them between DEPEND and DOACROSS clauses. Most of
the changes in Semantics are to accommodate the new contents of
OmpDependClause, and a mere introduction of OmpDoacrossClause.
There are no semantic checks specifically for DOACROSS.
Parse PRESENT modifier as well while we're at it (no MAPPER though). Add
semantic checks for these clauses in the TARGET UPDATE construct, TODO
messages in lowering.
Issue deprecation warning for these directives.
Lowering currently supports parallel master, for all other combined or
composite directives involving master, issue TODO errors.
Note: The first commit changes the formatting and generalizes the
deprecation message emission for reuse in the second commit. I can pull
it out into a separate commit if required.
Parse the locator list in OmpDependClause as an OmpObjectList (instead
of a list of Designators). When a common block appears in the locator
list, show an informative message.
Implement resolving symbols in DependSinkVec in a dedicated visitor
instead of having a visitor for OmpDependClause.
Resolve unresolved names common blocks in OmpObjectList.
Minor changes to the code organization:
- rename OmpDependenceType to OmpTaskDependenceType (to follow 5.2
terminology),
- rename Depend::WithLocators to Depend::DepType,
- add comments with more detailed spec references to parse-tree.h.
---------
Co-authored-by: Kiran Chandramohan <kiran.chandramohan@arm.com>
Define `OmpIteratorSpecifier` and `OmpIteratorModifier` parser classes,
and add parsing for them. Those are reusable between any clauses that
use iterator modifiers.
Add support for iterator modifiers to the MAP clause up to lowering,
where a TODO message is emitted.
For test program like this variable array is mentioned in both shared
clause and map clause. For OMP TARGET compound directives like this
where we have OMP TARGET TEAMS, map clause applies to TARGET directive
and SHARED clause applies to TEAMS directive. So both SHARED and MAP
clauses can co-exist.
> program test
> implicit none
> integer :: array(10,10),i,j
> !$omp target teams shared(array) map(tofrom:array)
> do i=1,10
> !$omp parallel do
> do j=1,10
> array(j,i)=i+j
> end do
> end do
> !$omp end target teams
> print *, array
> end program test
>
>
Before this PR we were checking for exclusivity for all target
directives set which is now relaxed to exclusivity check not being
applied to compound directives which can accept SHARED clause.
This commit adds parsing of type modifiers for the MAP clause: CLOSE,
OMPX_HOLD, and PRESENT. The support for ALWAYS has already existed.
The new modifiers are not yet handled in lowering: when present, a TODO
message is emitted and compilation stops.
OpenMP prohibits privatisation of variables that appear in expressions
for statement functions.
This is a re-working of an old patch https://reviews.llvm.org/D93213 by
@praveen-g-ctt.
The old patch couldn't be landed because of ordering concerns. Statement
functions are rewritten during parse tree rewriting, but this was done
after resolve-directives and so some array expressions were incorrectly
identified as statement functions. For this reason **I have opted to
re-order the semantics driver so that resolve-directives is run after
parse tree rewriting**.
Closes#54677
---------
Co-authored-by: Praveen <praveen@compilertree.com>
(This is a big patch, but it's nearly an NFC. No test results have
changed and all Fortran tests in the LLVM test suites work as expected.)
Allow a parser::Message for a warning to be marked with the
common::LanguageFeature or common::UsageWarning that controls it. This
will allow a later patch to add hooks whereby a driver will be able to
decorate warning messages with the names of its options that enable each
particular warning, and to add hooks whereby a driver can map those
enumerators by name to command-line options that enable/disable the
language feature and enable/disable the messages.
The default settings in the constructor for LanguageFeatureControl were
moved from its header file into its C++ source file.
Hooks for a driver to use to map the name of a feature or warning to its
enumerator were also added.
To simplify the tagging of warnings with their corresponding language
feature or usage warning, to ensure that they are properly controlled by
ShouldWarn(), and to ensure that warnings never issue at code sites in
module files, two new Warn() member function templates were added to
SemanticsContext and other contextual frameworks. Warn() can't be used
before source locations can be mapped to scopes, but the bulk of
existing code blocks testing ShouldWarn() and FindModuleFile() before
calling Say() were convertible into calls to Warn(). The ones that were
not convertible were extended with explicit calls to
Message::set_languageFeature() and set_usageWarning().
This is allowed by the OpenMP and F23 standards. But variables in a
namelist are not allowed in OpenMP privatisation. I suspect this was an
oversight.
If we allow this we run into problems masking the original symbol with
the symbol for the reduction variable when the variable is accessed via
a namelist initialised as a global variable. See #101907. One solution
for this would be to force the namelist to always be initilized inside
of the block in which it is used (therefore using the correct mapping
for the reduction variable), but this could make some production
applications slow.
I tentatively think it is probably better to disallow a (perhaps
mistaken) edge case of the standards with (I think) little practical
use, than to make real applications slow in order to make this work. If
reviewers would rather keep to the letter of the standard, see #109303
which implements the alternative solution. I'm open to either path
forward.
Fixes#101907
The associate name preserves the association with the selector
established in the associate statement. Therefore it is incorrect to
change the data-sharing attribute of the name.
Closes#58041
Mark the symbol with OmpShared, and then check that later in lowering to
avoid making a local loop index.
OpenMP 5.2 says: "Loop iteration variables of loops that are not associated
with any OpenMP directive maybe listed in data-sharing attribute clauses on
the surrounding teams, parallel or taskgenerating construct, and on enclosed
constructs, subject to other restrictions."
Tests updated to match the extra OmpShared attribute.
Add regression test for lowering to hlfir.
Closes#102961
---------
Co-authored-by: Tom Eccles <tom.eccles@arm.com>
Previously we tracked data sharing attributes by the symbol itself not
by the ultimate symbol. When the private clause came first, subsequent
uses of the symbol found a host-associated version instead of the
ultimate symbol and so the check didn't consider them to be the same
symbol. Always adding and checking for the ultimate symbol ensures that
we have the same behaviour no matter the order of clauses.
The modified list is only used for this multiple clause check.
Closes#78235
