Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

[LoopVersioningLICM] Only mark pointers with generated checks as noalias #135168

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
May 12, 2025
Merged

[LoopVersioningLICM] Only mark pointers with generated checks as noalias #135168

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8bbcb5d
[LoopVersioningLICM] Only mark pointers with generated checks as noalias
john-brawn-arm Apr 1, 2025
e8f5ee2
clang-format
john-brawn-arm Apr 10, 2025
27cd50e
Use LVer.annotateLoopWithNoAlias and remove setNoAliasToLoop
john-brawn-arm Apr 22, 2025
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
59 changes: 15 additions & 44 deletions 59 llvm/lib/Transforms/Scalar/LoopVersioningLICM.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,6 @@ struct LoopVersioningLICM {
bool legalLoopInstructions();
bool legalLoopMemoryAccesses();
bool isLoopAlreadyVisited();
void setNoAliasToLoop(Loop *VerLoop);
bool instructionSafeForVersioning(Instruction *I);
};

Expand Down Expand Up @@ -344,6 +343,13 @@ bool LoopVersioningLICM::instructionSafeForVersioning(Instruction *I) {
}
LoadAndStoreCounter++;
Value *Ptr = St->getPointerOperand();
// Don't allow stores that we don't have runtime checks for, as we won't be
// able to mark them noalias meaning they would prevent any code motion.
auto &Pointers = LAI->getRuntimePointerChecking()->Pointers;
if (!any_of(Pointers, [&](auto &P) { return P.PointerValue == Ptr; })) {
LLVM_DEBUG(dbgs() << " Found a store without a runtime check.\n");
return false;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this check needed for this patch, or for the future LAA change you have in mind?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it makes sense to do this change in this patch, as it's part of handling the case where only some pointers have had checks generated for them.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, but doesn't LVer.annotateLoopWithNoAlias() automatically take care of this? It should only add annotations to instructions it has RT checks for

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this is not intended as a legality check (maybe it shouldn't be in instructionSafeForVersioning...) but as a profitability check, on the reasoning that missing noalias metadata on a store will prevent LICM of accesses.

But thinking about this more carefully, I'm not sure this heuristic really makes sense. A loop may well have perfectly analyzable stores that will not interfere with LICM of other accesses (say if the store is to a non-escaped alloca).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, this is a profitability check. An example of where we need this is the function gep_loaded_offset_with_store in the added load-from-unknown-address.ll test. Without this check we version the loop, but LICM can't do anything and we end up with two identical copies of the loop. We could possibly have a more precise check here, but I think that's something to look at in the future not in this patch.

// Check loop invariant.
if (SE->isLoopInvariant(SE->getSCEV(Ptr), CurLoop))
InvariantCounter++;
Expand All @@ -361,6 +367,13 @@ bool LoopVersioningLICM::legalLoopInstructions() {
InvariantCounter = 0;
IsReadOnlyLoop = true;
using namespace ore;
// Get LoopAccessInfo from current loop via the proxy.
LAI = &LAIs.getInfo(*CurLoop);
// Check LoopAccessInfo for need of runtime check.
if (LAI->getRuntimePointerChecking()->getChecks().empty()) {
LLVM_DEBUG(dbgs() << " LAA: Runtime check not found !!\n");
return false;
}
// Iterate over loop blocks and instructions of each block and check
// instruction safety.
for (auto *Block : CurLoop->getBlocks())
Expand All @@ -374,13 +387,6 @@ bool LoopVersioningLICM::legalLoopInstructions() {
return false;
}
}
// Get LoopAccessInfo from current loop via the proxy.
LAI = &LAIs.getInfo(*CurLoop);
// Check LoopAccessInfo for need of runtime check.
if (LAI->getRuntimePointerChecking()->getChecks().empty()) {
LLVM_DEBUG(dbgs() << " LAA: Runtime check not found !!\n");
return false;
}
// Number of runtime-checks should be less then RuntimeMemoryCheckThreshold
if (LAI->getNumRuntimePointerChecks() >
VectorizerParams::RuntimeMemoryCheckThreshold) {
Expand Down Expand Up @@ -501,41 +507,6 @@ bool LoopVersioningLICM::isLegalForVersioning() {
return true;
}

/// Update loop with aggressive aliasing assumptions.
/// It marks no-alias to any pairs of memory operations by assuming
/// loop should not have any must-alias memory accesses pairs.
/// During LoopVersioningLICM legality we ignore loops having must
/// aliasing memory accesses.
void LoopVersioningLICM::setNoAliasToLoop(Loop *VerLoop) {
// Get latch terminator instruction.
Instruction *I = VerLoop->getLoopLatch()->getTerminator();
// Create alias scope domain.
MDBuilder MDB(I->getContext());
MDNode *NewDomain = MDB.createAnonymousAliasScopeDomain("LVDomain");
StringRef Name = "LVAliasScope";
MDNode *NewScope = MDB.createAnonymousAliasScope(NewDomain, Name);
SmallVector<Metadata *, 4> Scopes{NewScope}, NoAliases{NewScope};
// Iterate over each instruction of loop.
// set no-alias for all load & store instructions.
for (auto *Block : CurLoop->getBlocks()) {
for (auto &Inst : *Block) {
// Only interested in instruction that may modify or read memory.
if (!Inst.mayReadFromMemory() && !Inst.mayWriteToMemory())
continue;
// Set no-alias for current instruction.
Inst.setMetadata(
LLVMContext::MD_noalias,
MDNode::concatenate(Inst.getMetadata(LLVMContext::MD_noalias),
MDNode::get(Inst.getContext(), NoAliases)));
// set alias-scope for current instruction.
Inst.setMetadata(
LLVMContext::MD_alias_scope,
MDNode::concatenate(Inst.getMetadata(LLVMContext::MD_alias_scope),
MDNode::get(Inst.getContext(), Scopes)));
}
}
}

bool LoopVersioningLICM::run(DominatorTree *DT) {
// Do not do the transformation if disabled by metadata.
if (hasLICMVersioningTransformation(CurLoop) & TM_Disable)
Expand Down Expand Up @@ -563,7 +534,7 @@ bool LoopVersioningLICM::run(DominatorTree *DT) {
addStringMetadataToLoop(LVer.getVersionedLoop(),
"llvm.mem.parallel_loop_access");
// Update version loop with aggressive aliasing assumption.
setNoAliasToLoop(LVer.getVersionedLoop());
LVer.annotateLoopWithNoAlias();
Changed = true;
}
return Changed;
Expand Down
Loading
Loading
Morty Proxy This is a proxified and sanitized view of the page, visit original site.