[flang-commits] [flang] [flang] Implement !DIR$ VECTOR ALWAYS (PR #93830)

David Truby via flang-commits flang-commits at lists.llvm.org
Wed Jun 12 04:14:11 PDT 2024


https://github.com/DavidTruby updated https://github.com/llvm/llvm-project/pull/93830

>From 3d80163a572e6c622f5e0df0b73c8f23a9f6e192 Mon Sep 17 00:00:00 2001
From: David Truby <david.truby at arm.com>
Date: Thu, 30 May 2024 14:25:47 +0000
Subject: [PATCH 1/5] [flang] Implement !DIR$ VECTOR ALWAYS

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.
---
 flang/include/flang/Lower/PFTBuilder.h        |  1 +
 .../include/flang/Optimizer/Dialect/FIROps.h  |  1 +
 .../include/flang/Optimizer/Dialect/FIROps.td |  3 +-
 flang/include/flang/Parser/dump-parse-tree.h  |  1 +
 flang/include/flang/Parser/parse-tree.h       |  3 +-
 flang/lib/Lower/Bridge.cpp                    | 53 +++++++++++++++++--
 .../Transforms/ControlFlowConverter.cpp       |  6 ++-
 flang/lib/Parser/Fortran-parsers.cpp          |  3 ++
 flang/lib/Parser/unparse.cpp                  |  3 ++
 flang/lib/Semantics/resolve-names.cpp         |  3 ++
 flang/test/Fir/vector-always.fir              | 42 +++++++++++++++
 flang/test/Lower/vector-always.f90            | 29 ++++++++++
 12 files changed, 141 insertions(+), 7 deletions(-)
 create mode 100644 flang/test/Fir/vector-always.fir
 create mode 100644 flang/test/Lower/vector-always.f90

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 e7da3af5485cc..b2d55aea3161e 100644
--- a/flang/include/flang/Optimizer/Dialect/FIROps.td
+++ b/flang/include/flang/Optimizer/Dialect/FIROps.td
@@ -2160,7 +2160,8 @@ def fir_DoLoopOp : region_Op<"do_loop", [AttrSizedOperandSegments,
     Variadic<AnyType>:$initArgs,
     OptionalAttr<UnitAttr>:$unordered,
     OptionalAttr<UnitAttr>:$finalValue,
-    OptionalAttr<ArrayAttr>:$reduceAttrs
+    OptionalAttr<ArrayAttr>:$reduceAttrs,
+    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 4232e85a6e595..3a37dc141dfb3 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -204,6 +204,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 12e35075d2a69..8617863b11592 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3334,6 +3334,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;
@@ -3341,7 +3342,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 202efa57d4a36..30f22e311233a 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -1929,7 +1929,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();
@@ -1974,8 +1974,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) {
@@ -2040,6 +2054,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;
       }
 
@@ -2573,8 +2596,30 @@ 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 b98aae8e8f7a2..575dc29fd4e3f 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -1828,6 +1828,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 7397c3a51b61e..7889bdc2e23c4 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -8886,6 +8886,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

>From 1d3351465be68f588268c030985e4405628e4e06 Mon Sep 17 00:00:00 2001
From: David Truby <david.truby at arm.com>
Date: Fri, 31 May 2024 13:25:29 +0000
Subject: [PATCH 2/5] nit fixes for review

---
 flang/include/flang/Lower/PFTBuilder.h |  3 ++-
 flang/lib/Lower/Bridge.cpp             | 12 +++++-------
 flang/lib/Semantics/resolve-names.cpp  |  7 +++++--
 flang/test/Fir/vector-always.fir       |  2 +-
 flang/test/Lower/vector-always.f90     |  4 ++--
 5 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/flang/include/flang/Lower/PFTBuilder.h b/flang/include/flang/Lower/PFTBuilder.h
index aa83f1603c2a8..8bc5e02a23dcd 100644
--- a/flang/include/flang/Lower/PFTBuilder.h
+++ b/flang/include/flang/Lower/PFTBuilder.h
@@ -347,7 +347,8 @@ struct Evaluation : EvaluationVariant {
   parser::CharBlock position{};
   std::optional<parser::Label> label{};
   std::unique_ptr<EvaluationList> evaluationList; // nested evaluations
-  llvm::SmallVector<const parser::CompilerDirective *> dirs;
+  // associated compiler directives
+  llvm::SmallVector<const parser::CompilerDirective *, 1> 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/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index 30f22e311233a..6ebaa78323d63 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -2596,18 +2596,16 @@ class FirConverter : public Fortran::lower::AbstractConverter {
     }
   }
 
-  void attachLoopDirective(const Fortran::parser::CompilerDirective &dir,
+  void attachDirectiveToLoop(const Fortran::parser::CompilerDirective &dir,
                            Fortran::lower::pft::Evaluation *e) {
-    while (e->isDirective()) {
+    while (e->isDirective())
       e = e->lexicalSuccessor;
-    }
 
-    if (e->isA<Fortran::parser::NonLabelDoStmt>()) {
+    if (e->isA<Fortran::parser::NonLabelDoStmt>())
       e->dirs.push_back(&dir);
-    } else {
+    else
       fir::emitFatalError(toLocation(),
                           "loop directive must appear before a loop");
-    }
   }
 
   void genFIR(const Fortran::parser::CompilerDirective &dir) {
@@ -2616,7 +2614,7 @@ class FirConverter : public Fortran::lower::AbstractConverter {
     std::visit(
         Fortran::common::visitors{
             [&](const Fortran::parser::CompilerDirective::VectorAlways &) {
-              attachLoopDirective(dir, &eval);
+              attachDirectiveToLoop(dir, &eval);
             },
             [&](const auto &) {}},
         dir.u);
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index 7889bdc2e23c4..5f16d37b8a9ce 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -8886,9 +8886,12 @@ 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)})
+  //if (const auto *dir{
+  //        std::get_if<parser::CompilerDirective::VectorAlways>(&x.u)})
+
+  if (std::holds_alternative<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
index b6dcf237ed59a..e7e07c41ffee9 100644
--- a/flang/test/Fir/vector-always.fir
+++ b/flang/test/Fir/vector-always.fir
@@ -19,7 +19,7 @@ func.func @_QPvector_always() {
     %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} {
+    %8:2 = fir.do_loop %arg0 = %5 to %6 step %c1 iter_args(%arg1 = %7) -> (index, i32) attributes {loopAnnotation = #loop_annotation} {
       fir.store %arg1 to %4 : !fir.ref<i32>
       %9 = fir.load %4 : !fir.ref<i32>
       %10 = fir.load %4 : !fir.ref<i32>
diff --git a/flang/test/Lower/vector-always.f90 b/flang/test/Lower/vector-always.f90
index 1994163626f16..5806054078f7f 100644
--- a/flang/test/Lower/vector-always.f90
+++ b/flang/test/Lower/vector-always.f90
@@ -10,7 +10,7 @@
 subroutine vector_always
   integer :: a(10)
   !dir$ vector always
-  !CHECK: fir.do_loop {{.*}} attributes {loop_annotation = #loop_annotation}
+  !CHECK: fir.do_loop {{.*}} attributes {loopAnnotation = #loop_annotation}
   do i=1,10
      a(i)=i
   end do
@@ -22,7 +22,7 @@ subroutine intermediate_directive
   integer :: a(10)
   !dir$ vector always
   !dir$ unknown
-  !CHECK: fir.do_loop {{.*}} attributes {loop_annotation = #loop_annotation1}
+  !CHECK: fir.do_loop {{.*}} attributes {loopAnnotation = #loop_annotation1}
   do i=1,10
      a(i)=i
   end do

>From 55926990309586a6369ebcfdfdea2582541a465f Mon Sep 17 00:00:00 2001
From: David Truby <david.truby at arm.com>
Date: Thu, 6 Jun 2024 14:28:45 +0000
Subject: [PATCH 3/5] Add check for directive location in semantics

Refactor tests
---
 flang/docs/Directives.md                      |   3 +
 flang/include/flang/Parser/dump-parse-tree.h  |   4 +-
 flang/lib/Lower/Bridge.cpp                    |   7 +-
 flang/lib/Semantics/CMakeLists.txt            |   1 +
 .../lib/Semantics/canonicalize-directives.cpp | 122 ++++++++++++++++++
 flang/lib/Semantics/canonicalize-directives.h |  22 ++++
 flang/lib/Semantics/resolve-names.cpp         |   6 +-
 flang/lib/Semantics/semantics.cpp             |   2 +
 flang/test/Fir/vector-always-cfg.fir          |  32 +++++
 flang/test/Fir/vector-always.fir              |  43 ++----
 flang/test/Integration/vector-always.f90      |  16 +++
 flang/test/Parser/compiler-directives.f90     |   9 +-
 flang/test/Semantics/loop-directives.f90      |  13 ++
 13 files changed, 238 insertions(+), 42 deletions(-)
 create mode 100644 flang/lib/Semantics/canonicalize-directives.cpp
 create mode 100644 flang/lib/Semantics/canonicalize-directives.h
 create mode 100644 flang/test/Fir/vector-always-cfg.fir
 create mode 100644 flang/test/Integration/vector-always.f90
 create mode 100644 flang/test/Semantics/loop-directives.f90

diff --git a/flang/docs/Directives.md b/flang/docs/Directives.md
index fe08b4f855f23..4bd5f39f14243 100644
--- a/flang/docs/Directives.md
+++ b/flang/docs/Directives.md
@@ -36,3 +36,6 @@ A list of non-standard directives supported by Flang
   and is limited to 256.
   [This directive is currently recognised by the parser, but not
   handled by the other parts of the compiler].
+* `!dir$ vector always` forces vectorization on the following loop regardless 
+  of cost model decisions. The loop must still be vectorizable.
+  [This directive currently only works on plain do loops without labels].
diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index 3a37dc141dfb3..37c3370b48a08 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -201,12 +201,12 @@ class ParseTreeDumper {
   NODE(parser, CommonStmt)
   NODE(CommonStmt, Block)
   NODE(parser, CompilerDirective)
+  NODE(CompilerDirective, AssumeAligned)
   NODE(CompilerDirective, IgnoreTKR)
   NODE(CompilerDirective, LoopCount)
-  NODE(CompilerDirective, AssumeAligned)
-  NODE(CompilerDirective, VectorAlways)
   NODE(CompilerDirective, NameValue)
   NODE(CompilerDirective, Unrecognized)
+  NODE(CompilerDirective, VectorAlways)
   NODE(parser, ComplexLiteralConstant)
   NODE(parser, ComplexPart)
   NODE(parser, ComponentArraySpec)
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index 6ebaa78323d63..3ecf99093be12 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -1977,12 +1977,13 @@ class FirConverter : public Fortran::lower::AbstractConverter {
   void addLoopAnnotationAttr(IncrementLoopInfo &info) {
     mlir::BoolAttr f = mlir::BoolAttr::get(builder->getContext(), false);
     mlir::LLVM::LoopVectorizeAttr va = mlir::LLVM::LoopVectorizeAttr::get(
-        builder->getContext(), f, {}, {}, {}, {}, {}, {});
+        builder->getContext(), /*disable=*/f, {}, {}, {}, {}, {}, {});
+    // Create distinct access group
     mlir::LLVM::AccessGroupAttr ag =
         mlir::LLVM::AccessGroupAttr::get(builder->getContext());
     mlir::LLVM::LoopAnnotationAttr la = mlir::LLVM::LoopAnnotationAttr::get(
-        builder->getContext(), {}, va, {}, {}, {}, {}, {}, {}, {}, {}, {}, {},
-        {}, {}, {ag});
+        builder->getContext(), {}, /*vectorize=*/va, {}, {}, {}, {}, {}, {}, {},
+        {}, {}, {}, {}, {}, /*parallelAccess=*/{ag});
     info.doLoop.setLoopAnnotationAttr(la);
   }
 
diff --git a/flang/lib/Semantics/CMakeLists.txt b/flang/lib/Semantics/CMakeLists.txt
index 809206565fc1c..41406ecf50e00 100644
--- a/flang/lib/Semantics/CMakeLists.txt
+++ b/flang/lib/Semantics/CMakeLists.txt
@@ -2,6 +2,7 @@ add_flang_library(FortranSemantics
   assignment.cpp
   attr.cpp
   canonicalize-acc.cpp
+  canonicalize-directives.cpp
   canonicalize-do.cpp
   canonicalize-omp.cpp
   check-acc-structure.cpp
diff --git a/flang/lib/Semantics/canonicalize-directives.cpp b/flang/lib/Semantics/canonicalize-directives.cpp
new file mode 100644
index 0000000000000..a3908127d9e8b
--- /dev/null
+++ b/flang/lib/Semantics/canonicalize-directives.cpp
@@ -0,0 +1,122 @@
+//===-- lib/Semantics/check-directives.cpp --------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "canonicalize-directives.h"
+#include "flang/Parser/parse-tree-visitor.h"
+
+namespace Fortran::semantics {
+
+using namespace parser::literals;
+
+// Check that directives are associated with the correct constructs
+class CanonicalizationOfDirectives {
+public:
+  CanonicalizationOfDirectives(parser::Messages &messages)
+      : messages_{messages} {}
+
+  template <typename T> bool Pre(T &) { return true; }
+  template <typename T> void Post(T &) {}
+
+  // Move directives that must appear in the Execution part out of the
+  // Specification part.
+  void Post(parser::SpecificationPart &spec);
+  bool Pre(parser::ExecutionPart &x);
+
+  // Ensure that directives associated with constructs appear accompanying the
+  // construct.
+  void Post(parser::Block &block);
+
+private:
+  // Ensure that loop directives appear immediately before a loop.
+  void CheckLoopDirective(parser::CompilerDirective &dir, parser::Block &block,
+      std::list<parser::ExecutionPartConstruct>::iterator it);
+
+  parser::Messages &messages_;
+
+  // Directives to be moved to the Execution part from the Specification part.
+  std::list<common::Indirection<parser::CompilerDirective>>
+      directivesToConvert_;
+};
+
+bool CanonicalizeDirectives(
+    parser::Messages &messages, parser::Program &program) {
+  CanonicalizationOfDirectives dirs{messages};
+  Walk(program, dirs);
+  return !messages.AnyFatalError();
+}
+
+static bool IsExecutionDirective(const parser::CompilerDirective &dir) {
+  return std::holds_alternative<parser::CompilerDirective::VectorAlways>(dir.u);
+}
+
+void CanonicalizationOfDirectives::Post(parser::SpecificationPart &spec) {
+  auto &list{
+      std::get<std::list<common::Indirection<parser::CompilerDirective>>>(
+          spec.t)};
+  for (auto it{list.begin()}; it != list.end();) {
+    if (IsExecutionDirective(it->value())) {
+      directivesToConvert_.emplace_back(std::move(*it));
+      it = list.erase(it);
+    } else {
+      ++it;
+    }
+  }
+}
+
+bool CanonicalizationOfDirectives::Pre(parser::ExecutionPart &x) {
+  auto origFirst{x.v.begin()};
+  for (auto &dir : directivesToConvert_) {
+    x.v.insert(origFirst,
+        parser::ExecutionPartConstruct{
+            parser::ExecutableConstruct{std::move(dir)}});
+  }
+
+  directivesToConvert_.clear();
+  return true;
+}
+
+template <typename T> T *GetConstructIf(parser::ExecutionPartConstruct &x) {
+  if (auto *y{std::get_if<parser::ExecutableConstruct>(&x.u)}) {
+    if (auto *z{std::get_if<common::Indirection<T>>(&y->u)}) {
+      return &z->value();
+    }
+  }
+  return nullptr;
+}
+
+void CanonicalizationOfDirectives::CheckLoopDirective(
+    parser::CompilerDirective &dir, parser::Block &block,
+    std::list<parser::ExecutionPartConstruct>::iterator it) {
+
+  // Skip over this and other compiler directives
+  while (GetConstructIf<parser::CompilerDirective>(*it)) {
+    ++it;
+  }
+
+  if (it == block.end() || !GetConstructIf<parser::DoConstruct>(*it)) {
+    std::string s{parser::ToUpperCaseLetters(dir.source.ToString())};
+    s.pop_back(); // Remove trailing newline from source string
+    messages_.Say(
+        dir.source, "A DO loop must follow the %s directive"_err_en_US, s);
+  }
+}
+
+void CanonicalizationOfDirectives::Post(parser::Block &block) {
+  for (auto it = block.begin(); it != block.end(); ++it) {
+    if (auto *dir{GetConstructIf<parser::CompilerDirective>(*it)}) {
+      std::visit(
+          common::visitors{[&](parser::CompilerDirective::VectorAlways &) {
+                             CheckLoopDirective(*dir, block, it);
+                           },
+              [&](auto &) {}},
+          dir->u);
+    }
+  }
+}
+
+} // namespace Fortran::semantics
diff --git a/flang/lib/Semantics/canonicalize-directives.h b/flang/lib/Semantics/canonicalize-directives.h
new file mode 100644
index 0000000000000..52c74d91dbff8
--- /dev/null
+++ b/flang/lib/Semantics/canonicalize-directives.h
@@ -0,0 +1,22 @@
+//===-- lib/Semantics/check-directives.h ------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef FORTRAN_SEMANTICS_CHECK_DIRECTIVES_H_
+#define FORTRAN_SEMANTICS_CHECK_DIRECTIVES_H_
+
+namespace Fortran::parser {
+struct Program;
+class Messages;
+} // namespace Fortran::parser
+
+namespace Fortran::semantics {
+bool CanonicalizeDirectives(
+    parser::Messages &messages, parser::Program &program);
+}
+
+#endif // FORTRAN_SEMANTICS_CHECK_DIRECTIVES_H_
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index 5f16d37b8a9ce..677f2cf78c13c 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -8886,10 +8886,8 @@ 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)})
-
-  if (std::holds_alternative<parser::CompilerDirective::VectorAlways>(x.u)) {
+  if (const auto *dir{
+          std::get_if<parser::CompilerDirective::VectorAlways>(&x.u)}) {
     return;
   }
   if (const auto *tkr{
diff --git a/flang/lib/Semantics/semantics.cpp b/flang/lib/Semantics/semantics.cpp
index d51cc62d804e8..1bb0679b75110 100644
--- a/flang/lib/Semantics/semantics.cpp
+++ b/flang/lib/Semantics/semantics.cpp
@@ -9,6 +9,7 @@
 #include "flang/Semantics/semantics.h"
 #include "assignment.h"
 #include "canonicalize-acc.h"
+#include "canonicalize-directives.h"
 #include "canonicalize-do.h"
 #include "canonicalize-omp.h"
 #include "check-acc-structure.h"
@@ -599,6 +600,7 @@ bool Semantics::Perform() {
       CanonicalizeAcc(context_.messages(), program_) &&
       CanonicalizeOmp(context_.messages(), program_) &&
       CanonicalizeCUDA(program_) &&
+      CanonicalizeDirectives(context_.messages(), program_) &&
       PerformStatementSemantics(context_, program_) &&
       ModFileWriter{context_}.WriteAll();
 }
diff --git a/flang/test/Fir/vector-always-cfg.fir b/flang/test/Fir/vector-always-cfg.fir
new file mode 100644
index 0000000000000..45c2ea056a707
--- /dev/null
+++ b/flang/test/Fir/vector-always-cfg.fir
@@ -0,0 +1,32 @@
+// RUN: fir-opt --fir-to-llvm-ir %s | FileCheck %s
+
+#access_group = #llvm.access_group<id = distinct[0]<>>
+// CHECK: #[[ACCESS:.*]] = #llvm.access_group<id = distinct[0]<>>
+#loop_vectorize = #llvm.loop_vectorize<disable = false>
+// CHECK: #[[VECTORIZE:.*]] = #llvm.loop_vectorize<disable = false>
+#loop_annotation = #llvm.loop_annotation<vectorize = #loop_vectorize, parallelAccesses = #access_group>
+// CHECK: #[[ANNOTATION:.*]] = #llvm.loop_annotation<vectorize = #[[VECTORIZE]], parallelAccesses = #[[ACCESS]]>
+
+func.func @_QPvector_always() -> i32 {
+  %c1 = arith.constant 1 : index
+  %c10_i32 = arith.constant 10 : i32
+  %c1_i32 = arith.constant 1 : i32
+  %c10 = arith.constant 10 : index
+  %0 = arith.subi %c10, %c1 : index
+  %1 = arith.addi %0, %c1 : index
+  %2 = arith.divsi %1, %c1 : index
+  cf.br ^bb1(%c1, %c1_i32, %2 : index, i32, index)
+^bb1(%3: index, %4: i32, %5: index):  // 2 preds: ^bb0, ^bb2
+  %c0 = arith.constant 0 : index
+  %6 = arith.cmpi sgt, %5, %c0 : index
+  cf.cond_br %6, ^bb2, ^bb3 {loop_annotation = #loop_annotation}
+// CHECK:   llvm.cond_br %{{.*}}, ^{{.*}}, ^{{.*}} {loop_annotation = #[[ANNOTATION]]}
+^bb2:  // pred: ^bb1
+  %7 = arith.addi %3, %c1 : index
+  %c1_0 = arith.constant 1 : index
+  %8 = arith.subi %5, %c1_0 : index
+  cf.br ^bb1(%7, %c1_i32, %8 : index, i32, index)
+^bb3:  // pred: ^bb1
+  return %4 : i32
+}
+
diff --git a/flang/test/Fir/vector-always.fir b/flang/test/Fir/vector-always.fir
index e7e07c41ffee9..4ebb7912e137b 100644
--- a/flang/test/Fir/vector-always.fir
+++ b/flang/test/Fir/vector-always.fir
@@ -1,42 +1,21 @@
-// RUN: %flang_fc1 -emit-llvm -o - %s | FileCheck %s
+// RUN: fir-opt --cfg-conversion %s | FileCheck %s
 
 #access_group = #llvm.access_group<id = distinct[0]<>>
+// CHECK: #[[ACCESS:.*]] = #llvm.access_group<id = distinct[0]<>>
 #loop_vectorize = #llvm.loop_vectorize<disable = false>
+// CHECK: #[[VECTORIZE:.*]] = #llvm.loop_vectorize<disable = false>
 #loop_annotation = #llvm.loop_annotation<vectorize = #loop_vectorize, parallelAccesses = #access_group>
+// CHECK: #[[ANNOTATION:.*]] = #llvm.loop_annotation<vectorize = #[[VECTORIZE]], parallelAccesses = #[[ACCESS]]>
 
-// CHECK-LABEL: @vector_always_
-// CHECK:   br i1 {{.*}}, label {{.*}}, label {{.*}}, !llvm.loop ![[ANNOTATION:.*]]
-func.func @_QPvector_always() {
+// CHECK-LABEL: @_QPvector_always
+func.func @_QPvector_always() -> i32 {
     %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 {loopAnnotation = #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
+// CHECK:   cf.cond_br %{{.*}}, ^{{.*}}, ^{{.*}} {loop_annotation = #[[ANNOTATION]]}
+    %8:2 = fir.do_loop %arg0 = %c1 to %c10 step %c1 iter_args(%arg1 = %c1_i32) -> (index, i32) attributes {loopAnnotation = #loop_annotation} {
+      fir.result %c1, %c1_i32 : 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 !{}
+    return %8#1 : i32
+  }
\ No newline at end of file
diff --git a/flang/test/Integration/vector-always.f90 b/flang/test/Integration/vector-always.f90
new file mode 100644
index 0000000000000..5aa04248886cd
--- /dev/null
+++ b/flang/test/Integration/vector-always.f90
@@ -0,0 +1,16 @@
+! RUN: %flang_fc1 -emit-llvm -o - %s | FileCheck %s
+
+! CHECK-LABEL: vector_always
+subroutine vector_always
+  integer :: a(10)
+  !dir$ vector always
+  ! CHECK:   br i1 {{.*}}, label {{.*}}, label {{.*}}, !llvm.loop ![[ANNOTATION:.*]]
+  do i=1,10
+     a(i)=i
+  end do
+end subroutine vector_always
+
+! 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/Parser/compiler-directives.f90 b/flang/test/Parser/compiler-directives.f90
index d4c99ae12f14e..246eaf985251c 100644
--- a/flang/test/Parser/compiler-directives.f90
+++ b/flang/test/Parser/compiler-directives.f90
@@ -1,4 +1,4 @@
-! RUN: %flang_fc1 -fdebug-unparse %s 2>&1
+! RUN: %flang_fc1 -fdebug-unparse %s 2>&1 | FileCheck %s
 
 ! Test that compiler directives can appear in various places.
 
@@ -28,3 +28,10 @@ module m
      !dir$  align : 1024 :: d
   end type stuff
 end
+
+subroutine vector_always
+  !dir$ vector always
+  ! CHECK: !DIR$ VECTOR ALWAYS
+  do i=1,10
+  enddo
+end subroutine
diff --git a/flang/test/Semantics/loop-directives.f90 b/flang/test/Semantics/loop-directives.f90
new file mode 100644
index 0000000000000..9c7e6dadad3bd
--- /dev/null
+++ b/flang/test/Semantics/loop-directives.f90
@@ -0,0 +1,13 @@
+! RUN: %python %S/test_errors.py %s %flang
+
+program empty
+  ! ERROR: A DO loop must follow the VECTOR ALWAYS directive
+  !dir$ vector always
+end program empty
+
+program non_do
+  ! ERROR: A DO loop must follow the VECTOR ALWAYS directive
+  !dir$ vector always
+  a = 1
+end program non_do
+

>From 8658dc492bcf0f1d47abbbdf98c1e14b4f37ae44 Mon Sep 17 00:00:00 2001
From: David Truby <david.truby at arm.com>
Date: Tue, 11 Jun 2024 15:07:07 +0000
Subject: [PATCH 4/5] Add documentation for change in Directives.md

Other misc fixes for review
---
 flang/docs/Directives.md                      | 42 ++++++++++++++++++-
 .../include/flang/Optimizer/Dialect/FIROps.td |  2 +-
 flang/lib/Lower/Bridge.cpp                    |  2 +-
 .../Transforms/ControlFlowConverter.cpp       |  4 +-
 .../lib/Semantics/canonicalize-directives.cpp |  9 ++--
 flang/lib/Semantics/canonicalize-directives.h |  9 ++--
 flang/test/Fir/vector-always.fir              |  2 +-
 flang/test/Lower/vector-always.f90            |  2 +-
 flang/test/Semantics/loop-directives.f90      | 14 +++++--
 9 files changed, 68 insertions(+), 18 deletions(-)

diff --git a/flang/docs/Directives.md b/flang/docs/Directives.md
index 4bd5f39f14243..0c16a20de3507 100644
--- a/flang/docs/Directives.md
+++ b/flang/docs/Directives.md
@@ -36,6 +36,46 @@ A list of non-standard directives supported by Flang
   and is limited to 256.
   [This directive is currently recognised by the parser, but not
   handled by the other parts of the compiler].
-* `!dir$ vector always` forces vectorization on the following loop regardless 
+* `!dir$ vector always` forces vectorization on the following loop regardless
   of cost model decisions. The loop must still be vectorizable.
   [This directive currently only works on plain do loops without labels].
+
+# Directive Details
+
+## Introduction
+Directives are commonly used in Fortran programs to specify additional actions 
+to be performed by the compiler. The directives are always specified with the 
+`!dir$` or `cdir$` prefix. 
+
+## Loop Directives
+Some directives are associated with the following construct, for example loop
+directives. Directives on loops are used to specify additional transformation to
+be performed by the compiler like enabling vectorisation, unrolling, interchange
+etc.
+
+### Array Expressions
+It is to be decided whether loop directives should also be able to be associated
+with array expressions.
+
+## Semantics
+Dirctives that are associated with constructs must appear in the same section as
+the construct they are associated with, for example loop directives must appear
+in the executable section as the loops appear there. To facilitate this the
+parse tree is corrected to move such directives that appear in the specification
+part into the execution part.
+When a directive that must be associated with a construct appears, a search 
+forward from that directive to the next non-directive construct is performed to
+check that that construct matches the expected construct for the directive.
+Skipping other intermediate directives allows multiple directives to appear on
+the same construct.
+
+## Lowering 
+Evaluation is extended with a new field called dirs for representing directives
+associated with that Evaluation. When lowering loop directives, the associated
+Do Loop's evaluation is found and the directive is added to it. This information
+is used only during the lowering of the loop.
+
+## Testing
+Since directives must maintain a flow from source to LLVM IR, an integration
+test is provided that tests the `vector always` directive, as well as individual
+lit tests for each of the parsing, semantics and lowering stages.
diff --git a/flang/include/flang/Optimizer/Dialect/FIROps.td b/flang/include/flang/Optimizer/Dialect/FIROps.td
index b2d55aea3161e..baf095263479b 100644
--- a/flang/include/flang/Optimizer/Dialect/FIROps.td
+++ b/flang/include/flang/Optimizer/Dialect/FIROps.td
@@ -2161,7 +2161,7 @@ def fir_DoLoopOp : region_Op<"do_loop", [AttrSizedOperandSegments,
     OptionalAttr<UnitAttr>:$unordered,
     OptionalAttr<UnitAttr>:$finalValue,
     OptionalAttr<ArrayAttr>:$reduceAttrs,
-    OptionalAttr<LoopAnnotationAttr>:$loop_annotation
+    OptionalAttr<LoopAnnotationAttr>:$loopAnnotation
   );
   let results = (outs Variadic<AnyType>:$results);
   let regions = (region SizedRegion<1>:$region);
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index 3ecf99093be12..81ff08a55b8bd 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -2598,7 +2598,7 @@ class FirConverter : public Fortran::lower::AbstractConverter {
   }
 
   void attachDirectiveToLoop(const Fortran::parser::CompilerDirective &dir,
-                           Fortran::lower::pft::Evaluation *e) {
+                             Fortran::lower::pft::Evaluation *e) {
     while (e->isDirective())
       e = e->lexicalSuccessor;
 
diff --git a/flang/lib/Optimizer/Transforms/ControlFlowConverter.cpp b/flang/lib/Optimizer/Transforms/ControlFlowConverter.cpp
index b40c06de8787b..1af5a68e85297 100644
--- a/flang/lib/Optimizer/Transforms/ControlFlowConverter.cpp
+++ b/flang/lib/Optimizer/Transforms/ControlFlowConverter.cpp
@@ -136,9 +136,9 @@ class CfgLoopConv : public mlir::OpRewritePattern<fir::DoLoopOp> {
         loc, comparison, firstBlock, llvm::ArrayRef<mlir::Value>(), endBlock,
         llvm::ArrayRef<mlir::Value>());
 
-    if (auto ann = loop.getLoopAnnotation()) {
+    // Copy loop annotations from the do loop to the loop entry condition.
+    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.
diff --git a/flang/lib/Semantics/canonicalize-directives.cpp b/flang/lib/Semantics/canonicalize-directives.cpp
index a3908127d9e8b..d4f8dc1dcdf9e 100644
--- a/flang/lib/Semantics/canonicalize-directives.cpp
+++ b/flang/lib/Semantics/canonicalize-directives.cpp
@@ -1,4 +1,5 @@
-//===-- lib/Semantics/check-directives.cpp --------------------------------===//
+//===-- lib/Semantics/canonicalize-directives.cpp
+//--------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -13,7 +14,9 @@ namespace Fortran::semantics {
 
 using namespace parser::literals;
 
-// Check that directives are associated with the correct constructs
+// Check that directives are associated with the correct constructs.
+// Directives that need to be associated with other constructs in the execution
+// part are moved to the execution part so they can be checked there.
 class CanonicalizationOfDirectives {
 public:
   CanonicalizationOfDirectives(parser::Messages &messages)
@@ -107,7 +110,7 @@ void CanonicalizationOfDirectives::CheckLoopDirective(
 }
 
 void CanonicalizationOfDirectives::Post(parser::Block &block) {
-  for (auto it = block.begin(); it != block.end(); ++it) {
+  for (auto it{block.begin()}; it != block.end(); ++it) {
     if (auto *dir{GetConstructIf<parser::CompilerDirective>(*it)}) {
       std::visit(
           common::visitors{[&](parser::CompilerDirective::VectorAlways &) {
diff --git a/flang/lib/Semantics/canonicalize-directives.h b/flang/lib/Semantics/canonicalize-directives.h
index 52c74d91dbff8..ca5b36f70ce41 100644
--- a/flang/lib/Semantics/canonicalize-directives.h
+++ b/flang/lib/Semantics/canonicalize-directives.h
@@ -1,4 +1,5 @@
-//===-- lib/Semantics/check-directives.h ------------------------*- C++ -*-===//
+//===-- lib/Semantics/canonicalize-directives.h ------------------*- C++
+//-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -6,8 +7,8 @@
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef FORTRAN_SEMANTICS_CHECK_DIRECTIVES_H_
-#define FORTRAN_SEMANTICS_CHECK_DIRECTIVES_H_
+#ifndef FORTRAN_SEMANTICS_CANONICALIZE_DIRECTIVES_H_
+#define FORTRAN_SEMANTICS_CANONICALIZE_DIRECTIVES_H_
 
 namespace Fortran::parser {
 struct Program;
@@ -19,4 +20,4 @@ bool CanonicalizeDirectives(
     parser::Messages &messages, parser::Program &program);
 }
 
-#endif // FORTRAN_SEMANTICS_CHECK_DIRECTIVES_H_
+#endif // FORTRAN_SEMANTICS_CANONICALIZE_DIRECTIVES_H_
diff --git a/flang/test/Fir/vector-always.fir b/flang/test/Fir/vector-always.fir
index 4ebb7912e137b..00eb0e7a756ee 100644
--- a/flang/test/Fir/vector-always.fir
+++ b/flang/test/Fir/vector-always.fir
@@ -18,4 +18,4 @@ func.func @_QPvector_always() -> i32 {
       fir.result %c1, %c1_i32 : index, i32
     }
     return %8#1 : i32
-  }
\ No newline at end of file
+  }
diff --git a/flang/test/Lower/vector-always.f90 b/flang/test/Lower/vector-always.f90
index 5806054078f7f..275ab6d102455 100644
--- a/flang/test/Lower/vector-always.f90
+++ b/flang/test/Lower/vector-always.f90
@@ -1,4 +1,4 @@
-! RUN: %flang_fc1 -emit-fir -o - %s | FileCheck %s
+! RUN: %flang_fc1 -emit-hlfir -o - %s | FileCheck %s
 
 ! CHECK: #access_group = #llvm.access_group<id = distinct[0]<>>
 ! CHECK: #access_group1 = #llvm.access_group<id = distinct[1]<>>
diff --git a/flang/test/Semantics/loop-directives.f90 b/flang/test/Semantics/loop-directives.f90
index 9c7e6dadad3bd..e2807c1f9d0e2 100644
--- a/flang/test/Semantics/loop-directives.f90
+++ b/flang/test/Semantics/loop-directives.f90
@@ -1,13 +1,19 @@
 ! RUN: %python %S/test_errors.py %s %flang
 
-program empty
+subroutine empty
   ! ERROR: A DO loop must follow the VECTOR ALWAYS directive
   !dir$ vector always
-end program empty
+end subroutine empty
 
-program non_do
+subroutine non_do
   ! ERROR: A DO loop must follow the VECTOR ALWAYS directive
   !dir$ vector always
   a = 1
-end program non_do
+end subroutine non_do
 
+subroutine execution_part
+  do i=1,10
+  ! ERROR: A DO loop must follow the VECTOR ALWAYS directive
+  !dir$ vector always
+  end do
+end subroutine execution_part

>From 1121dc58c3af5d937b2142ecb16b13152ca6c5a2 Mon Sep 17 00:00:00 2001
From: David Truby <david.truby at arm.com>
Date: Wed, 12 Jun 2024 11:10:12 +0000
Subject: [PATCH 5/5] More fixes for review

---
 flang/docs/Directives.md                      | 24 ++++++++++++++-----
 flang/lib/Lower/Bridge.cpp                    |  5 +---
 .../lib/Semantics/canonicalize-directives.cpp |  3 +--
 flang/lib/Semantics/canonicalize-directives.h |  3 +--
 flang/test/Integration/vector-always.f90      |  4 +---
 flang/test/Lower/vector-always.f90            |  7 ++----
 6 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/flang/docs/Directives.md b/flang/docs/Directives.md
index 0c16a20de3507..ce31615dc4ef6 100644
--- a/flang/docs/Directives.md
+++ b/flang/docs/Directives.md
@@ -58,12 +58,13 @@ It is to be decided whether loop directives should also be able to be associated
 with array expressions.
 
 ## Semantics
-Dirctives that are associated with constructs must appear in the same section as
-the construct they are associated with, for example loop directives must appear
-in the executable section as the loops appear there. To facilitate this the
-parse tree is corrected to move such directives that appear in the specification
-part into the execution part.
-When a directive that must be associated with a construct appears, a search 
+Directives that are associated with constructs must appear in the same section
+as the construct they are associated with, for example loop directives must
+appear in the executable section as the loops appear there. To facilitate this
+the parse tree is corrected to move such directives that appear in the
+specification part into the execution part.
+
+When a directive that must be associated with a construct appears, a search
 forward from that directive to the next non-directive construct is performed to
 check that that construct matches the expected construct for the directive.
 Skipping other intermediate directives allows multiple directives to appear on
@@ -75,6 +76,17 @@ associated with that Evaluation. When lowering loop directives, the associated
 Do Loop's evaluation is found and the directive is added to it. This information
 is used only during the lowering of the loop.
 
+### Representation in LLVM
+The `llvm.loop` metadata is used in LLVM to provide information to the optimizer
+about the loop. For example, the `llvm.loop.vectorize.enable` metadata informs
+the optimizer that a loop can be vectorized without considering its cost-model.
+This attribute is added to the loop condition branch.
+
+### Representation in MLIR 
+The MLIR LLVM dialect models this by an attribute called LoopAnnotation
+Attribute. The attribute can be added to the latch of the loop in the cf
+dialect and is then carried through lowering to the LLVM dialect.
+
 ## Testing
 Since directives must maintain a flow from source to LLVM IR, an integration
 test is provided that tests the `vector always` directive, as well as individual
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index 81ff08a55b8bd..57c371b671595 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -1978,12 +1978,9 @@ class FirConverter : public Fortran::lower::AbstractConverter {
     mlir::BoolAttr f = mlir::BoolAttr::get(builder->getContext(), false);
     mlir::LLVM::LoopVectorizeAttr va = mlir::LLVM::LoopVectorizeAttr::get(
         builder->getContext(), /*disable=*/f, {}, {}, {}, {}, {}, {});
-    // Create distinct access group
-    mlir::LLVM::AccessGroupAttr ag =
-        mlir::LLVM::AccessGroupAttr::get(builder->getContext());
     mlir::LLVM::LoopAnnotationAttr la = mlir::LLVM::LoopAnnotationAttr::get(
         builder->getContext(), {}, /*vectorize=*/va, {}, {}, {}, {}, {}, {}, {},
-        {}, {}, {}, {}, {}, /*parallelAccess=*/{ag});
+        {}, {}, {}, {}, {}, {});
     info.doLoop.setLoopAnnotationAttr(la);
   }
 
diff --git a/flang/lib/Semantics/canonicalize-directives.cpp b/flang/lib/Semantics/canonicalize-directives.cpp
index d4f8dc1dcdf9e..4bf36754eb10b 100644
--- a/flang/lib/Semantics/canonicalize-directives.cpp
+++ b/flang/lib/Semantics/canonicalize-directives.cpp
@@ -1,5 +1,4 @@
-//===-- lib/Semantics/canonicalize-directives.cpp
-//--------------------------===//
+//===-- lib/Semantics/canonicalize-directives.cpp -------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
diff --git a/flang/lib/Semantics/canonicalize-directives.h b/flang/lib/Semantics/canonicalize-directives.h
index ca5b36f70ce41..89f8a0e3fce4a 100644
--- a/flang/lib/Semantics/canonicalize-directives.h
+++ b/flang/lib/Semantics/canonicalize-directives.h
@@ -1,5 +1,4 @@
-//===-- lib/Semantics/canonicalize-directives.h ------------------*- C++
-//-*-===//
+//===-- lib/Semantics/canonicalize-directives.h -----------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
diff --git a/flang/test/Integration/vector-always.f90 b/flang/test/Integration/vector-always.f90
index 5aa04248886cd..7216698f901c1 100644
--- a/flang/test/Integration/vector-always.f90
+++ b/flang/test/Integration/vector-always.f90
@@ -10,7 +10,5 @@ subroutine vector_always
   end do
 end subroutine vector_always
 
-! CHECK: ![[ANNOTATION]] = distinct !{![[ANNOTATION]], ![[VECTORIZE:.*]], ![[PAR_ACCESS:.*]]}
+! CHECK: ![[ANNOTATION]] = distinct !{![[ANNOTATION]], ![[VECTORIZE:.*]]}
 ! 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
index 275ab6d102455..1822fc33dfdb8 100644
--- a/flang/test/Lower/vector-always.f90
+++ b/flang/test/Lower/vector-always.f90
@@ -1,10 +1,7 @@
 ! RUN: %flang_fc1 -emit-hlfir -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: #loop_annotation = #llvm.loop_annotation<vectorize = #loop_vectorize>
 
 ! CHECK-LABEL: vector_always
 subroutine vector_always
@@ -22,7 +19,7 @@ subroutine intermediate_directive
   integer :: a(10)
   !dir$ vector always
   !dir$ unknown
-  !CHECK: fir.do_loop {{.*}} attributes {loopAnnotation = #loop_annotation1}
+  !CHECK: fir.do_loop {{.*}} attributes {loopAnnotation = #loop_annotation}
   do i=1,10
      a(i)=i
   end do



More information about the flang-commits mailing list