[Polly] Skip vectorize.enable for FP loops with dist=1 dependences (#205756)

When -polly-annotate-metadata-vectorize is active, Polly marks its
generated loops with llvm.loop.vectorize.enable=true. This is harmful
for loops with a loop-carried dependence of distance 1 that involve
floating-point operations: the Loop Vectorizer reorders FP operations
(e.g. scalar reduction like q = factor*q), producing results that differ
from the sequential scalar reference and causing correctness failures.

Two changes are made:

1. IslAst.cpp: add PollyVectorizeMetadata to the PerformParallelTest
gate so that dependence-distance computation is performed whenever
-polly-annotate-metadata-vectorize is passed, not only when
-polly-parallel or a vectorizer is active.

2. IslNodeBuilder.cpp / LoopGenerators.cpp: when a loop has a dist=1
dependence involving FP operations, omit the vectorize.enable annotation
entirely. This lets the Loop Vectorizer apply its own cost model and
legality checks decide.

polly/include/polly/CodeGen/LoopGenerators.h

@@ -70,13 +70,17 @@ extern int PollyChunkSize;
 ///                           assume it.
 /// @param LoopVectDisabled   If the Loop vectorizer should be disabled for this
 ///                           loop.
+/// @param SkipVectorizeEnableMetadata
+///                           If Polly should avoid setting
+///                           llvm.loop.vectorize.enable=true for this loop.
 ///
 /// @return Value*    The newly created induction variable for this loop.
 Value *createLoop(Value *LowerBound, Value *UpperBound, Value *Stride,
                   PollyIRBuilder &Builder, LoopInfo &LI, DominatorTree &DT,
                   BasicBlock *&ExitBlock, ICmpInst::Predicate Predicate,
                   ScopAnnotator *Annotator = nullptr, bool Parallel = false,
-                  bool UseGuard = true, bool LoopVectDisabled = false);
+                  bool UseGuard = true, bool LoopVectDisabled = false,
+                  bool SkipVectorizeEnableMetadata = false);
 
 /// Create a DebugLoc representing generated instructions.
 ///

polly/lib/CodeGen/IslAst.cpp

@@ -81,6 +81,8 @@ static cl::opt<bool> DetectParallel("polly-ast-detect-parallel",
                                     cl::desc("Detect parallelism"), cl::Hidden,
                                     cl::cat(PollyCategory));
 
