[PATCH] D90342: [POC][LoopVectorizer] Propagate ElementCount to interfaces in preparation for scalable auto-vec.
Sander de Smalen via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Nov 3 13:42:01 PST 2020
sdesmalen marked 4 inline comments as done.
sdesmalen added inline comments.
================
Comment at: llvm/include/llvm/Support/TypeSize.h:352
+ LeafTy inc(ScalarTy Amount) const {
+ return static_cast<LeafTy>(
----------------
ctetreau wrote:
> I feel like a function called "inc" should mutate the thing being incremented. Morally, this is behaves like `operator+=`.
>
> All uses currently of this function would be fine with these semantics with some small changes (that should probably be done anyways)
You're right that `inc` suggests the object itself is modified. However, to my opinion that results in some odd looking code, like:
```MaxVF.inc(1); // Implicitly modifies MaxVF
for (ElementCount VF = MinVF; ElementCount::isKnownLT(VF, MaxVF); ) { ... }
```
I've pulled this change out into a separate patch, D90713, where I've added `add` and `sub` methods that return a new object.
================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7846
RSO << "Initial VPlan for VF={" << VF;
- for (VF *= 2; VF.getKnownMinValue() < Range.End; VF *= 2) {
+ assert(VF.isScalable() == Range.End.isScalable() &&
+ "Scalable flags don't match");
----------------
vkmr wrote:
> Do we need this assert here? Wouldn't it be redundant with the assert in the VFRange constructor?
We don't, good spot!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D90342/new/
https://reviews.llvm.org/D90342
More information about the llvm-commits
mailing list