-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[Flang] Add parser support for prefetch directive #139702
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[Flang] Add parser support for prefetch directive #139702
Conversation
* Recognize prefetch directive in the parser as `!dir$ prefetch ...` * Unparse the prefetch directive * Add required tests Details on the prefetch directive: `!dir$ prefetch designator[, designator]...`, where the designator list can be a variable or an array reference. This directive is used to insert a hint to the code generator to prefetch instructions for memory references.
@llvm/pr-subscribers-flang-parser Author: Thirumalai Shaktivel (Thirumalai-Shaktivel) ChangesImplementation details:
Details on the prefetch directive: Full diff: https://github.com/llvm/llvm-project/pull/139702.diff 6 Files Affected:
diff --git a/flang/docs/Directives.md b/flang/docs/Directives.md
index 91c27cb510ea0..9216516494523 100644
--- a/flang/docs/Directives.md
+++ b/flang/docs/Directives.md
@@ -50,6 +50,9 @@ A list of non-standard directives supported by Flang
integer that specifying the unrolling factor. When `N` is `0` or `1`, the loop
should not be unrolled at all. If `N` is omitted the optimizer will
selects the number of times to unroll the loop.
+* `!dir$ prefetch designator[, designator]...`, where the designator list can be
+ a variable or an array reference. This directive is used to insert a hint to
+ the code generator to prefetch instructions for memory references.
* `!dir$ novector` disabling vectorization on the following loop.
* `!dir$ nounroll` disabling unrolling on the following loop.
* `!dir$ nounroll_and_jam` disabling unrolling and jamming on the following loop.
diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index df9278697346f..c62d9b695108d 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -214,6 +214,7 @@ class ParseTreeDumper {
NODE(CompilerDirective, NoVector)
NODE(CompilerDirective, NoUnroll)
NODE(CompilerDirective, NoUnrollAndJam)
+ NODE(CompilerDirective, Prefetch)
NODE(parser, ComplexLiteralConstant)
NODE(parser, ComplexPart)
NODE(parser, ComponentArraySpec)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 254236b510544..cba7653be83d3 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3354,6 +3354,7 @@ struct StmtFunctionStmt {
// !DIR$ NOVECTOR
// !DIR$ NOUNROLL
// !DIR$ NOUNROLL_AND_JAM
+// !DIR$ PREFETCH designator[, designator]...
// !DIR$ <anything else>
struct CompilerDirective {
UNION_CLASS_BOILERPLATE(CompilerDirective);
@@ -3379,14 +3380,18 @@ struct CompilerDirective {
struct UnrollAndJam {
WRAPPER_CLASS_BOILERPLATE(UnrollAndJam, std::optional<std::uint64_t>);
};
+ struct Prefetch {
+ WRAPPER_CLASS_BOILERPLATE(
+ Prefetch, std::list<common::Indirection<Designator>>);
+ };
EMPTY_CLASS(NoVector);
EMPTY_CLASS(NoUnroll);
EMPTY_CLASS(NoUnrollAndJam);
EMPTY_CLASS(Unrecognized);
CharBlock source;
std::variant<std::list<IgnoreTKR>, LoopCount, std::list<AssumeAligned>,
- VectorAlways, std::list<NameValue>, Unroll, UnrollAndJam, Unrecognized,
- NoVector, NoUnroll, NoUnrollAndJam>
+ VectorAlways, std::list<NameValue>, Unroll, UnrollAndJam, Prefetch,
+ Unrecognized, NoVector, NoUnroll, NoUnrollAndJam>
u;
};
diff --git a/flang/lib/Parser/Fortran-parsers.cpp b/flang/lib/Parser/Fortran-parsers.cpp
index fbe629ab52935..782dff8a967b6 100644
--- a/flang/lib/Parser/Fortran-parsers.cpp
+++ b/flang/lib/Parser/Fortran-parsers.cpp
@@ -1294,6 +1294,7 @@ TYPE_PARSER(construct<StatOrErrmsg>("STAT =" >> statVariable) ||
// !DIR$ LOOP COUNT (n1[, n2]...)
// !DIR$ name[=value] [, name[=value]]...
// !DIR$ UNROLL [n]
+// !DIR$ PREFETCH designator[, designator]...
// !DIR$ <anything else>
constexpr auto ignore_tkr{
"IGNORE_TKR" >> optionalList(construct<CompilerDirective::IgnoreTKR>(
@@ -1308,6 +1309,8 @@ constexpr auto vectorAlways{
"VECTOR ALWAYS" >> construct<CompilerDirective::VectorAlways>()};
constexpr auto unroll{
"UNROLL" >> construct<CompilerDirective::Unroll>(maybe(digitString64))};
+constexpr auto prefetch{"PREFETCH" >>
+ construct<CompilerDirective::Prefetch>(nonemptyList(indirect(designator)))};
constexpr auto unrollAndJam{"UNROLL_AND_JAM" >>
construct<CompilerDirective::UnrollAndJam>(maybe(digitString64))};
constexpr auto novector{"NOVECTOR" >> construct<CompilerDirective::NoVector>()};
@@ -1321,6 +1324,7 @@ TYPE_PARSER(beginDirective >> "DIR$ "_tok >>
construct<CompilerDirective>(vectorAlways) ||
construct<CompilerDirective>(unrollAndJam) ||
construct<CompilerDirective>(unroll) ||
+ construct<CompilerDirective>(prefetch) ||
construct<CompilerDirective>(novector) ||
construct<CompilerDirective>(nounrollAndJam) ||
construct<CompilerDirective>(nounroll) ||
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index a626888b7dfe5..e4dbb16a6346c 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -1854,6 +1854,10 @@ class UnparseVisitor {
Word("!DIR$ UNROLL");
Walk(" ", unroll.v);
},
+ [&](const CompilerDirective::Prefetch &prefetch) {
+ Word("!DIR$ PREFETCH");
+ Walk(" ", prefetch.v);
+ },
[&](const CompilerDirective::UnrollAndJam &unrollAndJam) {
Word("!DIR$ UNROLL_AND_JAM");
Walk(" ", unrollAndJam.v);
diff --git a/flang/test/Parser/prefetch.f90 b/flang/test/Parser/prefetch.f90
new file mode 100644
index 0000000000000..1013a09c92117
--- /dev/null
+++ b/flang/test/Parser/prefetch.f90
@@ -0,0 +1,80 @@
+!RUN: %flang_fc1 -fdebug-unparse-no-sema %s 2>&1 | FileCheck %s -check-prefix=UNPARSE
+!RUN: %flang_fc1 -fdebug-dump-parse-tree-no-sema %s 2>&1 | FileCheck %s -check-prefix=TREE
+
+subroutine test_prefetch_01(a, b)
+ integer, intent(in) :: a
+ integer, intent(inout) :: b(5)
+ integer :: i = 2
+ integer :: res
+
+!TREE: | | DeclarationConstruct -> SpecificationConstruct -> CompilerDirective -> Prefetch -> Designator -> DataRef -> Name = 'a'
+
+!UNPARSE: !DIR$ PREFETCH a
+ !dir$ prefetch a
+ b(1) = a
+
+!TREE: | | ExecutionPartConstruct -> ExecutableConstruct -> CompilerDirective -> Prefetch -> Designator -> DataRef -> Name = 'b'
+
+!UNPARSE: !DIR$ PREFETCH b
+ !dir$ prefetch b
+ res = sum(b)
+
+!TREE: | | ExecutionPartConstruct -> ExecutableConstruct -> CompilerDirective -> Prefetch -> Designator -> DataRef -> Name = 'a'
+!TREE: | | Designator -> DataRef -> ArrayElement
+!TREE: | | | DataRef -> Name = 'b'
+!TREE: | | | SectionSubscript -> SubscriptTriplet
+!TREE: | | | | Scalar -> Integer -> Expr -> LiteralConstant -> IntLiteralConstant = '3'
+!TREE: | | | | Scalar -> Integer -> Expr -> LiteralConstant -> IntLiteralConstant = '5'
+
+!UNPARSE: !DIR$ PREFETCH a, b(3:5)
+ !dir$ prefetch a, b(3:5)
+ res = a + b(4)
+
+!TREE: | | ExecutionPartConstruct -> ExecutableConstruct -> CompilerDirective -> Prefetch -> Designator -> DataRef -> Name = 'res'
+!TREE: | | Designator -> DataRef -> ArrayElement
+!TREE: | | | DataRef -> Name = 'b'
+!TREE: | | | SectionSubscript -> Integer -> Expr -> Add
+!TREE: | | | | Expr -> Designator -> DataRef -> Name = 'i'
+!TREE: | | | | Expr -> LiteralConstant -> IntLiteralConstant = '2'
+
+!UNPARSE: !DIR$ PREFETCH res, b(i+2)
+ !dir$ prefetch res, b(i+2)
+ res = res + b(i+2)
+end subroutine
+
+subroutine test_prefetch_02(n, a)
+ integer, intent(in) :: n
+ integer, intent(in) :: a(n)
+ type :: t
+ real, allocatable :: x(:, :)
+ end type t
+ type(t) :: p
+
+ do i = 1, n
+!TREE: | | | | ExecutionPartConstruct -> ExecutableConstruct -> CompilerDirective -> Prefetch -> Designator -> DataRef -> ArrayElement
+!TREE: | | | | | DataRef -> StructureComponent
+!TREE: | | | | | | DataRef -> Name = 'p'
+!TREE: | | | | | | Name = 'x'
+!TREE: | | | | | SectionSubscript -> Integer -> Expr -> Designator -> DataRef -> Name = 'i'
+!TREE: | | | | | SectionSubscript -> SubscriptTriplet
+!TREE: | | | | Designator -> DataRef -> Name = 'a'
+
+!UNPARSE: !DIR$ PREFETCH p%x(i,:), a
+ !dir$ prefetch p%x(i, :), a
+ do j = 1, n
+!TREE: | | | | | | ExecutionPartConstruct -> ExecutableConstruct -> CompilerDirective -> Prefetch -> Designator -> DataRef -> ArrayElement
+!TREE: | | | | | | | DataRef -> StructureComponent
+!TREE: | | | | | | | | DataRef -> Name = 'p'
+!TREE: | | | | | | | | Name = 'x'
+!TREE: | | | | | | | SectionSubscript -> Integer -> Expr -> Designator -> DataRef -> Name = 'i'
+!TREE: | | | | | | | SectionSubscript -> Integer -> Expr -> Designator -> DataRef -> Name = 'j'
+!TREE: | | | | | | Designator -> DataRef -> ArrayElement
+!TREE: | | | | | | | DataRef -> Name = 'a'
+!TREE: | | | | | | | SectionSubscript -> Integer -> Expr -> Designator -> DataRef -> Name = 'i'
+
+!UNPARSE: !DIR$ PREFETCH p%x(i,j), a(i)
+ !dir$ prefetch p%x(i, j), a(i)
+ p%x(i, j) = p%x(i, j) ** a(j)
+ end do
+ end do
+end subroutine
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Please could you add a TODO(loc, "!$dir prefetch")
in lowering so that this does not get silently ignored until the codegen lands.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Please add the TODO as Tom mentioned.
Thanks for the reviews! I will add the required changes soon. |
Just a pass-through comment. |
Thanks for the poke, Kiran. Cray Compiler Environment documentation link: https://cpe.ext.hpe.com/docs/latest/cce/index.html Our syntax is slightly different
With this provided example showing it in practice:
|
Thanks @tmjbios for the quick reply. The question is whether the syntax proposed in this PR |
The IBM Open XL Fortran compiler has slightly different syntax for the https://www.ibm.com/docs/en/openxl-fortran-aix/17.1.3?topic=prefetch-by-load |
Both Cray CCE Intel's A current master branch So I would object to this new implementation's syntax being incompatible with the existing Fortran compilers. Is there a compelling reason for Flang to be different from the major vendors? This is especially concerning when there are existing codes in the wild which will break or see significantly degraded performance if they use |
One more thought. If we want to specialize the prefetch operation (e.g. for store or load) without introducing a new directive, the current syntax may be very limited (i.e. no way to distinguish a keyword and a variable name). |
The syntax proposed here is similar to the ones that are/were supported in pgfortran and classic-flang based compilers (AOCC, Huawei compilers, Arm compilers). They all had the syntax https://docs.nvidia.com/hpc-sdk/pgi-compilers/19.1/x86/pgi-ref-guide/index.htm#prefetch |
Is that because the prefetch directive is only applicable in limited contexts like loops? From the syntax in the links that you posted, it looks like the syntax accepted in this patch is a subset. |
Yes, sorta - this seems more of a superset of what we support in that we require more specificity from the user. CCE will tend to disallow prefetching a whole array in this manner. Instead we allow the user to specify a scalar or an array element along with a number of cache lines, whether it is for read or write, whether the data is temporal or non-temporal, and which level of cache to work with. I'm not suggesting anyone block or disapprove this PR - this is certainly a step in the right direction. I'm just reminded of xkcd 927. |
Implementation details:
!dir$ prefetch ...
Details on the prefetch directive:
!dir$ prefetch designator[, designator]...
, where the designator listcan be a variable or an array reference. This directive is used to
insert a hint to the code generator to prefetch instructions for
memory references.