Skip to content

[LSV] Insert casts to vectorize mismatched types #134436

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
136 changes: 135 additions & 1 deletion llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
#include "llvm/Transforms/Vectorize/LoadStoreVectorizer.h"
#include "llvm/ADT/APInt.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/Bitset.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/MapVector.h"
#include "llvm/ADT/PostOrderIterator.h"
Expand Down Expand Up @@ -324,6 +325,10 @@ class Vectorizer {
Instruction *ChainElem, Instruction *ChainBegin,
const DenseMap<Instruction *, APInt /*OffsetFromLeader*/> &ChainOffsets);

/// Merge equivalence classes if casts could be inserted in one to match
/// the total bitwidth of the instructions.
void insertCastsToMergeClasses(EquivalenceClassMap &EQClasses);

/// Merges the equivalence classes if they have underlying objects that differ
/// by one level of indirection (i.e., one is a getelementptr and the other is
/// the base pointer in that getelementptr).
Expand Down Expand Up @@ -1310,6 +1315,135 @@ std::optional<APInt> Vectorizer::getConstantOffsetSelects(
return std::nullopt;
}

void Vectorizer::insertCastsToMergeClasses(EquivalenceClassMap &EQClasses) {
if (EQClasses.size() < 2)
return;

auto CopyMetaDataFromTo = [&](Instruction *Src, Instruction *Dst) {
SmallVector<std::pair<unsigned, MDNode *>, 4> MD;
Src->getAllMetadata(MD);
for (const auto [ID, Node] : MD) {
Dst->setMetadata(ID, Node);
}
};

// For each class, determine if all instructions are of type int, FP or ptr.
// This information will help us determine the type instructions should be
// casted into.
MapVector<EqClassKey, Bitset<3>> ClassAllTy;
for (const auto &C : EQClasses) {
auto CommonTypeKind = [](Instruction *I) {
if (I->getType()->isIntOrIntVectorTy())
return 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why integers with different bitwidths would get the same class

if (I->getType()->isFPOrFPVectorTy())
return 1;
if (I->getType()->isPtrOrPtrVectorTy())
return 2;
return -1; // Invalid type kind
};

int FirstTypeKind = CommonTypeKind(EQClasses[C.first][0]);
if (FirstTypeKind != -1 && all_of(EQClasses[C.first], [&](Instruction *I) {
return CommonTypeKind(I) == FirstTypeKind;
})) {
ClassAllTy[C.first].set(FirstTypeKind);
}
}

// Loop over all equivalence classes and try to merge them. Keep track of
// classes that are merged into others.
DenseSet<EqClassKey> ClassesToErase;
for (auto EC1 : EQClasses) {
for (auto EC2 : EQClasses) {
// Skip if EC2 was already merged before, EC1 follows EC2 in the
// collection or EC1 is the same as EC2.
if (ClassesToErase.contains(EC2.first) || EC1 <= EC2 ||
EC1.first == EC2.first)
continue;

auto [Ptr1, AS1, TySize1, IsLoad1] = EC1.first;
auto [Ptr2, AS2, TySize2, IsLoad2] = EC2.first;

// Attempt to merge EC2 into EC1. Skip if the pointers, address spaces or
// whether the leader instruction is a load/store are different. Also skip
// if the scalar bitwidth of the first equivalence class is smaller than
// the second one to avoid reconsidering the same equivalence class pair.
if (Ptr1 != Ptr2 || AS1 != AS2 || IsLoad1 != IsLoad2 || TySize1 < TySize2)
continue;

// An All-FP class should only be merged into another All-FP class.
if ((ClassAllTy[EC1.first].test(1) && !ClassAllTy[EC2.first].test(1)) ||
(!ClassAllTy[EC1.first].test(2) && ClassAllTy[EC2.first].test(2)))
continue;

// Ensure all instructions in EC2 can be bitcasted into NewTy.
/// TODO: NewTyBits is needed as stuctured binded variables cannot be
/// captured by a lambda until C++20.
auto NewTyBits = std::get<2>(EC1.first);
if (any_of(EC2.second, [&](Instruction *I) {
return DL.getTypeSizeInBits(getLoadStoreType(I)) != NewTyBits;
}))
continue;

// Create a new type for the equivalence class.
auto &Ctx = EC2.second[0]->getContext();
Type *NewTy = Type::getIntNTy(EC2.second[0]->getContext(), NewTyBits);
if (ClassAllTy[EC1.first].test(1) && ClassAllTy[EC2.first].test(1)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do these magic numbers to test correspond to the result from CommonTypeKind? Introduce an enum for this?

if (NewTyBits == 16)
NewTy = Type::getHalfTy(Ctx);
else if (NewTyBits == 32)
NewTy = Type::getFloatTy(Ctx);
else if (NewTyBits == 64)
NewTy = Type::getDoubleTy(Ctx);
} else if (ClassAllTy[EC1.first].test(2) &&
ClassAllTy[EC2.first].test(2)) {
NewTy = PointerType::get(Ctx, AS2);
}

for (auto *Inst : EC2.second) {
Value *Ptr = getLoadStorePointerOperand(Inst);
Type *OrigTy = Inst->getType();
if (OrigTy == NewTy)
continue;
if (auto *LI = dyn_cast<LoadInst>(Inst)) {
Builder.SetInsertPoint(LI->getIterator());
auto *NewLoad = Builder.CreateLoad(NewTy, Ptr);
auto *Cast = Builder.CreateBitOrPointerCast(
NewLoad, OrigTy, NewLoad->getName() + ".cast");
LI->replaceAllUsesWith(Cast);
CopyMetaDataFromTo(LI, NewLoad);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copyMetadataForLoad

LI->eraseFromParent();
EQClasses[EC1.first].emplace_back(NewLoad);
} else {
auto *SI = cast<StoreInst>(Inst);
Builder.SetInsertPoint(SI->getIterator());
auto *Cast = Builder.CreateBitOrPointerCast(
SI->getValueOperand(), NewTy,
SI->getValueOperand()->getName() + ".cast");
auto *NewStore = Builder.CreateStore(
Cast, getLoadStorePointerOperand(SI), SI->isVolatile());
CopyMetaDataFromTo(SI, NewStore);
SI->eraseFromParent();
EQClasses[EC1.first].emplace_back(NewStore);
}
}

// Sort the instructions in the equivalence class by their order in the
// basic block. This is important to ensure that the instructions are
// vectorized in the correct order.
std::sort(EQClasses[EC1.first].begin(), EQClasses[EC1.first].end(),
[](const Instruction *A, const Instruction *B) {
return A && B && A->comesBefore(B);
});
ClassesToErase.insert(EC2.first);
}
}

// Erase the equivalence classes that were merged into others.
for (auto Key : ClassesToErase)
EQClasses.erase(Key);
}

