-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[MemCpyOpt] handle memcpy from memset in more cases #140954
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1367,8 +1367,9 @@ bool MemCpyOptPass::processMemSetMemCpyDependence(MemCpyInst *MemCpy, | |
| return true; | ||
| } | ||
|
|
||
| /// Determine whether the instruction has undefined content for the given Size, | ||
| /// either because it was freshly alloca'd or started its lifetime. | ||
| /// Determine whether the pointer V had only undefined content from Def up to | ||
| /// the given Size, either because it was freshly alloca'd or started its | ||
| /// lifetime. | ||
| static bool hasUndefContents(MemorySSA *MSSA, BatchAAResults &AA, Value *V, | ||
| MemoryDef *Def, Value *Size) { | ||
| if (MSSA->isLiveOnEntryDef(Def)) | ||
|
|
@@ -1403,6 +1404,24 @@ static bool hasUndefContents(MemorySSA *MSSA, BatchAAResults &AA, Value *V, | |
| return false; | ||
| } | ||
|
|
||
| static bool coversInputFully(MemorySSA *MSSA, MemCpyInst *MemCpy, | ||
| MemIntrinsic *MemSrc, BatchAAResults &BAA) { | ||
|
||
| // If the memcpy is larger than the previous, but the memory was undef prior | ||
| // to that, we can just ignore the tail. Technically we're only | ||
| // interested in the bytes from 0..MemSrcOffset and | ||
| // MemSrcLength+MemSrcOffset..CopySize here, but as we can't easily represent | ||
| // this location, we use the full 0..CopySize range. | ||
| Value *CopySize = MemCpy->getLength(); | ||
| MemoryLocation MemCpyLoc = MemoryLocation::getForSource(MemCpy); | ||
| MemoryUseOrDef *MemSrcAccess = MSSA->getMemoryAccess(MemSrc); | ||
| MemoryAccess *Clobber = MSSA->getWalker()->getClobberingMemoryAccess( | ||
| MemSrcAccess->getDefiningAccess(), MemCpyLoc, BAA); | ||
| if (auto *MD = dyn_cast<MemoryDef>(Clobber)) | ||
| if (hasUndefContents(MSSA, BAA, MemCpy->getSource(), MD, CopySize)) | ||
| return true; | ||
| return false; | ||
| } | ||
|
|
||
| /// Transform memcpy to memset when its source was just memset. | ||
| /// In other words, turn: | ||
| /// \code | ||
|
|
@@ -1418,51 +1437,52 @@ static bool hasUndefContents(MemorySSA *MSSA, BatchAAResults &AA, Value *V, | |
| bool MemCpyOptPass::performMemCpyToMemSetOptzn(MemCpyInst *MemCpy, | ||
| MemSetInst *MemSet, | ||
| BatchAAResults &BAA) { | ||
| // Make sure that memcpy(..., memset(...), ...), that is we are memsetting and | ||
| // memcpying from the same address. Otherwise it is hard to reason about. | ||
| if (!BAA.isMustAlias(MemSet->getRawDest(), MemCpy->getRawSource())) | ||
dianqk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return false; | ||
|
|
||
| Value *MemSetSize = MemSet->getLength(); | ||
| Value *CopySize = MemCpy->getLength(); | ||
|
|
||
| if (MemSetSize != CopySize) { | ||
| // Make sure the memcpy doesn't read any more than what the memset wrote. | ||
| // Don't worry about sizes larger than i64. | ||
| int64_t MOffset = 0; | ||
dianqk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| const DataLayout &DL = MemCpy->getModule()->getDataLayout(); | ||
| // We can only transforms memcpy's where the dest of one is the source of the | ||
| // other, or the memory transfer has a known offset from the memset. | ||
| if (MemCpy->getSource() != MemSet->getDest()) { | ||
| std::optional<int64_t> Offset = | ||
| MemCpy->getSource()->getPointerOffsetFrom(MemSet->getDest(), DL); | ||
| if (!Offset || *Offset < 0) | ||
| return false; | ||
| MOffset = *Offset; | ||
| } | ||
|
|
||
| // A known memset size is required. | ||
| MaybeAlign MDestAlign = MemCpy->getDestAlign(); | ||
nikic marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| if (MOffset != 0 || MemSetSize != CopySize) { | ||
dianqk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // Make sure the memcpy doesn't read any more than what the memset wrote, | ||
| // other than undef. Don't worry about sizes larger than i64. A known memset | ||
| // size is required. | ||
| auto *CMemSetSize = dyn_cast<ConstantInt>(MemSetSize); | ||
| if (!CMemSetSize) | ||
| return false; | ||
|
|
||
| // A known memcpy size is also required. | ||
| // A known memcpy size is required. | ||
dianqk marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| auto *CCopySize = dyn_cast<ConstantInt>(CopySize); | ||
| if (!CCopySize) | ||
| return false; | ||
| if (CCopySize->getZExtValue() > CMemSetSize->getZExtValue()) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we also reduce the diff here? How about just modifying the comments?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the helper function makes this function more readable, and the helper function should be more generally useful, since it implements a check that other functions in this file should also implement. In particular, I'm wanting to use this in
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In my opinion, making a new function is the more general approach when sharing code or when a piece of code grows significantly in size. --- a/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
+++ b/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
@@ -1790,6 +1790,9 @@ bool MemCpyOptPass::processMemCpy(MemCpyInst *M, BasicBlock::iterator &BBI) {
}
}
}
+ if (truncateUndefs(M, MI)) {
+ return true;
+ }
if (auto *MDep = dyn_cast<MemCpyInst>(MI))
if (processMemCpyMemCpyDependence(M, MDep, BAA))
return true;
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe this helper function should also be used in |
||
| // If the memcpy is larger than the memset, but the memory was undef prior | ||
| // to the memset, we can just ignore the tail. Technically we're only | ||
| // interested in the bytes from MemSetSize..CopySize here, but as we can't | ||
| // easily represent this location, we use the full 0..CopySize range. | ||
| MemoryLocation MemCpyLoc = MemoryLocation::getForSource(MemCpy); | ||
| bool CanReduceSize = false; | ||
| MemoryUseOrDef *MemSetAccess = MSSA->getMemoryAccess(MemSet); | ||
| MemoryAccess *Clobber = MSSA->getWalker()->getClobberingMemoryAccess( | ||
| MemSetAccess->getDefiningAccess(), MemCpyLoc, BAA); | ||
| if (auto *MD = dyn_cast<MemoryDef>(Clobber)) | ||
| if (hasUndefContents(MSSA, BAA, MemCpy->getSource(), MD, CopySize)) | ||
| CanReduceSize = true; | ||
|
|
||
| if (!CanReduceSize) | ||
| if (CCopySize->getZExtValue() + MOffset > CMemSetSize->getZExtValue()) { | ||
| if (!coversInputFully(MSSA, MemCpy, MemSet, BAA)) | ||
| return false; | ||
| CopySize = MemSetSize; | ||
| // Clip the memcpy to the bounds of the memset | ||
| if (MOffset == 0) | ||
| CopySize = MemSetSize; | ||
| else | ||
| CopySize = | ||
| ConstantInt::get(CopySize->getType(), | ||
| CMemSetSize->getZExtValue() <= (uint64_t)MOffset | ||
| ? 0 | ||
| : CMemSetSize->getZExtValue() - MOffset); | ||
| } | ||
| } | ||
|
|
||
| IRBuilder<> Builder(MemCpy); | ||
| Value *MDest = MemCpy->getRawDest(); | ||
| Instruction *NewM = | ||
| Builder.CreateMemSet(MemCpy->getRawDest(), MemSet->getOperand(1), | ||
| CopySize, MemCpy->getDestAlign()); | ||
| Builder.CreateMemSet(MDest, MemSet->getOperand(1), CopySize, MDestAlign); | ||
vtjnash marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| auto *LastDef = cast<MemoryDef>(MSSA->getMemoryAccess(MemCpy)); | ||
| auto *NewAccess = MSSAU->createMemoryAccessAfter(NewM, nullptr, LastDef); | ||
| MSSAU->insertDef(cast<MemoryDef>(NewAccess), /*RenameUses=*/true); | ||
|
|
@@ -1683,7 +1703,7 @@ bool MemCpyOptPass::performStackMoveOptzn(Instruction *Load, Instruction *Store, | |
| I->setMetadata(LLVMContext::MD_tbaa_struct, nullptr); | ||
| } | ||
|
|
||
| LLVM_DEBUG(dbgs() << "Stack Move: Performed staack-move optimization\n"); | ||
| LLVM_DEBUG(dbgs() << "Stack Move: Performed stack-move optimization\n"); | ||
vtjnash marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| NumStackMove++; | ||
| return true; | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.