Skip to content

Commit 0b8528e

Browse files
authored
[instcombine] Adjust style of MemIntrinsic code to be more idiomatic [nfc] (#138715)
Use an existing helper function. Remove the use of a local Changed variable which doesn't seem to interact with surrounding transforms in any meaningful way. (Both memcpy and memmove are MemTransfer instructions, so switching from one to the other doesn't change results.) Posted for review mostly for a sanity check that I'm not missing something with the logic around the Change flag.
1 parent 7087ee6 commit 0b8528e

File tree

1 file changed

+21
-26
lines changed

1 file changed

+21
-26
lines changed

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

Lines changed: 21 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1671,38 +1671,15 @@ Instruction *InstCombinerImpl::visitCallInst(CallInst &CI) {
16711671
// Intrinsics cannot occur in an invoke or a callbr, so handle them here
16721672
// instead of in visitCallBase.
16731673
if (auto *MI = dyn_cast<AnyMemIntrinsic>(II)) {
1674-
bool Changed = false;
1675-
16761674
// memmove/cpy/set of zero bytes is a noop.
16771675
if (Constant *NumBytes = dyn_cast<Constant>(MI->getLength())) {
16781676
if (NumBytes->isNullValue())
16791677
return eraseInstFromFunction(CI);
16801678
}
16811679

16821680
// No other transformations apply to volatile transfers.
1683-
if (auto *M = dyn_cast<MemIntrinsic>(MI))
1684-
if (M->isVolatile())
1685-
return nullptr;
1686-
1687-
// If we have a memmove and the source operation is a constant global,
1688-
// then the source and dest pointers can't alias, so we can change this
1689-
// into a call to memcpy.
1690-
if (auto *MMI = dyn_cast<AnyMemMoveInst>(MI)) {
1691-
if (GlobalVariable *GVSrc = dyn_cast<GlobalVariable>(MMI->getSource()))
1692-
if (GVSrc->isConstant()) {
1693-
Module *M = CI.getModule();
1694-
Intrinsic::ID MemCpyID =
1695-
isa<AtomicMemMoveInst>(MMI)
1696-
? Intrinsic::memcpy_element_unordered_atomic
1697-
: Intrinsic::memcpy;
1698-
Type *Tys[3] = { CI.getArgOperand(0)->getType(),
1699-
CI.getArgOperand(1)->getType(),
1700-
CI.getArgOperand(2)->getType() };
1701-
CI.setCalledFunction(
1702-
Intrinsic::getOrInsertDeclaration(M, MemCpyID, Tys));
1703-
Changed = true;
1704-
}
1705-
}
1681+
if (MI->isVolatile())
1682+
return nullptr;
17061683

17071684
if (AnyMemTransferInst *MTI = dyn_cast<AnyMemTransferInst>(MI)) {
17081685
// memmove(x,x,size) -> noop.
@@ -1734,7 +1711,25 @@ Instruction *InstCombinerImpl::visitCallInst(CallInst &CI) {
17341711
return eraseInstFromFunction(CI);
17351712
}
17361713

1737-
if (Changed) return II;
1714+
// If we have a memmove and the source operation is a constant global,
1715+
// then the source and dest pointers can't alias, so we can change this
1716+
// into a call to memcpy.
1717+
if (auto *MMI = dyn_cast<AnyMemMoveInst>(MI)) {
1718+
if (GlobalVariable *GVSrc = dyn_cast<GlobalVariable>(MMI->getSource()))
1719+
if (GVSrc->isConstant()) {
1720+
Module *M = CI.getModule();
1721+
Intrinsic::ID MemCpyID =
1722+
isa<AtomicMemMoveInst>(MMI)
1723+
? Intrinsic::memcpy_element_unordered_atomic
1724+
: Intrinsic::memcpy;
1725+
Type *Tys[3] = { CI.getArgOperand(0)->getType(),
1726+
CI.getArgOperand(1)->getType(),
1727+
CI.getArgOperand(2)->getType() };
1728+
CI.setCalledFunction(
1729+
Intrinsic::getOrInsertDeclaration(M, MemCpyID, Tys));
1730+
return II;
1731+
}
1732+
}
17381733
}
17391734

17401735
// For fixed width vector result intrinsics, use the generic demanded vector

0 commit comments

Comments
 (0)