[SVE] Add support for scalable vectors with vectorize.scalable.enable loop attribute

[david-arm]

Summary

In this patch I have added support for a new loop hint called
vectorize.scalable.enable that says whether we should enable scalable
vectorization or not. If a user wants to instruct the compiler to
vectorize a loop with scalable vectors they can now do this as
follows:

br i1 %exitcond, label %for.end, label %for.body, !llvm.loop !2
...
!2 = !{!2, !3, !4}
!3 = !{!"llvm.loop.vectorize.width", i32 8}
!4 = !{!"llvm.loop.vectorize.scalable.enable", i1 true}

Setting the hint to false simply reverts the behaviour back to the
default, using fixed width vectors.

[fhahn]

Could you also update the documentation for llvm.loop.vectorize.width? https://llvm.org/docs/LangRef.html#llvm-loop-vectorize-width-metadata

llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h

54

might be good to add comments indicating what the fields are used for.

llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp

67

are you anticipating additional hints with ElementCount metadata? If not, it might be simpler to just deal with HK_WIDTH up front and validate Val[0] and Val[1] here and leave the code handling the other cases mostly unchanged? Or maybe use named variables instead of an array, to make things a bit clearer?

[david-arm]

llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp

67

So the reason I structured it like this is because we are still maintaining backwards compatibility with the old style and allowing single integer constants instead of the node. I could deal with the HK_WIDTH up front, but I'd still need all this code. Would you be ok with using named variables instead?

[david-arm]

• I've updated the documentation for the new attribute format.
• Added comments to the union members.
• Renamed the variables in validateAndSet.

[sdesmalen]

llvm/docs/LangRef.rst

5893 ↗