void Vectorizer::mergeEquivalenceClasses(EquivalenceClassMap &EQClasses) const {
if (EQClasses.size() < 2) // There is nothing to merge.
return;
Expand Down Expand Up @@ -1495,7 +1629,7 @@ Vectorizer::collectEquivalenceClasses(BasicBlock::iterator Begin,
/*IsLoad=*/LI != nullptr}]
.emplace_back(&I);
}

insertCastsToMergeClasses(Ret);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is eagerly mutating the IR before vectorization is performed? Should try to only select a type, and coerce as part of the final vectorization

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After merging equivalence classes, LSV converts them into chains at which point it is too late to introduce cast instructions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it too late to introduce cast instructions at that point?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Theoretically, it may be possible. The necessary changes seem cumbersome to me, however. After collecting classes, LSV extracts chains from each class. These chains are split based on contiguity, alignment and MayAlias instructions, before vectorization. Merging chains after these splits would require careful handling of their instructions as vectorizeChain makes certain assumptions before determining the type of vectorized load/store.

I prefer to insert casts alongside mergeEquivalenceClasses(..) as gatherChains already understands what kinds of chains are handlable by vectorizeChain.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's important to not make pointless IR changes, and only do this if it vectorization will occur

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point, I will postpone the merging of class until vectorization then. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took a deep dive into inserting casts before vectorizeChain.

  • Firstly, all gathered chains are guaranteed to be vectorized except if the chain only has one element.
  • gatherChains is responsible for calculating offsets. The splitChain functions further splits the gathered chains based on the legality of prospective vectorization. Inserting casts after these functions may break the legality of these chains as the offsets may no longer be correct. There are two ways to tackle this challenge:
    • Recompute the offsets and re-run the split chain functions - sounds like too much of an overhead to me.
    • Store chains in a heap-based data structure which can preserve legality, further demanding a lot of bookkeeping in order to replace the splitChain functions - this approach seems quite inscalable for longer chains.

mergeEquivalenceClasses(Ret);
return Ret;
}
Expand Down
Loading