[SVE] Fix cast<FixedVectorType> in truncateToMinimalBitwidths

[CarolineConcatto]

Hi @DylanFleming-arm
I believe something wrong happened when you uploaded this patch.
It is not showing LoopVectorize.cpp file.
I can only see Context not available.
Is that happening for you too?

llvm/test/Transforms/LoopVectorize/AArch64/sve-trunc-min-bitwidth.ll

2 ↗

(On Diff #351904)

Remove that

3 ↗

(On Diff #351904)

Can you make that a more generic test using the flag:
-force-target-supports-scalable-vectors
?

5 ↗

(On Diff #351904)

Remove that too.

106 ↗

(On Diff #351904)

I believe you don't need that.

[sdesmalen]

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

3955

This looks like the same code, just different formatting.

Did you diff with the correct commit? It also says that llvm/test/Transforms/LoopVectorize/AArch64/sve-trunc-min-bitwidth.ll was moved, but this file does not exist upstream.

[kmclaughlin]

llvm/test/Transforms/LoopVectorize/scalable-trunc-min-bitwidth.ll

2 ↗

(On Diff #352460)

For most of the other scalable tests added to LoopVectorize we've used -scalable-vectorization=on

4 ↗

(On Diff #352460)

This function doesn't have any attributes, so you can remove the #0

68 ↗

(On Diff #352460)

I think you can change this to %for.body and then remove the for.body.pre block below.

[sdesmalen]

llvm/test/Transforms/LoopVectorize/scalable-trunc-min-bitwidth.ll

9 ↗

(On Diff #354515)

This loop is no longer vectorized, so this test is no longer testing your code.

[DylanFleming-arm]

Fixed issue where loop wasn't correctly vectorized with scalable vectors

[sdesmalen]

Just a few more nits on the test.

llvm/test/Transforms/LoopVectorize/scalable-trunc-min-bitwidth.ll

4 ↗

(On Diff #355831)

%dptr is not a pointer, but rather the number of loop iterations, e.g. %N.

4 ↗

(On Diff #355831)

sptr is also not a pointer, but rather some store value, e.g. %val.

4 ↗

(On Diff #355831)

if you make i16* %hptr into i16* noalias %hptr, then you avoid the memory checks because the compiler knows the data pointed to by %hptr doesn't alias with any other memory that's accessed in the function, so it won't have to actively check if there is a dependence between the load/store by adding the compares. See the LangRef.

[DylanFleming-arm]

Updated parameter names to make more sense
Made i16* %hptr into noalias

[sdesmalen]

LGTM

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

for (const auto &KV : Cost->getMinimalBitwidths()) {
     for (unsigned Part = 0; Part < UF; ++Part) {
       Value *I = State.get(Def, Part);
       if (Erased.count(I) || I->use_empty() || !isa<Instruction>(I))
         continue;
       Type *OriginalTy = I->getType();
       Type *ScalarTruncatedTy =
           IntegerType::get(OriginalTy->getContext(), KV.second);
       auto *TruncatedTy = VectorType::get(
-          ScalarTruncatedTy,
-          cast<VectorType>(OriginalTy)->getElementCount());
+          ScalarTruncatedTy, cast<VectorType>(OriginalTy)->getElementCount());
       if (TruncatedTy == OriginalTy)
         continue;
 
       IRBuilder<> B(cast<Instruction>(I));
       auto ShrinkOperand = [&](Value *V) -> Value * {
         if (auto *ZI = dyn_cast<ZExtInst>(V))
           if (ZI->getSrcTy() == TruncatedTy)
             return ZI->getOperand(0);