Page MenuHomePhabricator

D15913.id44087.diff
No OneTemporary

File Metadata

Created
Jan 24 2020, 4:26 PM

D15913.id44087.diff

Index: lib/CodeGen/BranchFolding.cpp
===================================================================
--- lib/CodeGen/BranchFolding.cpp
+++ lib/CodeGen/BranchFolding.cpp
@@ -744,18 +744,6 @@
return true;
}
-static bool hasIdenticalMMOs(const MachineInstr *MI1, const MachineInstr *MI2) {
- auto I1 = MI1->memoperands_begin(), E1 = MI1->memoperands_end();
- auto I2 = MI2->memoperands_begin(), E2 = MI2->memoperands_end();
- if ((E1 - I1) != (E2 - I2))
- return false;
- for (; I1 != E1; ++I1, ++I2) {
- if (**I1 != **I2)
- return false;
- }
- return true;
-}
-
static void
removeMMOsFromMemoryOperations(MachineBasicBlock::iterator MBBIStartPos,
MachineBasicBlock &MBBCommon) {
@@ -792,8 +780,7 @@
assert(MBBICommon->isIdenticalTo(&*MBBI) && "Expected matching MIIs!");
if (MBBICommon->mayLoad() || MBBICommon->mayStore())
- if (!hasIdenticalMMOs(&*MBBI, &*MBBICommon))
- MBBICommon->dropMemRefs();
+ MBBICommon->setMemRefs(MBBI->mergeMemRefsWith(*MBBI));
++MBBI;
++MBBICommon;
Index: lib/CodeGen/MachineInstr.cpp
===================================================================
--- lib/CodeGen/MachineInstr.cpp
+++ lib/CodeGen/MachineInstr.cpp
@@ -868,21 +868,39 @@
std::pair<MachineInstr::mmo_iterator, unsigned>
MachineInstr::mergeMemRefsWith(const MachineInstr& Other) {
- // TODO: If we end up with too many memory operands, return the empty
- // conservative set rather than failing asserts.
+ // If either of the incoming memrefs are empty, we must be conservative and
+ // treat this as if we've exhausted our space for memrefs and dropped them.
+ if (memoperands_empty() || Other.memoperands_empty())
+ return std::make_pair(nullptr, 0);
+
+ // If both instructions have identical memrefs, we don't need to merge them.
+ // Since many instructions have a single memref, and we tend to merge things
+ // like pairs of loads from the same location, this catches a large number of
+ // cases in practice.
+ if (NumMemRefs == Other.NumMemRefs &&
+ std::equal(memoperands_begin(), memoperands_end(),
+ Other.memoperands_begin()))
+ return std::make_pair(MemRefs, NumMemRefs);
+
// TODO: consider uniquing elements within the operand lists to reduce
// space usage and fall back to conservative information less often.
size_t CombinedNumMemRefs = (memoperands_end() - memoperands_begin())
+ (Other.memoperands_end() - Other.memoperands_begin());
+ // If we don't have enough room to store this many memrefs, be conservative
+ // and drop them. Otherwise, we'd fail asserts when trying to add them to
+ // the new instruction.
+ if (CombinedNumMemRefs != uint8_t(CombinedNumMemRefs))
+ return std::make_pair(nullptr, 0);
+
MachineFunction *MF = getParent()->getParent();
mmo_iterator MemBegin = MF->allocateMemRefsArray(CombinedNumMemRefs);
mmo_iterator MemEnd = std::copy(memoperands_begin(), memoperands_end(),
MemBegin);
MemEnd = std::copy(Other.memoperands_begin(), Other.memoperands_end(),
MemEnd);
- assert(MemEnd - MemBegin == CombinedNumMemRefs && "missing memrefs");
-
+ assert(MemEnd - MemBegin == (ptrdiff_t)CombinedNumMemRefs &&
+ "missing memrefs");
return std::make_pair(MemBegin, CombinedNumMemRefs);
}

Event Timeline