[PATCH] D105283: [VP] Introducing VectorBuilder, the VP intrinsic builder

[Craig Topper via Phabricator via llvm-commits]

craig.topper accepted this revision.
craig.topper added a comment.
This revision is now accepted and ready to land.

LGTM you can address the comments if you want, but I don't want to hold this up anymore.



================
Comment at: llvm/lib/IR/VectorBuilder.cpp:80
+
+    bool maskBeforeVLen =
+        MaskPosOpt.getValueOr(0) <= VLenPosOpt.getValueOr(NumVPParams);
----------------
What if we just called `IntrinParams.resize(NumVPParams);` and then assigned the mask and vl into their slots?


================
Comment at: llvm/unittests/IR/VectorBuilderTest.cpp:130
+
+static bool isLegalConstEVL(Value *Val, unsigned ExpectedEVL) {
+  if (!isa<ConstantInt>(Val))
----------------
Can simplify a little by merging the isa and cast to a single dyn_cast.

```
static bool isLegalConstEVL(Value *Val, unsigned ExpectedEVL) {
  auto *ConstEVL = dyn_cast<ConstantInt>(Val);
  if (!ConstEVL)
    return false;

  // Value check.
  
  if (ConstEVL->getZExtValue() != ExpectedEVL)
    return false;

  // Type check.
  return ConstEVL->getType()->isIntegerTy(32);
}
```

Or go really packed and do.
```
static bool isLegalConstEVL(Value *Val, unsigned ExpectedEVL) {
  auto *ConstEVL = dyn_cast<ConstantInt>(Val);
  return ConstEVL && ConstEVL->getZExtValue() == ExpectedEVL && ConstEVL->getType()->isIntegerTy(32);
}
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105283/new/

https://reviews.llvm.org/D105283