[PATCH] D93478: [LoopVectorizer] Fix invalid scenario that is allowed to interleave

[Danilo Carvalho Grael via Phabricator via llvm-commits]

dancgr created this revision.
Herald added a subscriber: hiraditya.
dancgr requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Tentative change to Loop Vectorizer to avoid a case of interleaving that creates incorrect code.

This problem is described on bug https://bugs.llvm.org/show_bug.cgi?id=48546.

In short, there is a case of compiling the following code with -O2 which triggers the loop vectorizer bug with clang -O2 test.c

  int printf(const char *, ...);
  int a, b, e;
  char c, d;
  int main() {
    d = 19;
    for (; d < 45; d++) {
      char *f = &c;
      *f &= 1;
      e = b== 0 ?:a%b;
      (*f)--;
    }
    printf("%d\n", c);
  }

The output compilation produces incorrect code. The value printed is 254 instead of 0.

This is because we determine that we can interleave this loop, when instead it should not be allowed.

There are cases without interleaving that would make it legal to vectorize this code, so the condition is placed only if we can interleave.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93478

Files:
  llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h
  llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
  llvm/lib/Transforms/Vectorize/LoopVectorize.cpp


Index: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
===================================================================
--- llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -8066,6 +8066,13 @@
     IC = CM.selectInterleaveCount(VF.Width, VF.Cost, LVI);
   }
 
+  // Check if it is legal to interleave the loop.
+  if (IC > 1 && !LVL.isLegalToInterleave()) {
+    LLVM_DEBUG(dbgs() << "LV: Not vectorizing: Can't interleave reduction.\n");
+    Hints.emitRemarkWithHints();
+    return false;
+  }
+
   // Identify the diagnostic messages that should be produced.
   std::pair<StringRef, std::string> VecDiagMsg, IntDiagMsg;
   bool VectorizeLoop = true, InterleaveLoop = true;
Index: llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
===================================================================
--- llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
+++ llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
@@ -1263,4 +1263,18 @@
   return true;
 }
 
+bool LoopVectorizationLegality::isLegalToInterleave() {
+  // Check if reduction has invalid type for interleaving.
+  for (auto &Reduction : *getReductionVars()) {
+    auto Descriptor = Reduction.second;
+    auto ComputedType = Descriptor.getRecurrenceType();
+    auto IsSigned = Descriptor.isSigned();
+    // Eliminate degenerate types.
+    if (ComputedType->isIntegerTy(1) && IsSigned) {
+      return false;
+    }
+  }
+  return true;
+}
+
 } // namespace llvm
Index: llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h
===================================================================
--- llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h
+++ llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h
@@ -313,6 +313,8 @@
   // Returns true if the NoNaN attribute is set on the function.
   bool hasFunNoNaNAttr() const { return HasFunNoNaNAttr; }
 
+  bool isLegalToInterleave();
+
 private:
   /// Return true if the pre-header, exiting and latch blocks of \p Lp and all
   /// its nested loops are considered legal for vectorization. These legal


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D93478.312568.patch
Type: text/x-patch
Size: 2144 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201217/3a3fa7bf/attachment.bin>