[Flang] Add support for assume_aligned directive #81747
Conversation
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-parser Author: Mats Petersson (Leporacanthicus) ChangesThis adds the parsing (and unparse) of the compiler drective assume_aligned. The compiler will issue a warning that the directive is ignored. Full diff: https://github.com/llvm/llvm-project/pull/81747.diff 5 Files Affected:
diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index d067a7273540f0..048008a8d80c79 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -205,6 +205,7 @@ class ParseTreeDumper {
NODE(parser, CompilerDirective)
NODE(CompilerDirective, IgnoreTKR)
NODE(CompilerDirective, LoopCount)
+ NODE(CompilerDirective, AssumeAligned)
NODE(CompilerDirective, NameValue)
NODE(parser, ComplexLiteralConstant)
NODE(parser, ComplexPart)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index e9bfb728a2bef6..f8f4074f9a3b1b 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3309,12 +3309,19 @@ struct CompilerDirective {
struct LoopCount {
WRAPPER_CLASS_BOILERPLATE(LoopCount, std::list<std::uint64_t>);
};
+ struct AssumeAligned
+ {
+ TUPLE_CLASS_BOILERPLATE(AssumeAligned);
+ std::tuple<common::Indirection<Designator>, uint64_t> t;
+ };
struct NameValue {
TUPLE_CLASS_BOILERPLATE(NameValue);
std::tuple<Name, std::optional<std::uint64_t>> t;
};
CharBlock source;
- std::variant<std::list<IgnoreTKR>, LoopCount, std::list<NameValue>> u;
+ std::variant<std::list<IgnoreTKR>, LoopCount, AssumeAligned,
+ std::list<NameValue>>
+ u;
};
// (CUDA) ATTRIBUTE(attribute) [::] name-list
diff --git a/flang/lib/Parser/Fortran-parsers.cpp b/flang/lib/Parser/Fortran-parsers.cpp
index 0dd95d69d3c662..1bea0dc69f1c43 100644
--- a/flang/lib/Parser/Fortran-parsers.cpp
+++ b/flang/lib/Parser/Fortran-parsers.cpp
@@ -1268,9 +1268,13 @@ constexpr auto ignore_tkr{
constexpr auto loopCount{
"DIR$ LOOP COUNT" >> construct<CompilerDirective::LoopCount>(
parenthesized(nonemptyList(digitString64)))};
+constexpr auto assumeAligned{"DIR$ ASSUME_ALIGNED" >>
+ construct<CompilerDirective::AssumeAligned>(
+ indirect(designator), ":"_tok >> digitString64)};
TYPE_PARSER(beginDirective >>
sourced(construct<CompilerDirective>(ignore_tkr) ||
construct<CompilerDirective>(loopCount) ||
+ construct<CompilerDirective>(assumeAligned) ||
construct<CompilerDirective>(
"DIR$" >> many(construct<CompilerDirective::NameValue>(name,
maybe(("="_tok || ":"_tok) >> digitString64))))) /
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index 1df49a688a12a0..41c1a754d68682 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -1819,6 +1819,10 @@ class UnparseVisitor {
[&](const CompilerDirective::LoopCount &lcount) {
Walk("!DIR$ LOOP COUNT (", lcount.v, ", ", ")");
},
+ [&](const CompilerDirective::AssumeAligned &assumeAligned) {
+ Word("!DIR$ ASSUME_ALIGNED ");
+ Walk(assumeAligned);
+ },
[&](const std::list<CompilerDirective::NameValue> &names) {
Walk("!DIR$ ", names, " ");
},
@@ -1841,6 +1845,11 @@ class UnparseVisitor {
Walk(std::get<Name>(x.t));
Walk("=", std::get<std::optional<std::uint64_t>>(x.t));
}
+ void Unparse(const CompilerDirective::AssumeAligned &x) {
+ Walk(std::get<common::Indirection<Designator>>(x.t));
+ Put(":");
+ Walk(std::get<uint64_t>(x.t));
+ }
// OpenACC Directives & Clauses
void Unparse(const AccAtomicCapture &x) {
diff --git a/flang/test/Parser/assume-aligned.f90 b/flang/test/Parser/assume-aligned.f90
new file mode 100644
index 00000000000000..426916f2544dbe
--- /dev/null
+++ b/flang/test/Parser/assume-aligned.f90
@@ -0,0 +1,50 @@
+! RUN: %flang_fc1 -fdebug-unparse-no-sema %s 2>&1 | FileCheck %s
+
+SUBROUTINE aa(a, nn)
+ IMPLICIT NONE
+ INTEGER, INTENT(IN) :: nn
+ COMPLEX(8), INTENT(INOUT), DIMENSION(1:nn) :: a
+ INTEGER :: i
+ !DIR$ assume_aligned a:16
+!CHECK: !DIR$ ASSUME_ALIGNED a:16
+ !DIR$ assume_aligned a (1):16
+!CHECK: !DIR$ ASSUME_ALIGNED a(1):16
+ !DIR$ assume_aligned a(1):16
+!CHECK: !DIR$ ASSUME_ALIGNED a(1):16
+ !DIR$ assume_aligned a(nn):16
+!CHECK: !DIR$ ASSUME_ALIGNED a(nn):16
+ !DIR$ assume_aligned a(44):16
+!CHECK: !DIR$ ASSUME_ALIGNED a(44):16
+ DO i=1,nn
+ a(i)=a(i)+1.5
+ END DO
+END SUBROUTINE aa
+
+SUBROUTINE bb(v, s, e)
+ IMPLICIT NONE
+ INTEGER, INTENT(IN) :: s(3), e(3)
+ INTEGER :: y,z
+ REAL(8), INTENT(IN) :: v(s(1):e(1),s(2):e(2),s(3):e(3))
+ !DIR$ assume_aligned v(s(1),y,z) :64
+!CHECK: !DIR$ ASSUME_ALIGNED v(s(1),y,z):64
+END SUBROUTINE bb
+
+SUBROUTINE f(n)
+ IMPLICIT NONE
+ TYPE node
+ REAL(KIND=8), POINTER :: a(:,:)
+ END TYPE NODE
+
+ TYPE(NODE), POINTER :: nodes
+ INTEGER :: i
+ INTEGER, INTENT(IN) :: n
+
+ ALLOCATE(nodes)
+ ALLOCATE(nodes%a(1000,1000))
+
+ !DIR$ ASSUME_ALIGNED nodes%a(1,1) : 16
+!CHECK: !DIR$ ASSUME_ALIGNED nodes%a(1,1):16
+ DO i=1,n
+ nodes%a(1,i) = nodes%a(1,i)+1
+ END DO
+END SUBROUTINE f
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
This adds the parsing (and unparse) of the compiler drective assume_aligned. The compiler will issue a warning that the directive is ignored.
Leporacanthicus
force-pushed
the
assume-aligned-fix
branch
from
4439a97
to
b3448b0
Compare
Add documentation added as per review comment. Also, the directive should accept multiple pairs of variable (or designator) and alignment values on the same line, so fix that and add test.
|
LG so far. Where is the warning generated? Could you write a test for that? nit: please write the commit title in the normal format |
|
why is a general designator allowed instead of a simple base object name? why would subscripts be needed to set alignment of the base object? |
Because that's what's allowed by the original implementation, and in some source code that we're trying to compile. The test-case with the three array index variant I added is a simplified version of that original source. Would have been MUCH easier if it was just the name of the base object. That already works [it produces a warning that it's ignored - fine by me]. Here's the Intel blurb about this directive: By the way this directive doesn't set the alignment, it is telling the compiler what the actual alignment is, however that may have been achieved. At present, my main concern is to get the compiler to accept this at all - in the form it is in the original source, which I don't think is available on the internet, I'm sorry about that. I was originally just accepting a name and Because there are parenthesis involved, the current version of the compiler says "expected end of statement" when it hits the parenthesis. Here's a some nonsense-code to show the difference between "already works" and "causes compilation error". It will fail on line 4, pointing at the parenthesis. |
kiranchandramohan
changed the title
|
@klausler Hi Peter, do you have any further feedback. Kiran's comment contains links to existing uses in among other things SALMON, that use the format of |
I do not. |
It is warned for in the generic handling of directives.There is no special handling for this case, it is just a "if the compiler gets to the end of the list without finding a match, give a warning that it wasn't recognized". I don't believe we need a specific test - it's not an actively desired feature.
|
This adds the parsing (and unparse) of the compiler drective assume_aligned.
The compiler will issue a warning that the directive is ignored.