(On Diff #296895)

Rather than talking about two forms, is it sufficient to say:

The vector width is an ElementCount tuple, represented in Metadata as:

.. code-block:: llvm

   !0 = !{!"llvm.loop.vectorize.width", !1}
   !1 = !{i32 4, i32 1}

where ``i32 4`` specifies the vector width and ``i32 1`` indicates if the vectorization factor is scalable, meaning that the loop-vectorizer should use vector-length agnostic vectorization.

For fixed-width vectorizatoin-factors, a short-hand `i32` operand for llvm.loop.vectorize.width is also supported.

.. code-block:: llvm
    !0 = !{!"llvm.loop.vectorize.width", i32 4}

llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp

67

nit: For the cases below, is it worthing using something like:

auto MaySetIntValue = [this](int IntVal, bool Condition) { if (Condition) this->Value.U32 = IntVal; return Condition; };
auto MaySetECValue = [this](ElementCount EC, bool Condition) { if (Condition) this->Value.EC = EC; return Condition; };

switch (Kind) {
case HK_WIDTH:
  return MaySetECValue(ElementCount::get(IntVal, IsScalable), isPowerOf2_32(IntVal) && IntVal <= VectorizerParams::MaxVectorWidth);
case HK_UNROLL:
  return MaySetIntValue(IntVal, isPowerOf2_32(Val) && Val <= MaxInterleaveFactor);
[...]
}

[david-arm]

• Updated documentation for vectorize_width loop attribute.
• Added lambda functions to be used when validating and setting loop hint attributes.

[sdesmalen]

LGTM with nits addressed. @fhahn are you happy with the changes?

llvm/docs/LangRef.rst

5909 ↗

(On Diff #298980)

nit: s/i32 1/i1 true/

5911 ↗

(On Diff #298980)

nit:

s/vector width/minimum known vector width/
s/and `i32 1`/and the non-zero value `i1 true`/

llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp

61

nit: s/unsigned IsScalable = 0;/bool IsScalable = false;/

79

nit: s/Maybe/Conditionally/

85

nit: this is only used once, so better to inline.

llvm/test/Transforms/LoopVectorize/metadata-width.ll

55

nit: s/i32 0/i1 false/

[david-arm]

llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp

61

Given that it is possible to specify this as "i32" type, even if the docs says "i1", are you saying that it's fine to specify the vector width as "i32 4, i32 345", for example? In this example with your changes "i32 345" will be treated as scalable = true. This is the reason I had this value as i32, in order to capture odd cases.

[fhahn]

In D88962#2354863, @sdesmalen wrote:

LGTM with nits addressed. @fhahn are you happy with the changes?

LG in general, thanks for the updates.

llvm/docs/LangRef.rst

5911 ↗

(On Diff #298980)

nit: I'd start with saying that the first value of the tuple is the vector width and the second value indicates whether the vectorization factor is scalable and then follow with the example saying this means min vector width of 4 & scalable vectorization

[david-arm]

llvm/test/Transforms/LoopVectorize/metadata-width.ll

55

If I make this change the test fails. We will still always print out "i32 0". Is there a way to control the format generated?

[david-arm]

llvm/test/Transforms/LoopVectorize/metadata-width.ll

55

Sorry I just realised this is an input to the test, rather than testing the output! Please ignore my comment.

[david-arm]

• Updated documentation.
• Addressed review comments.

[david-arm]

Hi @fhahn are you happy with the changes now and happy to accept them?

[fhahn]

This comment has been deleted.

llvm/docs/LangRef.rst

5933 ↗

(On Diff #301657)

ElementCount is an internal name of the LLVM codebase I guess and nothing defines it in LangRef? I think it would be better to drop it, because here it is just a regular metadata tuple and you already outline what the fields hold.

llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h

63

nit: using {0} seems inconsistent with the other braces used here?

llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp

66

might be good to a comment that we are looking for a (width, isScalable) tuple here.

269

C is only used in the condition, maybe just check mdconst::dyn_extract<ConstantInt>(Arg) in the if?

But now that we pass the metadata node to validateAndSet, do we actually need the check here or can we just do all the validation in the validateAndSet? Seems like the time-savings by existing a bit earlier would be very minor.

llvm/test/Transforms/LoopVectorize/metadata-width.ll

55

also needs a test which actually sets isScalable to true?

[david-arm]

llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h

63

Good spot. Since I'm initialising Value explictly below I can simply remove "Value{0}, " entirely.

llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp

269

Sure, sounds sensible!

llvm/test/Transforms/LoopVectorize/metadata-width.ll

55

Unfortunately I can't do that at the moment because the vectorizer crashes when trying to perform scalable vectorisation. This is why I only tested the 'false' case. In theory I could create a negative test that tests the vectoriser crashes, which we could remove later once we support vectorisation. Any thoughts @fhahn or @sdesmalen ?

[sdesmalen]

llvm/test/Transforms/LoopVectorize/metadata-width.ll

55

The proof of concept for scalable loop vectorization (D90343) depends on this patch and shows the isScalable = true case does something sensible with this information. Hopefully that is sufficient for now while we work on actually implementing scalable auto-vec support in the LoopVectorizer.

[sdesmalen]

[this time adding the comment to the right patch]

Hi @david-arm I just found that two uses of llvm.loop.vectorize.width are not yet updated.

• WarnMissedTransforms.cpp in warnAboutLeftoverTransformations.
• LoopUtils.cpp in llvm::hasVectorizeTransformation.

The cases seem quite trivial to fix up, can you include those changes in this patch?

[david-arm]

• Fixed up some remaining vectorize.width attribute cases.

[fhahn]

llvm/test/Transforms/LoopVectorize/metadata-width.ll

55

Hm, I am a bit reluctant to add something to LangRef which causes passes to crash on valid IR. Would it be possible to either convert the scalable VF into a fixed one (replacing n with 1 should be valid?), or just bail out on a scalable VF, without too much effort?

[sdesmalen]

llvm/test/Transforms/LoopVectorize/metadata-width.ll

55

Hm, I am a bit reluctant to add something to LangRef which causes passes to crash on valid IR. Would it be possible to either convert the scalable VF into a fixed one (replacing n with 1 should be valid?), or just bail out on a scalable VF, without too much effort?

That would lead to a bit of a chicken/egg problem, because we're adding this feature so we can incrementally make the loop vectorizer cope with scalable vectors. There are currently too many code-paths in there to fix up in one go. D90343 uses this metadata to enable vectorization of an individual loop with scalable vectors and gives us a route to enable and test individual loops in unit tests or in existing code (using the Clang attribute in D89031). The fact that the vectorizer currently crashes on most inputs, is a bug, it just happens to be a bug we know about. It is also a temporary broken state that we will want fix as soon as possible.

Is it not possible to add a line to the LangRef saying that this is experimental until the loop vectorizer supports scalable vectors? (and we remove that line when the vectorizer is stable enough).

llvm/test/Transforms/LoopVectorize/no_array_bounds2.ll

3 ↗

(On Diff #303482)

Can you add a description of what this file is testing?

For the file name itself, is there a better name than no_array_bounds2.ll ?

[fhahn]

llvm/test/Transforms/LoopVectorize/metadata-width.ll

55

That would lead to a bit of a chicken/egg problem, because we're adding this feature so we can incrementally make the loop vectorizer cope with scalable vectors. There are currently too many code-paths in there to fix up in one go. D90343 uses this metadata to enable vectorization of an individual loop with scalable vectors and gives us a route to enable and test individual loops in unit tests or in existing code (using the Clang attribute in D89031). The fact that the vectorizer currently crashes on most inputs, is a bug, it just happens to be a bug we know about. It is also a temporary broken state that we will want fix as soon as possible.

That makes sense, my suggestion was to just convert the scalable user VF to a fixed one after getting the hint instead of crashing. As in

   // Get user vectorization factor and interleave count.
   ElementCount UserVF = Hints.getWidth();
+  if (UserVF.isScalable()) {
+    // TODO: Use scalable user VF, once LV is ready. For now, just assume n == 1.
+    UserVF = ElementCount::getFixed(UserVF.getKnownMinValue());
+  }
   unsigned UserIC = Hints.getInterleave();

Only processLoopInVPlanNativePath and processLoop would need to be updated I think. as we are free to drop the hints, I think that would be preferable to crashing and should be legal?

[sdesmalen]

llvm/test/Transforms/LoopVectorize/metadata-width.ll

55

Okay, so you mean assuming Fixed for now and then re-enable this in e.g. D91077 (which adds scalable vector support for some simple loop, but which doesn't yet guarantee that it won't crash on all inputs).
Yes, I think that makes sense.

[fhahn]

llvm/test/Transforms/LoopVectorize/metadata-width.ll

55

+1

[david-arm]

• In LoopVectorize.cpp I've forced the UserVF to fall back on fixed width vectorisation for now.
• Added comments to the new test file and renamed it.

[sdesmalen]

Can you add a test-case to llvm/test/Transforms/LoopVectorize/metadata-width.ll where it also tries to compile a loop with scalable VF? (and ensure it falls back to the fixed-width case until we add support for it)

llvm/docs/LangRef.rst

5933–5934 ↗

(On Diff #304478)

nit: @c-rhodes and I noticed yesterday that the term minimum vector width can give confusion, because it is not the width of the vector that the metadata describes (in terms of bytes), but rather the vectorization factor, i.e. the (minimum) number of vector lanes used to vectorize a loop. Can you clarify that in the description?

[david-arm]

• Reverted changes to vectorize.width loop hint attribute.
• Added new vectorize.scalable.enable loop hint attribute to control scalable vectorisation.

[david-arm]

After discussions on the child patch (D89031) we've decided to do things differently so instead of adapting vectorize.width to accept a tuple we're now adding a new vectorize.scalable.enable hint that can be used with vectorize.width to create a ElementCount. For the clang patch I also intend to update it so that we use a new vectorize_style #pragma instead of using vectorize_width to force scalable vectorisation.

[paulwalker-arm]

Hi @david-arm, can you document the reasons for the change?

Personally, I don't care much about the details of the metadata layout so if this switched layout represents value then great. However, I'm less happy with the proposed pragma change because there's been a lot of effort spent (with plenty more to come) converting existing fixed length only representations of vectors into new types that can represent both fixed and scalable vectors. To then force this information to be split into its constitute parts at the user level (i.e. pragmas and command line options) seems like a backward step.

[david-arm]

Hi @paulwalker-arm, so the reason for this change is related to the child patch where we changed the vectorize_width #pragma to accept an additional optional argument "scalable" or "fixed". @SjoerdMeijer felt this was the wrong approach and that the scalable property should be specified only as an additional pragma, i.e.

#pragma clang loop vectorize_width(4) vectorize_style(scalable)

The reason for this is that we would have to add vectorize_style(scalable) anyway to support vectorisation where the width isn't specified, i.e.

#pragma clang loop some_other_pragma vectorize_style(scalable)

Also, if we support both a vectorize_style(scalable|fixed) pragma in addition to specifying the scalable property in vectorize_width it also means extra work managing potential conflicts. As a result of this change in approach it made sense to update the LLVM loop hint attribute too to reflect this.

[paulwalker-arm]

Thanks for the info @david-arm. I just figured we'd support vector_width(2), vector_width(2, fixed), vector_width(2, scalable), vector_width(fixed), vector_width(scalable) so I still say splitting the width property across multiple pragmas is against our goal of moving away from fixed length only representations. That said, if this is the consensus then so be it.

[SjoerdMeijer]

In D88962#2402292, @paulwalker-arm wrote:

Thanks for the info @david-arm. I just figured we'd support vector_width(2), vector_width(2, fixed), vector_width(2, scalable), vector_width(fixed), vector_width(scalable) so I still say splitting the width property across multiple pragmas is against our goal of moving away from fixed length only representations. That said, if this is the consensus then so be it.

I won't block D89031 and would be happy to be convinced otherwise, but I don't see any advantages of D89031, in fact I see only disadvantages.

The approach here looks good to me; have inlined a question.

llvm/docs/LangRef.rst

5930 ↗

(On Diff #306006)

I am wondering if we need to describe if and how this interacts with llvm.loop.vectorize.enable?

llvm/test/Transforms/LoopVectorize/no_array_bounds_scalable.ll

65 ↗

(On Diff #306006)

nit: we probably don't need all of this.

[david-arm]

• Updated the documentation to mention that the new attribute only has any effect if vectorisation is enabled for the loop.
• Cleaned up one of the test files.

[david-arm]

Hi @fhahn @SjoerdMeijer @paulwalker-arm, are you happy with the patch now? The implementation on the LLVM IR side of things is independent of the clang pragma patch. For the clang patch I intend to write a proposal to the mailing list for changing the vectorize_width #pragma. @fhahn would you be happy with removing the "Request changes" cross?

llvm/docs/LangRef.rst

5930 ↗

(On Diff #306006)

I adapted this documentation from the vectorize.predicate.enable case above, which didn't discuss the interaction with vectorize.enable so I just thought I didn't need to here either. I've made it clear that this flag only has any effect if vectorisation is already enabled (although this could be through simply building with -O2).

[fhahn]

The updated metadata looks fine and should be flexible enough to cover the scenarios outlined by @paulwalker-arm, whatever the clang support looks like. A few small remaining comments.

llvm/include/llvm/Transforms/Utils/LoopUtils.h

217 ↗

(On Diff #307029)

It this is exposed here, this should probably have a comment describing what this does?

llvm/lib/Transforms/Utils/LoopUtils.cpp

306 ↗

(On Diff #307029)

Here we get both the integer values of llvm.loop.vectorize.width and llvm.loop.vectorize.scalable.enable", right? Could this be simplified by using the existing getOptionalIntLoopAttribute?

llvm/test/Transforms/LoopVectorize/no_array_bounds_scalable.ll

65 ↗

(On Diff #306006)

looks like the TBAA metadata still can be removed?

[david-arm]

• Added documentation to getOptionalElementCountLoopAttribute and simplified the implementation in the function.
• Removed some tbaa metadata from the tests.

[fhahn]

LGTM, thanks. Given that there has been quite some discussion, it would be good to wait with landing this for a few days, in case there are any more comments.

llvm/include/llvm/Transforms/Utils/LoopUtils.h

220 ↗

(On Diff #307537)

nit: drop llvm:: (there are some inconsistencies about that in the file, but it should not be required and it is also not used for other things in the llvm namespace, like Loop).

llvm/lib/Transforms/Utils/LoopUtils.cpp

475 ↗

(On Diff #307537)

nit: the extra () are not needed?

[SjoerdMeijer]

I am also happy with this (and the new proposal in D89031 and the mail to cfe dev list).

What do we do when vectorize.scalable is not supported by the target? We want to issue a diagnostic/remark? Do we need to say something about this in the LangRef part?

[david-arm]

Hi @SjoerdMeijer, what I have in the clang pragma patch at the moment is a warning emitted when the target doesn't support scalable vectors, and falling back on fixed width. However, this is something for discussion as I believe there are others who think that the fronted should simply accept the scalable hint regardless and let the vectoriser make that decision based upon cost analysis.

[sdesmalen]

In D88962#2422190, @SjoerdMeijer wrote:

What do we do when vectorize.scalable is not supported by the target? We want to issue a diagnostic/remark? Do we need to say something about this in the LangRef part?

Yes, although I think that's worth addressing separately, I made a similar comment on D91718 where such functionality is needed. If that's addressed separately, are you happy @david-arm to land this patch @SjoerdMeijer?

(it is worth noting that this patch still ignores the 'scalable' VF in the vectorizer. This is only enabled again in D91077, which has a dependence on this patch)

[SjoerdMeijer]

Yep, LGTM too

llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h

 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_TRANSFORMS_VECTORIZE_LOOPVECTORIZATIONLEGALITY_H
 #define LLVM_TRANSFORMS_VECTORIZE_LOOPVECTORIZATIONLEGALITY_H
 
 #include "llvm/ADT/MapVector.h"
 #include "llvm/Analysis/LoopAccessAnalysis.h"
 #include "llvm/Analysis/OptimizationRemarkEmitter.h"
+#include "llvm/Support/TypeSize.h"
 #include "llvm/Transforms/Utils/LoopUtils.h"
 
 namespace llvm {
 
 /// Utility class for getting and setting loop vectorizer hints in the form
 /// of loop metadata.
 /// This class keeps a number of loop annotations locally (as member variables)
 /// and can, upon request, write them back as metadata on the loop. It will
 /// initially scan the loop for existing metadata, and will update the local
 /// values based on information in the loop.
 /// We cannot write all values to metadata, as the mere presence of some info,
 /// for example 'force', means a decision has been made. So, we need to be
 /// careful NOT to add them if the user hasn't specifically asked so.
 class LoopVectorizeHints {
   enum HintKind { HK_WIDTH, HK_UNROLL, HK_FORCE, HK_ISVECTORIZED,
                   HK_PREDICATE };
 
   /// Hint - associates name and validation with the hint value.
   struct Hint {
     const char *Name;
-    unsigned Value; // This may have to change for non-numeric values.
+    union {
+      unsigned U32;
+      ElementCount EC;
+    } Value;
     HintKind Kind;
 
     Hint(const char *Name, unsigned Value, HintKind Kind)
-        : Name(Name), Value(Value), Kind(Kind) {}
+        : Name(Name), Value({Value}), Kind(Kind) {}
 
-    bool validate(unsigned Val);
+    Hint(const char *Name, ElementCount EC) : Name(Name), Kind(HK_WIDTH) {
+      Value.EC = EC;
+    }
+
+    bool validateAndSet(const Metadata *Arg);
   };
 
   /// Vectorization width.
   Hint Width;
 
   /// Vectorization interleave factor.
   Hint Interleave;
 
public:
   void setAlreadyVectorized();
 
   bool allowVectorization(Function *F, Loop *L,
                           bool VectorizeOnlyWhenForced) const;
 
   /// Dumps all the hint information.
   void emitRemarkWithHints() const;
 
-  unsigned getWidth() const { return Width.Value; }
-  unsigned getInterleave() const { return Interleave.Value; }
-  unsigned getIsVectorized() const { return IsVectorized.Value; }
-  unsigned getPredicate() const { return Predicate.Value; }
+  ElementCount getWidth() const { return Width.Value.EC; }
+  unsigned getInterleave() const { return Interleave.Value.U32; }
+  unsigned getIsVectorized() const { return IsVectorized.Value.U32; }
+  unsigned getPredicate() const { return Predicate.Value.U32; }
   enum ForceKind getForce() const {
-    if ((ForceKind)Force.Value == FK_Undefined &&
+    if ((ForceKind)Force.Value.U32 == FK_Undefined &&
         hasDisableAllTransformsHint(TheLoop))
       return FK_Disabled;
-    return (ForceKind)Force.Value;
+    return (ForceKind)Force.Value.U32;
   }
 
   /// If hints are provided that force vectorization, use the AlwaysPrint
   /// pass name to force the frontend to print the diagnostic.
   const char *vectorizeAnalysisPassName() const;
 
   bool allowReordering() const {
     // When enabling loop hints are provided we allow the vectorizer to change
     // the order of operations that is given by the scalar loop. This is not
     // enabled by default because can be unsafe or inefficient. For example,
     // reordering floating-point operations will change the way round-off
     // error accumulates in the loop.
-    return getForce() == LoopVectorizeHints::FK_Enabled || getWidth() > 1;
+    ElementCount EC = getWidth();
+    return getForce() == LoopVectorizeHints::FK_Enabled ||
+           EC.getKnownMinValue() > 1;
   }
 
   bool isPotentiallyUnsafe() const {
     // Avoid FP vectorization if the target is unsure about proper support.
     // This may be related to the SIMD unit in the target not handling
     // IEEE 754 FP ops properly, or bad single-to-double promotions.
     // Otherwise, a sequence of vectorized loops, even without reduction,
     // could lead to different end results on the destination vectors.
     return getForce() != LoopVectorizeHints::FK_Enabled && PotentiallyUnsafe;
   }
 
   void setPotentiallyUnsafe() { PotentiallyUnsafe = true; }
 
 private:
   /// Find hints specified in the loop metadata and update local values.
   void getHintsFromMetadata();
 
   /// Checks string hint with one operand and set value if valid.
-  void setHint(StringRef Name, Metadata *Arg);
+  void setHint(StringRef Name, const Metadata *Arg);
 
   /// The loop these hints belong to.
   const Loop *TheLoop;
 
   /// Interface to emit optimization remarks.
   OptimizationRemarkEmitter &ORE;
 };
 

llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp

static cl::opt<unsigned> PragmaVectorizeSCEVCheckThreshold(
     cl::desc("The maximum number of SCEV checks allowed with a "
              "vectorize(enable) pragma"));
 
 /// Maximum vectorization interleave count.
 static const unsigned MaxInterleaveFactor = 16;
 
 namespace llvm {
 
-bool LoopVectorizeHints::Hint::validate(unsigned Val) {
+bool LoopVectorizeHints::Hint::validateAndSet(const Metadata *Arg) {
+  unsigned Vals[2] = {0, 0};
+
+  if (const ConstantInt *C = mdconst::dyn_extract<ConstantInt>(Arg))
+    Vals[0] = C->getZExtValue();
+  else if (const MDNode *MD = dyn_cast<MDNode>(Arg)) {
+    if (Kind != HK_WIDTH || MD->getNumOperands() != 2)
+      return false;
+    const ConstantInt *C0 =
+        mdconst::dyn_extract<ConstantInt>(MD->getOperand(0));
+    const ConstantInt *C1 =
+        mdconst::dyn_extract<ConstantInt>(MD->getOperand(1));
+    if (!C0 || !C1)
+      return false;
+    Vals[0] = C0->getZExtValue();
+    Vals[1] = C1->getZExtValue();
+  } else
+    return false;
+
   switch (Kind) {
-  case HK_WIDTH:
-    return isPowerOf2_32(Val) && Val <= VectorizerParams::MaxVectorWidth;
+  case HK_WIDTH: {
+    if (!isPowerOf2_32(Vals[0]) || Vals[0] > VectorizerParams::MaxVectorWidth)
+      return false;
+
+    if (Vals[1] > 1)
+      return false;
+
+    Value.EC = ElementCount::get(Vals[0], Vals[1]);
+    return true;
+  }
   case HK_UNROLL:
-    return isPowerOf2_32(Val) && Val <= MaxInterleaveFactor;
+    if (isPowerOf2_32(Vals[0]) && Vals[0] <= MaxInterleaveFactor) {
+      Value.U32 = Vals[0];
+      return true;
+    }
+    break;
   case HK_FORCE:
-    return (Val <= 1);
+    if (Vals[0] <= 1) {
+      Value.U32 = Vals[0];
+      return true;
+    }
+    break;
   case HK_ISVECTORIZED:
   case HK_PREDICATE:
-    return (Val == 0 || Val == 1);
+    if (Vals[0] == 0 || Vals[0] == 1) {
+      Value.U32 = Vals[0];
+      return true;
+    }
+    break;
   }
   return false;
 }
 
 LoopVectorizeHints::LoopVectorizeHints(const Loop *L,
                                        bool InterleaveOnlyWhenForced,
                                        OptimizationRemarkEmitter &ORE)
-    : Width("vectorize.width", VectorizerParams::VectorizationFactor, HK_WIDTH),
+    : Width("vectorize.width",
+            ElementCount::getFixed(VectorizerParams::VectorizationFactor)),
       Interleave("interleave.count", InterleaveOnlyWhenForced, HK_UNROLL),
       Force("vectorize.enable", FK_Undefined, HK_FORCE),
       IsVectorized("isvectorized", 0, HK_ISVECTORIZED),
-      Predicate("vectorize.predicate.enable", FK_Undefined, HK_PREDICATE), TheLoop(L),
-      ORE(ORE) {
+      Predicate("vectorize.predicate.enable", FK_Undefined, HK_PREDICATE),
+      TheLoop(L), ORE(ORE) {
   // Populate values with existing loop metadata.
   getHintsFromMetadata();
 
   // force-vector-interleave overrides DisableInterleaving.
   if (VectorizerParams::isInterleaveForced())
-    Interleave.Value = VectorizerParams::VectorizationInterleave;
+    Interleave.Value.U32 = VectorizerParams::VectorizationInterleave;
 
-  if (IsVectorized.Value != 1)
+  if (IsVectorized.Value.U32 != 1)
     // If the vectorization width and interleaving count are both 1 then
     // consider the loop to have been already vectorized because there's
     // nothing more that we can do.
-    IsVectorized.Value = Width.Value == 1 && Interleave.Value == 1;
-  LLVM_DEBUG(if (InterleaveOnlyWhenForced && Interleave.Value == 1) dbgs()
+    IsVectorized.Value.U32 = Width.Value.EC == 1 && Interleave.Value.U32 == 1;
+  LLVM_DEBUG(if (InterleaveOnlyWhenForced && Interleave.Value.U32 == 1) dbgs()
              << "LV: Interleaving disabled by the pass manager\n");
 }
 
 void LoopVectorizeHints::setAlreadyVectorized() {
   LLVMContext &Context = TheLoop->getHeader()->getContext();
 
   MDNode *IsVectorizedMD = MDNode::get(
       Context,
       {MDString::get(Context, "llvm.loop.isvectorized"),
        ConstantAsMetadata::get(ConstantInt::get(Context, APInt(32, 1)))});
   MDNode *LoopID = TheLoop->getLoopID();
   MDNode *NewLoopID =
       makePostTransformationMetadata(Context, LoopID,
                                      {Twine(Prefix(), "vectorize.").str(),
                                       Twine(Prefix(), "interleave.").str()},
                                      {IsVectorizedMD});
   TheLoop->setLoopID(NewLoopID);
 
   // Update internal cache.
-  IsVectorized.Value = 1;
+  IsVectorized.Value.U32 = 1;
 }
 
 bool LoopVectorizeHints::allowVectorization(
     Function *F, Loop *L, bool VectorizeOnlyWhenForced) const {
   if (getForce() == LoopVectorizeHints::FK_Disabled) {
     LLVM_DEBUG(dbgs() << "LV: Not vectorizing: #pragma vectorize disable.\n");
     emitRemarkWithHints();
     return false;
bool LoopVectorizeHints::allowVectorization(
 
   return true;
 }
 
 void LoopVectorizeHints::emitRemarkWithHints() const {
   using namespace ore;
 
   ORE.emit([&]() {
-    if (Force.Value == LoopVectorizeHints::FK_Disabled)
+    if (Force.Value.U32 == LoopVectorizeHints::FK_Disabled)
       return OptimizationRemarkMissed(LV_NAME, "MissedExplicitlyDisabled",
                                       TheLoop->getStartLoc(),
                                       TheLoop->getHeader())
              << "loop not vectorized: vectorization is explicitly disabled";
     else {
       OptimizationRemarkMissed R(LV_NAME, "MissedDetails",
                                  TheLoop->getStartLoc(), TheLoop->getHeader());
       R << "loop not vectorized";
-      if (Force.Value == LoopVectorizeHints::FK_Enabled) {
+      if (Force.Value.U32 == LoopVectorizeHints::FK_Enabled) {
         R << " (Force=" << NV("Force", true);
-        if (Width.Value != 0)
-          R << ", Vector Width=" << NV("VectorWidth", Width.Value);
-        if (Interleave.Value != 0)
-          R << ", Interleave Count=" << NV("InterleaveCount", Interleave.Value);
+        if (Width.Value.EC.isNonZero())
+          R << ", Vector Width=" << NV("VectorWidth", Width.Value.EC);
+        if (Interleave.Value.U32 != 0)
+          R << ", Interleave Count="
+            << NV("InterleaveCount", Interleave.Value.U32);
         R << ")";
       }
       return R;
     }
   });
 }
 
 const char *LoopVectorizeHints::vectorizeAnalysisPassName() const {
   if (getWidth() == 1)
     return LV_NAME;
   if (getForce() == LoopVectorizeHints::FK_Disabled)
     return LV_NAME;
-  if (getForce() == LoopVectorizeHints::FK_Undefined && getWidth() == 0)
+  if (getForce() == LoopVectorizeHints::FK_Undefined && getWidth().isZero())
     return LV_NAME;
   return OptimizationRemarkAnalysis::AlwaysPrint;
 }
 
 void LoopVectorizeHints::getHintsFromMetadata() {
   MDNode *LoopID = TheLoop->getLoopID();
   if (!LoopID)
     return;
for (unsigned i = 1, ie = LoopID->getNumOperands(); i < ie; ++i) {
 
     // Check if the hint starts with the loop metadata prefix.
     StringRef Name = S->getString();
     if (Args.size() == 1)
       setHint(Name, Args[0]);
   }
 }
 
-void LoopVectorizeHints::setHint(StringRef Name, Metadata *Arg) {
+void LoopVectorizeHints::setHint(StringRef Name, const Metadata *Arg) {
   if (!Name.startswith(Prefix()))
     return;
   Name = Name.substr(Prefix().size(), StringRef::npos);
 
   const ConstantInt *C = mdconst::dyn_extract<ConstantInt>(Arg);
-  if (!C)
+  if (!C && Name != Width.Name)
     return;
-  unsigned Val = C->getZExtValue();
 
   Hint *Hints[] = {&Width, &Interleave, &Force, &IsVectorized, &Predicate};
   for (auto H : Hints) {
     if (Name == H->Name) {
-      if (H->validate(Val))
-        H->Value = Val;
-      else
+      if (!H->validateAndSet(Arg))
         LLVM_DEBUG(dbgs() << "LV: ignoring invalid hint '" << Name << "'\n");
       break;
     }
   }
 }
 
 bool LoopVectorizationRequirements::doesNotMeet(
     Function *F, Loop *L, const LoopVectorizeHints &Hints) {

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

static bool processLoopInVPlanNativePath(
   LoopVectorizationCostModel CM(SEL, L, PSE, LI, LVL, *TTI, TLI, DB, AC, ORE, F,
                                 &Hints, IAI);
   // Use the planner for outer loop vectorization.
   // TODO: CM is not used at this point inside the planner. Turn CM into an
   // optional argument if we don't need it in the future.
   LoopVectorizationPlanner LVP(L, LI, TLI, TTI, LVL, CM, IAI, PSE);
 
   // Get user vectorization factor.
-  const unsigned UserVF = Hints.getWidth();
+  const ElementCount UserVF = Hints.getWidth();
 
   // Plan how to best vectorize, return the best VF and its cost.
-  const VectorizationFactor VF =
-      LVP.planInVPlanNativePath(ElementCount::getFixed(UserVF));
+  const VectorizationFactor VF = LVP.planInVPlanNativePath(UserVF);
 
   // If we are stress testing VPlan builds, do not attempt to generate vector
   // code. Masked vector code generation support will follow soon.
   // Also, do not attempt to vectorize if no vector code will be produced.
   if (VPlanBuildStressTest || EnableVPlanPredication ||
       VectorizationFactor::Disabled() == VF)
     return false;
 
#endif /* NDEBUG */
   LoopVectorizationCostModel CM(SEL, L, PSE, LI, &LVL, *TTI, TLI, DB, AC, ORE,
                                 F, &Hints, IAI);
   CM.collectValuesToIgnore();
 
   // Use the planner for vectorization.
   LoopVectorizationPlanner LVP(L, LI, TLI, TTI, &LVL, CM, IAI, PSE);
 
   // Get user vectorization factor and interleave count.
-  unsigned UserVF = Hints.getWidth();
+  ElementCount UserVF = Hints.getWidth();
   unsigned UserIC = Hints.getInterleave();
 
   // Plan how to best vectorize, return the best VF and its cost.
-  Optional<VectorizationFactor> MaybeVF =
-      LVP.plan(ElementCount::getFixed(UserVF), UserIC);
+  Optional<VectorizationFactor> MaybeVF = LVP.plan(UserVF, UserIC);
 
   VectorizationFactor VF = VectorizationFactor::Disabled();
   unsigned IC = 1;
 
   if (MaybeVF) {
     VF = *MaybeVF;
     // Select the interleave count.
     IC = CM.selectInterleaveCount(VF.Width, VF.Cost);

llvm/test/Transforms/LoopVectorize/metadata-width.ll

for.body:                                         ; preds = %entry, %for.body
   %lftr.wideiv = trunc i64 %indvars.iv.next to i32
   %exitcond = icmp eq i32 %lftr.wideiv, %n
   br i1 %exitcond, label %for.end, label %for.body, !llvm.loop !0
 
 for.end:                                          ; preds = %for.body, %entry
   ret void
 }
 
+; CHECK-LABEL: @test2(
+; CHECK: store <8 x i32>
+; CHECK: ret void
+define void @test2(i32* nocapture %a, i32 %n) #0 {
+entry:
+  %cmp4 = icmp sgt i32 %n, 0
+  br i1 %cmp4, label %for.body, label %for.end
+
+for.body:                                         ; preds = %entry, %for.body
+  %indvars.iv = phi i64 [ %indvars.iv.next, %for.body ], [ 0, %entry ]
+  %arrayidx = getelementptr inbounds i32, i32* %a, i64 %indvars.iv
+  %0 = trunc i64 %indvars.iv to i32
+  store i32 %0, i32* %arrayidx, align 4
+  %indvars.iv.next = add i64 %indvars.iv, 1
+  %lftr.wideiv = trunc i64 %indvars.iv.next to i32
+  %exitcond = icmp eq i32 %lftr.wideiv, %n
+  br i1 %exitcond, label %for.end, label %for.body, !llvm.loop !2
+
+for.end:                                          ; preds = %for.body, %entry
+  ret void
+}
+
 attributes #0 = { nounwind uwtable "less-precise-fpmad"="false" "frame-pointer"="none" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "unsafe-fp-math"="false" "use-soft-float"="false" }
 
 !0 = !{!0, !1}
 !1 = !{!"llvm.loop.vectorize.width", i32 8}
+!2 = !{!2, !3}
+!3 = !{!"llvm.loop.vectorize.width", !4}
+!4 = !{i32 8, i32 0}