[flang] Implement !DIR$ VECTOR ALWAYS #93830
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-fir-hlfir @llvm/pr-subscribers-flang-semantics Author: David Truby (DavidTruby) ChangesThis patch implements support for the VECTOR ALWAYS directive, which forces Full diff: https://github.com/llvm/llvm-project/pull/93830.diff 12 Files Affected:
diff --git a/flang/include/flang/Lower/PFTBuilder.h b/flang/include/flang/Lower/PFTBuilder.h
index 9913f584133fa..aa83f1603c2a8 100644
--- a/flang/include/flang/Lower/PFTBuilder.h
+++ b/flang/include/flang/Lower/PFTBuilder.h
@@ -347,6 +347,7 @@ struct Evaluation : EvaluationVariant {
parser::CharBlock position{};
std::optional<parser::Label> label{};
std::unique_ptr<EvaluationList> evaluationList; // nested evaluations
+ llvm::SmallVector<const parser::CompilerDirective *> dirs;
Evaluation *parentConstruct{nullptr}; // set for nodes below the top level
Evaluation *lexicalSuccessor{nullptr}; // set for leaf nodes, some directives
Evaluation *controlSuccessor{nullptr}; // set for some leaf nodes
diff --git a/flang/include/flang/Optimizer/Dialect/FIROps.h b/flang/include/flang/Optimizer/Dialect/FIROps.h
index 9f07364ddb627..a21f8bbe17685 100644
--- a/flang/include/flang/Optimizer/Dialect/FIROps.h
+++ b/flang/include/flang/Optimizer/Dialect/FIROps.h
@@ -16,6 +16,7 @@
#include "flang/Optimizer/Dialect/FortranVariableInterface.h"
#include "mlir/Dialect/Arith/IR/Arith.h"
#include "mlir/Dialect/Func/IR/FuncOps.h"
+#include "mlir/Dialect/LLVMIR/LLVMAttrs.h"
#include "mlir/Interfaces/LoopLikeInterface.h"
#include "mlir/Interfaces/SideEffectInterfaces.h"
diff --git a/flang/include/flang/Optimizer/Dialect/FIROps.td b/flang/include/flang/Optimizer/Dialect/FIROps.td
index d9c1149040066..7bf68908e37dd 100644
--- a/flang/include/flang/Optimizer/Dialect/FIROps.td
+++ b/flang/include/flang/Optimizer/Dialect/FIROps.td
@@ -2096,7 +2096,8 @@ def fir_DoLoopOp : region_Op<"do_loop",
Index:$step,
Variadic<AnyType>:$initArgs,
OptionalAttr<UnitAttr>:$unordered,
- OptionalAttr<UnitAttr>:$finalValue
+ OptionalAttr<UnitAttr>:$finalValue,
+ OptionalAttr<LoopAnnotationAttr>:$loop_annotation
);
let results = (outs Variadic<AnyType>:$results);
let regions = (region SizedRegion<1>:$region);
diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index 68ae50c312cde..8cf790650fb49 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -206,6 +206,7 @@ class ParseTreeDumper {
NODE(CompilerDirective, IgnoreTKR)
NODE(CompilerDirective, LoopCount)
NODE(CompilerDirective, AssumeAligned)
+ NODE(CompilerDirective, VectorAlways)
NODE(CompilerDirective, NameValue)
NODE(CompilerDirective, Unrecognized)
NODE(parser, ComplexLiteralConstant)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 0a40aa8b8f616..116e5f02ff32b 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3316,6 +3316,7 @@ struct CompilerDirective {
TUPLE_CLASS_BOILERPLATE(AssumeAligned);
std::tuple<common::Indirection<Designator>, uint64_t> t;
};
+ EMPTY_CLASS(VectorAlways);
struct NameValue {
TUPLE_CLASS_BOILERPLATE(NameValue);
std::tuple<Name, std::optional<std::uint64_t>> t;
@@ -3323,7 +3324,7 @@ struct CompilerDirective {
EMPTY_CLASS(Unrecognized);
CharBlock source;
std::variant<std::list<IgnoreTKR>, LoopCount, std::list<AssumeAligned>,
- std::list<NameValue>, Unrecognized>
+ VectorAlways, std::list<NameValue>, Unrecognized>
u;
};
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index 4e50de3e7ee9c..8ce68f9b8caab 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -1881,7 +1881,7 @@ class FirConverter : public Fortran::lower::AbstractConverter {
// Increment loop begin code. (Infinite/while code was already generated.)
if (!infiniteLoop && !whileCondition)
- genFIRIncrementLoopBegin(incrementLoopNestInfo);
+ genFIRIncrementLoopBegin(incrementLoopNestInfo, doStmtEval.dirs);
// Loop body code.
auto iter = eval.getNestedEvaluations().begin();
@@ -1926,8 +1926,22 @@ class FirConverter : public Fortran::lower::AbstractConverter {
return builder->createIntegerConstant(loc, controlType, 1); // step
}
+ void addLoopAnnotationAttr(IncrementLoopInfo &info) {
+ mlir::BoolAttr f = mlir::BoolAttr::get(builder->getContext(), false);
+ mlir::LLVM::LoopVectorizeAttr va = mlir::LLVM::LoopVectorizeAttr::get(
+ builder->getContext(), f, {}, {}, {}, {}, {}, {});
+ mlir::LLVM::AccessGroupAttr ag =
+ mlir::LLVM::AccessGroupAttr::get(builder->getContext());
+ mlir::LLVM::LoopAnnotationAttr la = mlir::LLVM::LoopAnnotationAttr::get(
+ builder->getContext(), {}, va, {}, {}, {}, {}, {}, {}, {}, {}, {}, {},
+ {}, {}, {ag});
+ info.doLoop.setLoopAnnotationAttr(la);
+ }
+
/// Generate FIR to begin a structured or unstructured increment loop nest.
- void genFIRIncrementLoopBegin(IncrementLoopNestInfo &incrementLoopNestInfo) {
+ void genFIRIncrementLoopBegin(
+ IncrementLoopNestInfo &incrementLoopNestInfo,
+ llvm::SmallVectorImpl<const Fortran::parser::CompilerDirective *> &dirs) {
assert(!incrementLoopNestInfo.empty() && "empty loop nest");
mlir::Location loc = toLocation();
for (IncrementLoopInfo &info : incrementLoopNestInfo) {
@@ -1978,6 +1992,15 @@ class FirConverter : public Fortran::lower::AbstractConverter {
}
if (info.hasLocalitySpecs())
handleLocalitySpecs(info);
+
+ for (const auto *dir : dirs) {
+ std::visit(
+ Fortran::common::visitors{
+ [&](const Fortran::parser::CompilerDirective::VectorAlways
+ &d) { addLoopAnnotationAttr(info); },
+ [&](const auto &) {}},
+ dir->u);
+ }
continue;
}
@@ -2508,8 +2531,29 @@ class FirConverter : public Fortran::lower::AbstractConverter {
}
}
- void genFIR(const Fortran::parser::CompilerDirective &) {
- // TODO
+ void attachLoopDirective(const Fortran::parser::CompilerDirective &dir,
+ Fortran::lower::pft::Evaluation *e) {
+ while (e->isDirective()) {
+ e = e->lexicalSuccessor;
+ }
+
+ if (e->isA<Fortran::parser::NonLabelDoStmt>()) {
+ e->dirs.push_back(&dir);
+ } else {
+ fir::emitFatalError(toLocation(), "loop directive must appear before a loop");
+ }
+ }
+
+ void genFIR(const Fortran::parser::CompilerDirective &dir) {
+ Fortran::lower::pft::Evaluation &eval = getEval();
+
+ std::visit(
+ Fortran::common::visitors{
+ [&](const Fortran::parser::CompilerDirective::VectorAlways &) {
+ attachLoopDirective(dir, &eval);
+ },
+ [&](const auto &) {}},
+ dir.u);
}
void genFIR(const Fortran::parser::OpenACCConstruct &acc) {
diff --git a/flang/lib/Optimizer/Transforms/ControlFlowConverter.cpp b/flang/lib/Optimizer/Transforms/ControlFlowConverter.cpp
index a233e7fbdcd1e..b40c06de8787b 100644
--- a/flang/lib/Optimizer/Transforms/ControlFlowConverter.cpp
+++ b/flang/lib/Optimizer/Transforms/ControlFlowConverter.cpp
@@ -132,10 +132,14 @@ class CfgLoopConv : public mlir::OpRewritePattern<fir::DoLoopOp> {
auto comparison = rewriter.create<mlir::arith::CmpIOp>(
loc, arith::CmpIPredicate::sgt, itersLeft, zero);
- rewriter.create<mlir::cf::CondBranchOp>(
+ auto cond = rewriter.create<mlir::cf::CondBranchOp>(
loc, comparison, firstBlock, llvm::ArrayRef<mlir::Value>(), endBlock,
llvm::ArrayRef<mlir::Value>());
+ if (auto ann = loop.getLoopAnnotation()) {
+ cond->setAttr("loop_annotation", *ann);
+ }
+
// The result of the loop operation is the values of the condition block
// arguments except the induction variable on the last iteration.
auto args = loop.getFinalValue()
diff --git a/flang/lib/Parser/Fortran-parsers.cpp b/flang/lib/Parser/Fortran-parsers.cpp
index ff01974b549a1..d2241fb66a013 100644
--- a/flang/lib/Parser/Fortran-parsers.cpp
+++ b/flang/lib/Parser/Fortran-parsers.cpp
@@ -1276,10 +1276,13 @@ constexpr auto loopCount{
constexpr auto assumeAligned{"ASSUME_ALIGNED" >>
optionalList(construct<CompilerDirective::AssumeAligned>(
indirect(designator), ":"_tok >> digitString64))};
+constexpr auto vectorAlways{
+ "VECTOR ALWAYS" >> construct<CompilerDirective::VectorAlways>()};
TYPE_PARSER(beginDirective >> "DIR$ "_tok >>
sourced((construct<CompilerDirective>(ignore_tkr) ||
construct<CompilerDirective>(loopCount) ||
construct<CompilerDirective>(assumeAligned) ||
+ construct<CompilerDirective>(vectorAlways) ||
construct<CompilerDirective>(
many(construct<CompilerDirective::NameValue>(
name, maybe(("="_tok || ":"_tok) >> digitString64))))) /
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index bdd968b19a43f..5a4c7550d64dd 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -1824,6 +1824,9 @@ class UnparseVisitor {
Word("!DIR$ ASSUME_ALIGNED ");
Walk(" ", assumeAligned, ", ");
},
+ [&](const CompilerDirective::VectorAlways &valways) {
+ Word("!DIR$ VECTOR ALWAYS");
+ },
[&](const std::list<CompilerDirective::NameValue> &names) {
Walk("!DIR$ ", names, " ");
},
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index a46c0f378d5d0..e65788a02b725 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -8854,6 +8854,9 @@ void ResolveNamesVisitor::Post(const parser::AssignedGotoStmt &x) {
}
void ResolveNamesVisitor::Post(const parser::CompilerDirective &x) {
+ if (const auto *dir{
+ std::get_if<parser::CompilerDirective::VectorAlways>(&x.u)})
+ return;
if (const auto *tkr{
std::get_if<std::list<parser::CompilerDirective::IgnoreTKR>>(&x.u)}) {
if (currScope().IsTopLevel() ||
diff --git a/flang/test/Fir/vector-always.fir b/flang/test/Fir/vector-always.fir
new file mode 100644
index 0000000000000..b6dcf237ed59a
--- /dev/null
+++ b/flang/test/Fir/vector-always.fir
@@ -0,0 +1,42 @@
+// RUN: %flang_fc1 -emit-llvm -o - %s | FileCheck %s
+
+#access_group = #llvm.access_group<id = distinct[0]<>>
+#loop_vectorize = #llvm.loop_vectorize<disable = false>
+#loop_annotation = #llvm.loop_annotation<vectorize = #loop_vectorize, parallelAccesses = #access_group>
+
+// CHECK-LABEL: @vector_always_
+// CHECK: br i1 {{.*}}, label {{.*}}, label {{.*}}, !llvm.loop ![[ANNOTATION:.*]]
+func.func @_QPvector_always() {
+ %c1 = arith.constant 1 : index
+ %c10_i32 = arith.constant 10 : i32
+ %c1_i32 = arith.constant 1 : i32
+ %c10 = arith.constant 10 : index
+ %0 = fir.alloca !fir.array<10xi32> {bindc_name = "a", uniq_name = "_QFvector_alwaysEa"}
+ %1 = fir.shape %c10 : (index) -> !fir.shape<1>
+ %2 = fir.declare %0(%1) {uniq_name = "_QFvector_alwaysEa"} : (!fir.ref<!fir.array<10xi32>>, !fir.shape<1>) -> !fir.ref<!fir.array<10xi32>>
+ %3 = fir.alloca i32 {bindc_name = "i", uniq_name = "_QFvector_alwaysEi"}
+ %4 = fir.declare %3 {uniq_name = "_QFvector_alwaysEi"} : (!fir.ref<i32>) -> !fir.ref<i32>
+ %5 = fir.convert %c1_i32 : (i32) -> index
+ %6 = fir.convert %c10_i32 : (i32) -> index
+ %7 = fir.convert %5 : (index) -> i32
+ %8:2 = fir.do_loop %arg0 = %5 to %6 step %c1 iter_args(%arg1 = %7) -> (index, i32) attributes {loop_annotation = #loop_annotation} {
+ fir.store %arg1 to %4 : !fir.ref<i32>
+ %9 = fir.load %4 : !fir.ref<i32>
+ %10 = fir.load %4 : !fir.ref<i32>
+ %11 = fir.convert %10 : (i32) -> i64
+ %12 = fir.array_coor %2(%1) %11 : (!fir.ref<!fir.array<10xi32>>, !fir.shape<1>, i64) -> !fir.ref<i32>
+ fir.store %9 to %12 : !fir.ref<i32>
+ %13 = arith.addi %arg0, %c1 : index
+ %14 = fir.convert %c1 : (index) -> i32
+ %15 = fir.load %4 : !fir.ref<i32>
+ %16 = arith.addi %15, %14 : i32
+ fir.result %13, %16 : index, i32
+ }
+ fir.store %8#1 to %4 : !fir.ref<i32>
+ return
+ }
+
+// CHECK: ![[ANNOTATION]] = distinct !{![[ANNOTATION]], ![[VECTORIZE:.*]], ![[PAR_ACCESS:.*]]}
+// CHECK: ![[VECTORIZE]] = !{!"llvm.loop.vectorize.enable", i1 true}
+// CHECK: ![[PAR_ACCESS]] = !{!"llvm.loop.parallel_accesses", ![[DISTINCT:.*]]}
+// CHECK: ![[DISTINCT]] = distinct !{}
diff --git a/flang/test/Lower/vector-always.f90 b/flang/test/Lower/vector-always.f90
new file mode 100644
index 0000000000000..1994163626f16
--- /dev/null
+++ b/flang/test/Lower/vector-always.f90
@@ -0,0 +1,29 @@
+! RUN: %flang_fc1 -emit-fir -o - %s | FileCheck %s
+
+! CHECK: #access_group = #llvm.access_group<id = distinct[0]<>>
+! CHECK: #access_group1 = #llvm.access_group<id = distinct[1]<>>
+! CHECK: #loop_vectorize = #llvm.loop_vectorize<disable = false>
+! CHECK: #loop_annotation = #llvm.loop_annotation<vectorize = #loop_vectorize, parallelAccesses = #access_group>
+! CHECK: #loop_annotation1 = #llvm.loop_annotation<vectorize = #loop_vectorize, parallelAccesses = #access_group1>
+
+! CHECK-LABEL: vector_always
+subroutine vector_always
+ integer :: a(10)
+ !dir$ vector always
+ !CHECK: fir.do_loop {{.*}} attributes {loop_annotation = #loop_annotation}
+ do i=1,10
+ a(i)=i
+ end do
+end subroutine vector_always
+
+
+! CHECK-LABEL: intermediate_directive
+subroutine intermediate_directive
+ integer :: a(10)
+ !dir$ vector always
+ !dir$ unknown
+ !CHECK: fir.do_loop {{.*}} attributes {loop_annotation = #loop_annotation1}
+ do i=1,10
+ a(i)=i
+ end do
+end subroutine intermediate_directive
|
|
Note: for now this only works on plain do loops, I'd like to extend it for unstructured loops/loops with labels etc but wanted feedback on the approach first I'd appreciate feedback on the approach in general for tying directives to the following operations as this will be reused for other directives in future. |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
klausler
reviewed
May 30, 2024
|
@klausler Does the general approach look ok to you? I'm intending to do something similar for other directives that we need, as well as extending this to non-"NoLabelDoLoop"s |
There was a problem hiding this comment.
This is done by adding an attribute to the branch into the loop in LLVM
to indicate that the loop should always be vectorized.
Specify on which branch the attribute is being added here.
Also add to the summary all the variants not handled here.
-> unstructured loops
-> do concurrent
-> Array expressions (?)
Add an entry to the file https://github.com/llvm/llvm-project/blob/main/flang/docs/Directives.md
Yes. You might want to emit warnings about directives that don't immediately precede a |
tblah
reviewed
May 31, 2024
flang/lib/Lower/Bridge.cpp
Outdated
| void addLoopAnnotationAttr(IncrementLoopInfo &info) { | ||
| mlir::BoolAttr f = mlir::BoolAttr::get(builder->getContext(), false); | ||
| mlir::LLVM::LoopVectorizeAttr va = mlir::LLVM::LoopVectorizeAttr::get( | ||
| builder->getContext(), f, {}, {}, {}, {}, {}, {}); |
There was a problem hiding this comment.
nit: Please could you label what the false bool attribute is doing? Something like /*parameter_name=*/ will probably be enough.
There was a problem hiding this comment.
@DavidTruby Is this comment addressed?
I'm as happy as I ever get. |
klausler
approved these changes
Jun 7, 2024
flang/test/Fir/vector-always.fir
Outdated
| fir.result %c1, %c1_i32 : index, i32 | ||
| } | ||
| return %8#1 : i32 | ||
| } |
There was a problem hiding this comment.
Hi Kiran, what is the nit here?
There was a problem hiding this comment.
No newline at end of file.
|
Since there are some nontrivial changes here and it is the first time we are lowering directives, it is probably good to write a document that goes along with this patch. I am providing a sketch, you may choose to extend this or use a different one. |
This patch implements support for the VECTOR ALWAYS directive, which forces vectorization to occurr when possible regardless of a decision by the cost model. This is done by adding an attribute to the branch into the loop in LLVM to indicate that the loop should always be vectorized.
DavidTruby
force-pushed
the
vector_always
branch
from
213df8e
to
fefb03a
Compare
Other misc fixes for review
DavidTruby
force-pushed
the
vector_always
branch
from
fefb03a
to
8658dc4
Compare
flang/docs/Directives.md
Outdated
| with array expressions. | ||
|
|
||
| ## Semantics | ||
| Dirctives that are associated with constructs must appear in the same section as |
There was a problem hiding this comment.
Suggested change
| Dirctives that are associated with constructs must appear in the same section as | |
| Directives that are associated with constructs must appear in the same section as |
Comment on lines
1
to
2
| //===-- lib/Semantics/canonicalize-directives.h ------------------*- C++ | ||
| //-*-===// |
There was a problem hiding this comment.
Nit: All these should be one line.
I think I have resolved all of the comments now, are there any you think I have missed? |
kiranchandramohan
approved these changes
Jun 13, 2024
|
@DavidTruby, I'm seeing problems with this change. See issue #95780. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch implements support for the VECTOR ALWAYS directive, which forces
vectorization to occurr when possible regardless of a decision by the cost
model. This is done by adding an attribute to the branch into the loop in LLVM
to indicate that the loop should always be vectorized.