+extern cl::opt<bool> PollyVectorizeMetadata;
+
 static cl::opt<bool>
     PollyPrintAst("polly-print-ast",
                   cl::desc("Print the ISL abstract syntax tree"),
@@ -501,7 +503,8 @@ IslAst::IslAst(IslAst &&O)
 
 void IslAst::init(const Dependences &D) {
   bool PerformParallelTest = PollyParallel || DetectParallel ||
-                             PollyVectorizerChoice != VECTORIZER_NONE;
+                             PollyVectorizerChoice != VECTORIZER_NONE ||
+                             PollyVectorizeMetadata;
   auto ScheduleTree = S.getScheduleTree();
 
   // Skip AST and code generation if there was no benefit achieved.

polly/lib/CodeGen/IslNodeBuilder.cpp

@@ -28,7 +28,6 @@
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/PostOrderIterator.h"
 #include "llvm/ADT/SetVector.h"
-#include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/Analysis/AssumptionCache.h"
 #include "llvm/Analysis/LoopInfo.h"
@@ -75,6 +74,9 @@
 using namespace llvm;
 using namespace polly;
 
+// Declared in LoopGenerators.cpp
+extern llvm::cl::opt<bool> PollyVectorizeMetadata;
+
 #define DEBUG_TYPE "polly-codegen"
 
 STATISTIC(VersionedScops, "Number of SCoPs that required versioning.");
@@ -434,6 +436,34 @@ static bool IsLoopVectorizerDisabled(isl::ast_node_for Node) {
   return false;
 }
 
+/// Returns true if the loop has a dist=1 dependence involving FP operations
+/// (array-carried RAW/WAW or scalar FP reduction). In that case we omit the
+/// vectorize.enable annotation and let the Loop Vectorizer decide.
+static bool hasLoopCarriedDependence(isl::ast_node_for For, const Scop &S) {
+  isl::pw_aff PwaDist = IslAstInfo::getMinimalDependenceDistance(For);
+  if (PwaDist.is_null())
+    return false;
+
+  isl::set Dist = isl::manage(isl_pw_aff_domain(PwaDist.copy()));
+  isl::pw_aff PwaOne = isl::pw_aff(Dist, isl::val::one(S.getIslCtx()));
+  if (isl_pw_aff_is_equal(PwaDist.get(), PwaOne.get()) != isl_bool_true)
+    return false;
+
+  // dist=1: suppress forced vectorization if the body has FP operations.
+  for (isl::set StmtSet :
+       IslAstInfo::getSchedule(For).domain().get_set_list()) {
+    auto *Stmt = static_cast<ScopStmt *>(StmtSet.get_tuple_id().get_user());
+    for (Instruction *Inst : Stmt->getInstructions()) {
+      if (Inst->getType()->isFloatingPointTy() ||
+          (Inst->getNumOperands() > 0 &&
+           Inst->getOperand(0)->getType()->isFloatingPointTy()))
+        return true;
+    }
+  }
+
+  return false;
+}
+
 void IslNodeBuilder::createForSequential(isl::ast_node_for For,
                                          bool MarkParallel) {
   Value *ValueLB, *ValueUB, *ValueInc;
@@ -478,9 +508,22 @@ void IslNodeBuilder::createForSequential(isl::ast_node_for For,
   // omit the GuardBB in front of the loop.
   bool UseGuardBB = !GenSE->isKnownPredicate(Predicate, GenSE->getSCEV(ValueLB),
                                              GenSE->getSCEV(ValueUB));
+
+  // FIXME: This is a workaround for
+  // https://github.com/llvm/llvm-project/issues/198726.
+  // llvm.loop.vectorize.enable=true has an additional property beyond
+  // requesting vectorization — it implicitly allows FP operation reordering.
+  // This is a limitation of the metadata format: there is no way to separate
+  // the request for vectorization from the request for reassociating FP ops.
+  // Once LoopVectorize is fixed to not reorder FP ops without explicit
+  // permission, this workaround can be removed.
+  // For now, skip vectorize.enable for dist=1 FP loops to avoid correctness
+  // failures from FP reassociation.
+  bool SkipVectorizeEnableMetadata = hasLoopCarriedDependence(For, S);
+
   IV = createLoop(ValueLB, ValueUB, ValueInc, Builder, *GenLI, *GenDT,
                   ExitBlock, Predicate, &Annotator, MarkParallel, UseGuardBB,
-                  LoopVectorizerDisabled);
+                  LoopVectorizerDisabled, SkipVectorizeEnableMetadata);
   IDToValue[IteratorID.get()] = IV;
 
   create(Body.release());

polly/lib/CodeGen/LoopGenerators.cpp

@@ -90,7 +90,8 @@ Value *polly::createLoop(Value *LB, Value *UB, Value *Stride,
                          DominatorTree &DT, BasicBlock *&ExitBB,
                          ICmpInst::Predicate Predicate,
                          ScopAnnotator *Annotator, bool Parallel, bool UseGuard,
-                         bool LoopVectDisabled) {
+                         bool LoopVectDisabled,
+                         bool SkipVectorizeEnableMetadata) {
   Function *F = Builder.GetInsertBlock()->getParent();
   LLVMContext &Context = F->getContext();
 
@@ -165,15 +166,14 @@ Value *polly::createLoop(Value *LB, Value *UB, Value *Stride,
   // Create the loop latch and annotate it as such.
   CondBrInst *B = Builder.CreateCondBr(LoopCondition, HeaderBB, ExitBB);
 
-  // Don't annotate vectorize metadata when both LoopVectDisabled and
-  // PollyVectorizeMetadata are disabled. Annotate vectorize metadata to false
-  // when LoopVectDisabled is true. Otherwise we annotate the vectorize metadata
-  // to true.
+  // Emit vectorize.enable=false only for explicit user disable
+  // (LoopVectDisabled). For dist=1 FP loops (SkipVectorizeEnableMetadata), omit
+  // the annotation and let the Loop Vectorizer decide.
   if (Annotator) {
     std::optional<bool> EnableVectorizeMetadata;
     if (LoopVectDisabled)
       EnableVectorizeMetadata = false;
-    else if (PollyVectorizeMetadata)
+    else if (PollyVectorizeMetadata && !SkipVectorizeEnableMetadata)
       EnableVectorizeMetadata = true;
     Annotator->annotateLoopLatch(B, Parallel, EnableVectorizeMetadata);
   }

polly/test/CodeGen/Metadata/basic_vec_annotate.ll

@@ -14,8 +14,8 @@
 ; CHECK: br {{.*}} !llvm.loop [[POLLY_LOOP:![0-9]+]]
 ; CHECK: [[LOOP]] = distinct !{[[LOOP]], [[META2:![0-9]+]], [[META3:![0-9]+]]}
 ; CHECK: [[META3]] = !{!"llvm.loop.vectorize.enable", i32 0}
-; CHECK: [[POLLY_LOOP]] = distinct !{[[POLLY_LOOP]], [[META2:![0-9]+]], [[META3:![0-9]+]]}
-; CHECK: [[META3]] = !{!"llvm.loop.vectorize.enable", i1 true}
+; CHECK: [[POLLY_LOOP]] = distinct !{[[POLLY_LOOP]], {{.*}}}
+; CHECK-DAG: !{!"llvm.loop.vectorize.enable", i1 true}
 
 target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128-Fn32"
 target triple = "aarch64-unknown-linux-gnu"

polly/test/CodeGen/Metadata/skip_vec_annotate_fp_dist1.ll

@@ -0,0 +1,56 @@
+; RUN: opt %loadNPMPolly -S '-passes=polly<no-default-opts>' -polly-annotate-metadata-vectorize < %s | FileCheck %s
+
+; Verify that vectorize.enable metadata is NOT added for a loop with a dist=1
+; dependence involving floating-point operations. This is a workaround for
+; https://github.com/llvm/llvm-project/issues/198726 where vectorize.enable
+; implicitly allows FP reassociation causing correctness failures.
+
+; void scale(double *A, double factor, int n) {
+;   for (int i = 1; i < n; i++)
+;     A[i] = factor * A[i - 1];
+; }
+
+; The Polly-generated loop should NOT have vectorize.enable metadata.
+; CHECK: polly.stmt.for.body:
+; CHECK: br {{.*}} !llvm.loop [[POLLY_LOOP:![0-9]+]]
+; CHECK: [[POLLY_LOOP]] = distinct !{[[POLLY_LOOP]],
+; CHECK-NOT: !"llvm.loop.vectorize.enable", i1 true
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128-Fn32"
+target triple = "aarch64-unknown-linux-gnu"
+
+define dso_local void @scale(ptr nocapture noundef %A, double noundef %factor, i32 noundef %n) local_unnamed_addr #0 {
+entry:
+  br label %entry.split
+
+entry.split:                                      ; preds = %entry
+  %cmp5 = icmp sgt i32 %n, 1
+  br i1 %cmp5, label %for.body.preheader, label %for.cond.cleanup
+
+for.body.preheader:                               ; preds = %entry.split
+  %wide.trip.count = zext nneg i32 %n to i64
+  br label %for.body
+
+for.cond.cleanup.loopexit:                        ; preds = %for.body
+  br label %for.cond.cleanup
+
+for.cond.cleanup:                                 ; preds = %for.cond.cleanup.loopexit, %entry.split
+  ret void
+
+for.body:                                         ; preds = %for.body.preheader, %for.body
+  %indvars.iv = phi i64 [ 1, %for.body.preheader ], [ %indvars.iv.next, %for.body ]
+  %0 = add nsw i64 %indvars.iv, -1
+  %arrayidx = getelementptr inbounds double, ptr %A, i64 %0
+  %1 = load double, ptr %arrayidx, align 8
+  %mul = fmul double %factor, %1
+  %arrayidx2 = getelementptr inbounds double, ptr %A, i64 %indvars.iv
+  store double %mul, ptr %arrayidx2, align 8
+  %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
+  %exitcond.not = icmp eq i64 %indvars.iv.next, %wide.trip.count
+  br i1 %exitcond.not, label %for.cond.cleanup.loopexit, label %for.body, !llvm.loop !0
+}
+
+attributes #0 = { nofree norecurse nosync nounwind memory(argmem: readwrite) uwtable "frame-pointer"="non-leaf" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="cortex-a57" "target-features"="+aes,+crc,+fp-armv8,+neon,+outline-atomics,+perfmon,+sha2,+v8a,-fmv" }
+
+!0 = distinct !{!0, !1}
+!1 = !{!"llvm.loop.mustprogress"}