<div dir="ltr"><div>Hi Bruno, </div><div><br></div><div>This patch broke one of our internal tests. I filed a bug at <a href="https://urldefense.proofpoint.com/v2/url?u=https-3A__llvm.org_bugs_show-5Fbug.cgi-3Fid-3D24303&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=1MpiP6GCkw15FZb9ChTwitIOkKv4D0hwJqUCkkPYZhw&s=VdnQOp0U0twzNSVXJeAv0YAd5sdUhw-nJUhwCEGEBBI&e=">https://llvm.org/bugs/show_bug.cgi?id=24303</a>. </div><div><br></div><div>Would you mind me reverting it for now before we investigate the root cause? I am investigating it too. </div><div><br></div><div>Jingyue</div><div><br></div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jul 28, 2015 at 2:45 PM, Bruno Cardoso Lopes <span dir="ltr"><<a href="mailto:bruno.cardoso@gmail.com" target="_blank">bruno.cardoso@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: bruno<br>
Date: Tue Jul 28 16:45:50 2015<br>
New Revision: 243486<br>
<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D243486-26view-3Drev&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=1MpiP6GCkw15FZb9ChTwitIOkKv4D0hwJqUCkkPYZhw&s=xlxi2SHcL0vMUh7pa_bG8lRBmrwpq93vUSsyLzqcatE&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=243486&view=rev</a><br>
Log:<br>
[PeepholeOptimizer] Look through PHIs to find additional register sources<br>
<br>
Reapply 243271 with more fixes; although we are not handling multiple<br>
sources with coalescable copies, we were not properly skipping this<br>
case.<br>
<br>
- Teaches the ValueTracker in the PeepholeOptimizer to look through PHI<br>
instructions.<br>
- Add findNextSourceAndRewritePHI method to lookup into multiple sources<br>
returnted by the ValueTracker and rewrite PHIs with new sources.<br>
<br>
With these changes we can find more register sources and rewrite more<br>
copies to allow coaslescing of bitcast instructions. Hence, we eliminate<br>
unnecessary VR64 <-> GR64 copies in x86, but it could be extended to<br>
other archs by marking "isBitcast" on target specific instructions. The<br>
x86 example follows:<br>
<br>
A:<br>
psllq %mm1, %mm0<br>
movd %mm0, %r9<br>
jmp C<br>
<br>
B:<br>
por %mm1, %mm0<br>
movd %mm0, %r9<br>
jmp C<br>
<br>
C:<br>
movd %r9, %mm0<br>
pshufw $238, %mm0, %mm0<br>
<br>
Becomes:<br>
<br>
A:<br>
psllq %mm1, %mm0<br>
jmp C<br>
<br>
B:<br>
por %mm1, %mm0<br>
jmp C<br>
<br>
C:<br>
pshufw $238, %mm0, %mm0<br>
<br>
Differential Revision: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D11197&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=1MpiP6GCkw15FZb9ChTwitIOkKv4D0hwJqUCkkPYZhw&s=8u_AbiTcjERkap7y6FPeGHY8sIYLcCJLiEY5XFNAXbY&e=" rel="noreferrer" target="_blank">http://reviews.llvm.org/D11197</a><br>
rdar://problem/20404526<br>
<br>
Added:<br>
llvm/trunk/test/CodeGen/X86/mmx-coalescing.ll<br>
Modified:<br>
llvm/trunk/include/llvm/Target/TargetInstrInfo.h<br>
llvm/trunk/lib/CodeGen/PeepholeOptimizer.cpp<br>
llvm/trunk/lib/Target/X86/X86InstrMMX.td<br>
<br>
Modified: llvm/trunk/include/llvm/Target/TargetInstrInfo.h<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_include_llvm_Target_TargetInstrInfo.h-3Frev-3D243486-26r1-3D243485-26r2-3D243486-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=1MpiP6GCkw15FZb9ChTwitIOkKv4D0hwJqUCkkPYZhw&s=8buS_Zz9MAwTimB5F4dLlQqgj3-ZPxpeniTGET7hlBE&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Target/TargetInstrInfo.h?rev=243486&r1=243485&r2=243486&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/Target/TargetInstrInfo.h (original)<br>
+++ llvm/trunk/include/llvm/Target/TargetInstrInfo.h Tue Jul 28 16:45:50 2015<br>
@@ -1266,6 +1266,33 @@ private:<br>
unsigned CallFrameSetupOpcode, CallFrameDestroyOpcode;<br>
};<br>
<br>
+/// \brief Provide DenseMapInfo for TargetInstrInfo::RegSubRegPair.<br>
+template<><br>
+struct DenseMapInfo<TargetInstrInfo::RegSubRegPair> {<br>
+ typedef DenseMapInfo<unsigned> RegInfo;<br>
+<br>
+ static inline TargetInstrInfo::RegSubRegPair getEmptyKey() {<br>
+ return TargetInstrInfo::RegSubRegPair(RegInfo::getEmptyKey(),<br>
+ RegInfo::getEmptyKey());<br>
+ }<br>
+ static inline TargetInstrInfo::RegSubRegPair getTombstoneKey() {<br>
+ return TargetInstrInfo::RegSubRegPair(RegInfo::getTombstoneKey(),<br>
+ RegInfo::getTombstoneKey());<br>
+ }<br>
+ /// \brief Reuse getHashValue implementation from<br>
+ /// std::pair<unsigned, unsigned>.<br>
+ static unsigned getHashValue(const TargetInstrInfo::RegSubRegPair &Val) {<br>
+ std::pair<unsigned, unsigned> PairVal =<br>
+ std::make_pair(Val.Reg, Val.SubReg);<br>
+ return DenseMapInfo<std::pair<unsigned, unsigned>>::getHashValue(PairVal);<br>
+ }<br>
+ static bool isEqual(const TargetInstrInfo::RegSubRegPair &LHS,<br>
+ const TargetInstrInfo::RegSubRegPair &RHS) {<br>
+ return RegInfo::isEqual(LHS.Reg, RHS.Reg) &&<br>
+ RegInfo::isEqual(LHS.SubReg, RHS.SubReg);<br>
+ }<br>
+};<br>
+<br>
} // End llvm namespace<br>
<br>
#endif<br>
<br>
Modified: llvm/trunk/lib/CodeGen/PeepholeOptimizer.cpp<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_CodeGen_PeepholeOptimizer.cpp-3Frev-3D243486-26r1-3D243485-26r2-3D243486-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=1MpiP6GCkw15FZb9ChTwitIOkKv4D0hwJqUCkkPYZhw&s=zYEOUjp9AGSKMaoz328UMv8BKznqstfSDL6YszoALeg&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/PeepholeOptimizer.cpp?rev=243486&r1=243485&r2=243486&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/CodeGen/PeepholeOptimizer.cpp (original)<br>
+++ llvm/trunk/lib/CodeGen/PeepholeOptimizer.cpp Tue Jul 28 16:45:50 2015<br>
@@ -98,6 +98,12 @@ static cl::opt<bool><br>
DisableAdvCopyOpt("disable-adv-copy-opt", cl::Hidden, cl::init(false),<br>
cl::desc("Disable advanced copy optimization"));<br>
<br>
+// Limit the number of PHI instructions to process<br>
+// in PeepholeOptimizer::getNextSource.<br>
+static cl::opt<unsigned> RewritePHILimit(<br>
+ "rewrite-phi-limit", cl::Hidden, cl::init(10),<br>
+ cl::desc("Limit the length of PHI chains to lookup"));<br>
+<br>
STATISTIC(NumReuse, "Number of extension results reused");<br>
STATISTIC(NumCmps, "Number of compares eliminated");<br>
STATISTIC(NumImmFold, "Number of move immediate folded");<br>
@@ -132,6 +138,10 @@ namespace {<br>
}<br>
}<br>
<br>
+ /// \brief Track Def -> Use info used for rewriting copies.<br>
+ typedef SmallDenseMap<TargetInstrInfo::RegSubRegPair, ValueTrackerResult><br>
+ RewriteMapTy;<br>
+<br>
private:<br>
bool optimizeCmpInstr(MachineInstr *MI, MachineBasicBlock *MBB);<br>
bool optimizeExtInstr(MachineInstr *MI, MachineBasicBlock *MBB,<br>
@@ -143,7 +153,8 @@ namespace {<br>
bool optimizeCoalescableCopy(MachineInstr *MI);<br>
bool optimizeUncoalescableCopy(MachineInstr *MI,<br>
SmallPtrSetImpl<MachineInstr *> &LocalMIs);<br>
- bool findNextSource(unsigned &Reg, unsigned &SubReg);<br>
+ bool findNextSource(unsigned Reg, unsigned SubReg,<br>
+ RewriteMapTy &RewriteMap);<br>
bool isMoveImmediate(MachineInstr *MI,<br>
SmallSet<unsigned, 4> &ImmDefRegs,<br>
DenseMap<unsigned, MachineInstr*> &ImmDefMIs);<br>
@@ -220,6 +231,20 @@ namespace {<br>
assert(Idx < getNumSources() && "SubReg source out of index");<br>
return RegSrcs[Idx].SubReg;<br>
}<br>
+<br>
+ bool operator==(const ValueTrackerResult &Other) {<br>
+ if (Other.getInst() != getInst())<br>
+ return false;<br>
+<br>
+ if (Other.getNumSources() != getNumSources())<br>
+ return false;<br>
+<br>
+ for (int i = 0, e = Other.getNumSources(); i != e; ++i)<br>
+ if (Other.getSrcReg(i) != getSrcReg(i) ||<br>
+ Other.getSrcSubReg(i) != getSrcSubReg(i))<br>
+ return false;<br>
+ return true;<br>
+ }<br>
};<br>
<br>
/// \brief Helper class to track the possible sources of a value defined by<br>
@@ -281,6 +306,8 @@ namespace {<br>
/// \brief Specialized version of getNextSource for SubregToReg<br>
/// instructions.<br>
ValueTrackerResult getNextSourceFromSubregToReg();<br>
+ /// \brief Specialized version of getNextSource for PHI instructions.<br>
+ ValueTrackerResult getNextSourceFromPHI();<br>
<br>
public:<br>
/// \brief Create a ValueTracker instance for the value defined by \p Reg.<br>
@@ -583,58 +610,143 @@ static bool shareSameRegisterFile(const<br>
<br>
/// \brief Try to find the next source that share the same register file<br>
/// for the value defined by \p Reg and \p SubReg.<br>
-/// When true is returned, \p Reg and \p SubReg are updated with the<br>
-/// register number and sub-register index of the new source.<br>
+/// When true is returned, the \p RewriteMap can be used by the client to<br>
+/// retrieve all Def -> Use along the way up to the next source. Any found<br>
+/// Use that is not itself a key for another entry, is the next source to<br>
+/// use. During the search for the next source, multiple sources can be found<br>
+/// given multiple incoming sources of a PHI instruction. In this case, we<br>
+/// look in each PHI source for the next source; all found next sources must<br>
+/// share the same register file as \p Reg and \p SubReg. The client should<br>
+/// then be capable to rewrite all intermediate PHIs to get the next source.<br>
/// \return False if no alternative sources are available. True otherwise.<br>
-bool PeepholeOptimizer::findNextSource(unsigned &Reg, unsigned &SubReg) {<br>
+bool PeepholeOptimizer::findNextSource(unsigned Reg, unsigned SubReg,<br>
+ RewriteMapTy &RewriteMap) {<br>
// Do not try to find a new source for a physical register.<br>
// So far we do not have any motivating example for doing that.<br>
// Thus, instead of maintaining untested code, we will revisit that if<br>
// that changes at some point.<br>
if (TargetRegisterInfo::isPhysicalRegister(Reg))<br>
return false;<br>
-<br>
const TargetRegisterClass *DefRC = MRI->getRegClass(Reg);<br>
- unsigned DefSubReg = SubReg;<br>
<br>
- unsigned Src;<br>
- unsigned SrcSubReg;<br>
+ SmallVector<TargetInstrInfo::RegSubRegPair, 4> SrcToLook;<br>
+ TargetInstrInfo::RegSubRegPair CurSrcPair(Reg, SubReg);<br>
+ SrcToLook.push_back(CurSrcPair);<br>
bool ShouldRewrite = false;<br>
<br>
- // Follow the chain of copies until we reach the top of the use-def chain<br>
- // or find a more suitable source.<br>
- ValueTracker ValTracker(Reg, DefSubReg, *MRI, !DisableAdvCopyOpt, TII);<br>
- do {<br>
- ValueTrackerResult Res = ValTracker.getNextSource();<br>
- if (!Res.isValid())<br>
- break;<br>
- Src = Res.getSrcReg(0);<br>
- SrcSubReg = Res.getSrcSubReg(0);<br>
+ unsigned PHILimit = RewritePHILimit;<br>
+ while (!SrcToLook.empty() && PHILimit) {<br>
+ TargetInstrInfo::RegSubRegPair Pair = SrcToLook.pop_back_val();<br>
+ // As explained above, do not handle physical registers<br>
+ if (TargetRegisterInfo::isPhysicalRegister(Pair.Reg))<br>
+ return false;<br>
<br>
- // Do not extend the live-ranges of physical registers as they add<br>
- // constraints to the register allocator.<br>
- // Moreover, if we want to extend the live-range of a physical register,<br>
- // unlike SSA virtual register, we will have to check that they are not<br>
- // redefine before the related use.<br>
- if (TargetRegisterInfo::isPhysicalRegister(Src))<br>
- break;<br>
+ CurSrcPair = Pair;<br>
+ ValueTracker ValTracker(CurSrcPair.Reg, CurSrcPair.SubReg, *MRI,<br>
+ !DisableAdvCopyOpt, TII);<br>
+ ValueTrackerResult Res;<br>
+<br>
+ do {<br>
+ // Follow the chain of copies until we reach the top of the use-def chain<br>
+ // or find a more suitable source.<br>
+ Res = ValTracker.getNextSource();<br>
+ if (!Res.isValid())<br>
+ break;<br>
+<br>
+ // Insert the Def -> Use entry for the recently found source.<br>
+ ValueTrackerResult CurSrcRes = RewriteMap.lookup(CurSrcPair);<br>
+ if (CurSrcRes.isValid()) {<br>
+ assert(CurSrcRes == Res && "ValueTrackerResult found must match");<br>
+ // An existent entry with multiple sources is a PHI cycle we must avoid.<br>
+ // Otherwise it's an entry with a valid next source we already found.<br>
+ if (CurSrcRes.getNumSources() > 1) {<br>
+ DEBUG(dbgs() << "findNextSource: found PHI cycle, aborting...\n");<br>
+ return false;<br>
+ }<br>
+ break;<br>
+ }<br>
+ RewriteMap.insert(std::make_pair(CurSrcPair, Res));<br>
+<br>
+ // ValueTrackerResult usually have one source unless it's the result from<br>
+ // a PHI instruction. Add the found PHI edges to be looked up further.<br>
+ unsigned NumSrcs = Res.getNumSources();<br>
+ if (NumSrcs > 1) {<br>
+ PHILimit--;<br>
+ for (unsigned i = 0; i < NumSrcs; ++i)<br>
+ SrcToLook.push_back(TargetInstrInfo::RegSubRegPair(<br>
+ Res.getSrcReg(i), Res.getSrcSubReg(i)));<br>
+ break;<br>
+ }<br>
+<br>
+ CurSrcPair.Reg = Res.getSrcReg(0);<br>
+ CurSrcPair.SubReg = Res.getSrcSubReg(0);<br>
+ // Do not extend the live-ranges of physical registers as they add<br>
+ // constraints to the register allocator. Moreover, if we want to extend<br>
+ // the live-range of a physical register, unlike SSA virtual register,<br>
+ // we will have to check that they aren't redefine before the related use.<br>
+ if (TargetRegisterInfo::isPhysicalRegister(CurSrcPair.Reg))<br>
+ return false;<br>
+<br>
+ const TargetRegisterClass *SrcRC = MRI->getRegClass(CurSrcPair.Reg);<br>
+<br>
+ // If this source does not incur a cross register bank copy, use it.<br>
+ ShouldRewrite = shareSameRegisterFile(*TRI, DefRC, SubReg, SrcRC,<br>
+ CurSrcPair.SubReg);<br>
+ } while (!ShouldRewrite);<br>
<br>
- const TargetRegisterClass *SrcRC = MRI->getRegClass(Src);<br>
+ // Continue looking for new sources...<br>
+ if (Res.isValid())<br>
+ continue;<br>
<br>
- // If this source does not incur a cross register bank copy, use it.<br>
- ShouldRewrite = shareSameRegisterFile(*TRI, DefRC, DefSubReg, SrcRC,<br>
- SrcSubReg);<br>
- } while (!ShouldRewrite);<br>
+ if (!PHILimit) {<br>
+ DEBUG(dbgs() << "findNextSource: PHI limit reached\n");<br>
+ return false;<br>
+ }<br>
+<br>
+ // Do not continue searching for a new source if the there's at least<br>
+ // one use-def which cannot be rewritten.<br>
+ if (!ShouldRewrite)<br>
+ return false;<br>
+ }<br>
<br>
// If we did not find a more suitable source, there is nothing to optimize.<br>
- if (!ShouldRewrite || Src == Reg)<br>
+ if (CurSrcPair.Reg == Reg)<br>
return false;<br>
<br>
- Reg = Src;<br>
- SubReg = SrcSubReg;<br>
return true;<br>
}<br>
<br>
+/// \brief Insert a PHI instruction with incoming edges \p SrcRegs that are<br>
+/// guaranteed to have the same register class. This is necessary whenever we<br>
+/// successfully traverse a PHI instruction and find suitable sources coming<br>
+/// from its edges. By inserting a new PHI, we provide a rewritten PHI def<br>
+/// suitable to be used in a new COPY instruction.<br>
+MachineInstr *<br>
+insertPHI(MachineRegisterInfo *MRI, const TargetInstrInfo *TII,<br>
+ const SmallVectorImpl<TargetInstrInfo::RegSubRegPair> &SrcRegs,<br>
+ MachineInstr *OrigPHI) {<br>
+ assert(!SrcRegs.empty() && "No sources to create a PHI instruction?");<br>
+<br>
+ const TargetRegisterClass *NewRC = MRI->getRegClass(SrcRegs[0].Reg);<br>
+ unsigned NewVR = MRI->createVirtualRegister(NewRC);<br>
+ MachineBasicBlock *MBB = OrigPHI->getParent();<br>
+ MachineInstrBuilder MIB = BuildMI(*MBB, OrigPHI, OrigPHI->getDebugLoc(),<br>
+ TII->get(TargetOpcode::PHI), NewVR);<br>
+<br>
+ unsigned MBBOpIdx = 2;<br>
+ for (auto RegPair : SrcRegs) {<br>
+ MIB.addReg(RegPair.Reg, 0, RegPair.SubReg);<br>
+ MIB.addMBB(OrigPHI->getOperand(MBBOpIdx).getMBB());<br>
+ // Since we're extended the lifetime of RegPair.Reg, clear the<br>
+ // kill flags to account for that and make RegPair.Reg reaches<br>
+ // the new PHI.<br>
+ MRI->clearKillFlags(RegPair.Reg);<br>
+ MBBOpIdx += 2;<br>
+ }<br>
+<br>
+ return MIB;<br>
+}<br>
+<br>
namespace {<br>
/// \brief Helper class to rewrite the arguments of a copy-like instruction.<br>
class CopyRewriter {<br>
@@ -702,22 +814,78 @@ public:<br>
virtual bool RewriteCurrentSource(unsigned NewReg, unsigned NewSubReg) {<br>
if (!CopyLike.isCopy() || CurrentSrcIdx != 1)<br>
return false;<br>
+ DEBUG(dbgs() << "-- RewriteCurrentSource\n");<br>
+ DEBUG(dbgs() << " Replacing: " << CopyLike);<br>
MachineOperand &MOSrc = CopyLike.getOperand(CurrentSrcIdx);<br>
MOSrc.setReg(NewReg);<br>
MOSrc.setSubReg(NewSubReg);<br>
+ DEBUG(dbgs() << " With: " << CopyLike);<br>
return true;<br>
}<br>
<br>
- /// \brief Rewrite the current source with \p NewSrcReg and \p NewSecSubReg<br>
- /// by creating a new COPY instruction. \p DefReg and \p DefSubReg contain the<br>
- /// definition to be rewritten from the original copylike instruction.<br>
- /// \return The new COPY if the rewriting was possible, nullptr otherwise.<br>
- /// This is needed to handle Uncoalescable copies, since they are copy<br>
- /// like instructions that aren't recognized by the register allocator.<br>
- virtual MachineInstr *RewriteCurrentSource(unsigned DefReg,<br>
- unsigned DefSubReg,<br>
- unsigned NewSrcReg,<br>
- unsigned NewSrcSubReg) {<br>
+ /// \brief Given a \p Def.Reg and Def.SubReg pair, use \p RewriteMap to find<br>
+ /// the new source to use for rewrite. If \p HandleMultipleSources is true and<br>
+ /// multiple sources for a given \p Def are found along the way, we found a<br>
+ /// PHI instructions that needs to be rewritten.<br>
+ /// TODO: HandleMultipleSources should be removed once we test PHI handling<br>
+ /// with coalescable copies.<br>
+ TargetInstrInfo::RegSubRegPair<br>
+ getNewSource(MachineRegisterInfo *MRI, const TargetInstrInfo *TII,<br>
+ TargetInstrInfo::RegSubRegPair Def,<br>
+ PeepholeOptimizer::RewriteMapTy &RewriteMap,<br>
+ bool HandleMultipleSources = true) {<br>
+<br>
+ TargetInstrInfo::RegSubRegPair LookupSrc(Def.Reg, Def.SubReg);<br>
+ do {<br>
+ ValueTrackerResult Res = RewriteMap.lookup(LookupSrc);<br>
+ // If there are no entries on the map, LookupSrc is the new source.<br>
+ if (!Res.isValid())<br>
+ return LookupSrc;<br>
+<br>
+ // There's only one source for this definition, keep searching...<br>
+ unsigned NumSrcs = Res.getNumSources();<br>
+ if (NumSrcs == 1) {<br>
+ LookupSrc.Reg = Res.getSrcReg(0);<br>
+ LookupSrc.SubReg = Res.getSrcSubReg(0);<br>
+ continue;<br>
+ }<br>
+<br>
+ // TODO: remove once multiple srcs w/ coaslescable copies are supported.<br>
+ if (!HandleMultipleSources)<br>
+ break;<br>
+<br>
+ // Multiple sources, recurse into each source to find a new source<br>
+ // for it. Then, rewrite the PHI accordingly to its new edges.<br>
+ SmallVector<TargetInstrInfo::RegSubRegPair, 4> NewPHISrcs;<br>
+ for (unsigned i = 0; i < NumSrcs; ++i) {<br>
+ TargetInstrInfo::RegSubRegPair PHISrc(Res.getSrcReg(i),<br>
+ Res.getSrcSubReg(i));<br>
+ NewPHISrcs.push_back(<br>
+ getNewSource(MRI, TII, PHISrc, RewriteMap, HandleMultipleSources));<br>
+ }<br>
+<br>
+ // Build the new PHI node and return its def register as the new source.<br>
+ MachineInstr *OrigPHI = const_cast<MachineInstr *>(Res.getInst());<br>
+ MachineInstr *NewPHI = insertPHI(MRI, TII, NewPHISrcs, OrigPHI);<br>
+ DEBUG(dbgs() << "-- getNewSource\n");<br>
+ DEBUG(dbgs() << " Replacing: " << *OrigPHI);<br>
+ DEBUG(dbgs() << " With: " << *NewPHI);<br>
+ const MachineOperand &MODef = NewPHI->getOperand(0);<br>
+ return TargetInstrInfo::RegSubRegPair(MODef.getReg(), MODef.getSubReg());<br>
+<br>
+ } while (1);<br>
+<br>
+ return TargetInstrInfo::RegSubRegPair(0, 0);<br>
+ }<br>
+<br>
+ /// \brief Rewrite the source found through \p Def, by using the \p RewriteMap<br>
+ /// and create a new COPY instruction. More info about RewriteMap in<br>
+ /// PeepholeOptimizer::findNextSource. Right now this is only used to handle<br>
+ /// Uncoalescable copies, since they are copy like instructions that aren't<br>
+ /// recognized by the register allocator.<br>
+ virtual MachineInstr *<br>
+ RewriteSource(TargetInstrInfo::RegSubRegPair Def,<br>
+ PeepholeOptimizer::RewriteMapTy &RewriteMap) {<br>
return nullptr;<br>
}<br>
};<br>
@@ -765,31 +933,44 @@ public:<br>
return true;<br>
}<br>
<br>
- /// \brief Rewrite the current source with \p NewSrcReg and \p NewSrcSubReg<br>
- /// by creating a new COPY instruction. \p DefReg and \p DefSubReg contain the<br>
- /// definition to be rewritten from the original copylike instruction.<br>
- /// \return The new COPY if the rewriting was possible, nullptr otherwise.<br>
- MachineInstr *RewriteCurrentSource(unsigned DefReg, unsigned DefSubReg,<br>
- unsigned NewSrcReg,<br>
- unsigned NewSrcSubReg) override {<br>
- assert(!TargetRegisterInfo::isPhysicalRegister(DefReg) &&<br>
+ /// \brief Rewrite the source found through \p Def, by using the \p RewriteMap<br>
+ /// and create a new COPY instruction. More info about RewriteMap in<br>
+ /// PeepholeOptimizer::findNextSource. Right now this is only used to handle<br>
+ /// Uncoalescable copies, since they are copy like instructions that aren't<br>
+ /// recognized by the register allocator.<br>
+ MachineInstr *<br>
+ RewriteSource(TargetInstrInfo::RegSubRegPair Def,<br>
+ PeepholeOptimizer::RewriteMapTy &RewriteMap) override {<br>
+ assert(!TargetRegisterInfo::isPhysicalRegister(Def.Reg) &&<br>
"We do not rewrite physical registers");<br>
<br>
- const TargetRegisterClass *DefRC = MRI.getRegClass(DefReg);<br>
+ // Find the new source to use in the COPY rewrite.<br>
+ TargetInstrInfo::RegSubRegPair NewSrc =<br>
+ getNewSource(&MRI, &TII, Def, RewriteMap);<br>
+<br>
+ // Insert the COPY.<br>
+ const TargetRegisterClass *DefRC = MRI.getRegClass(Def.Reg);<br>
unsigned NewVR = MRI.createVirtualRegister(DefRC);<br>
<br>
MachineInstr *NewCopy =<br>
BuildMI(*CopyLike.getParent(), &CopyLike, CopyLike.getDebugLoc(),<br>
TII.get(TargetOpcode::COPY), NewVR)<br>
- .addReg(NewSrcReg, 0, NewSrcSubReg);<br>
+ .addReg(NewSrc.Reg, 0, NewSrc.SubReg);<br>
<br>
- NewCopy->getOperand(0).setSubReg(DefSubReg);<br>
- if (DefSubReg)<br>
+ NewCopy->getOperand(0).setSubReg(Def.SubReg);<br>
+ if (Def.SubReg)<br>
NewCopy->getOperand(0).setIsUndef();<br>
<br>
- MRI.replaceRegWith(DefReg, NewVR);<br>
+ DEBUG(dbgs() << "-- RewriteSource\n");<br>
+ DEBUG(dbgs() << " Replacing: " << CopyLike);<br>
+ DEBUG(dbgs() << " With: " << *NewCopy);<br>
+ MRI.replaceRegWith(Def.Reg, NewVR);<br>
MRI.clearKillFlags(NewVR);<br>
<br>
+ // We extended the lifetime of NewSrc.Reg, clear the kill flags to<br>
+ // account for that.<br>
+ MRI.clearKillFlags(NewSrc.Reg);<br>
+<br>
return NewCopy;<br>
}<br>
};<br>
@@ -1033,16 +1214,25 @@ bool PeepholeOptimizer::optimizeCoalesca<br>
unsigned SrcReg, SrcSubReg, TrackReg, TrackSubReg;<br>
while (CpyRewriter->getNextRewritableSource(SrcReg, SrcSubReg, TrackReg,<br>
TrackSubReg)) {<br>
- unsigned NewSrc = TrackReg;<br>
- unsigned NewSubReg = TrackSubReg;<br>
+ // Keep track of PHI nodes and its incoming edges when looking for sources.<br>
+ RewriteMapTy RewriteMap;<br>
// Try to find a more suitable source. If we failed to do so, or get the<br>
// actual source, move to the next source.<br>
- if (!findNextSource(NewSrc, NewSubReg) || SrcReg == NewSrc)<br>
+ if (!findNextSource(TrackReg, TrackSubReg, RewriteMap))<br>
continue;<br>
+<br>
+ // Get the new source to rewrite. TODO: Only enable handling of multiple<br>
+ // sources (PHIs) once we have a motivating example and testcases for it.<br>
+ TargetInstrInfo::RegSubRegPair TrackPair(TrackReg, TrackSubReg);<br>
+ TargetInstrInfo::RegSubRegPair NewSrc = CpyRewriter->getNewSource(<br>
+ MRI, TII, TrackPair, RewriteMap, false /* multiple sources */);<br>
+ if (SrcReg == NewSrc.Reg || NewSrc.Reg == 0)<br>
+ continue;<br>
+<br>
// Rewrite source.<br>
- if (CpyRewriter->RewriteCurrentSource(NewSrc, NewSubReg)) {<br>
+ if (CpyRewriter->RewriteCurrentSource(NewSrc.Reg, NewSrc.SubReg)) {<br>
// We may have extended the live-range of NewSrc, account for that.<br>
- MRI->clearKillFlags(NewSrc);<br>
+ MRI->clearKillFlags(NewSrc.Reg);<br>
Changed = true;<br>
}<br>
}<br>
@@ -1071,9 +1261,7 @@ bool PeepholeOptimizer::optimizeUncoales<br>
assert(MI && isUncoalescableCopy(*MI) && "Invalid argument");<br>
<br>
// Check if we can rewrite all the values defined by this instruction.<br>
- SmallVector<<br>
- std::pair<TargetInstrInfo::RegSubRegPair, TargetInstrInfo::RegSubRegPair>,<br>
- 4> RewritePairs;<br>
+ SmallVector<TargetInstrInfo::RegSubRegPair, 4> RewritePairs;<br>
// Get the right rewriter for the current copy.<br>
std::unique_ptr<CopyRewriter> CpyRewriter(getCopyRewriter(*MI, *TII, *MRI));<br>
// If none exists, bails out.<br>
@@ -1083,39 +1271,32 @@ bool PeepholeOptimizer::optimizeUncoales<br>
// Rewrite each rewritable source by generating new COPYs. This works<br>
// differently from optimizeCoalescableCopy since it first makes sure that all<br>
// definitions can be rewritten.<br>
- unsigned SrcReg, SrcSubReg, TrackReg, TrackSubReg;<br>
- while (CpyRewriter->getNextRewritableSource(SrcReg, SrcSubReg, TrackReg,<br>
- TrackSubReg)) {<br>
+ RewriteMapTy RewriteMap;<br>
+ unsigned Reg, SubReg, CopyDefReg, CopyDefSubReg;<br>
+ while (CpyRewriter->getNextRewritableSource(Reg, SubReg, CopyDefReg,<br>
+ CopyDefSubReg)) {<br>
// If a physical register is here, this is probably for a good reason.<br>
// Do not rewrite that.<br>
- if (TargetRegisterInfo::isPhysicalRegister(TrackReg))<br>
+ if (TargetRegisterInfo::isPhysicalRegister(CopyDefReg))<br>
return false;<br>
<br>
// If we do not know how to rewrite this definition, there is no point<br>
// in trying to kill this instruction.<br>
- TargetInstrInfo::RegSubRegPair Def(TrackReg, TrackSubReg);<br>
- TargetInstrInfo::RegSubRegPair Src = Def;<br>
- if (!findNextSource(Src.Reg, Src.SubReg))<br>
+ TargetInstrInfo::RegSubRegPair Def(CopyDefReg, CopyDefSubReg);<br>
+ if (!findNextSource(Def.Reg, Def.SubReg, RewriteMap))<br>
return false;<br>
<br>
- RewritePairs.push_back(std::make_pair(Def, Src));<br>
+ RewritePairs.push_back(Def);<br>
}<br>
<br>
// The change is possible for all defs, do it.<br>
- for (const auto &PairDefSrc : RewritePairs) {<br>
- const auto &Def = PairDefSrc.first;<br>
- const auto &Src = PairDefSrc.second;<br>
-<br>
+ for (const auto &Def : RewritePairs) {<br>
// Rewrite the "copy" in a way the register coalescer understands.<br>
- MachineInstr *NewCopy = CpyRewriter->RewriteCurrentSource(<br>
- Def.Reg, Def.SubReg, Src.Reg, Src.SubReg);<br>
+ MachineInstr *NewCopy = CpyRewriter->RewriteSource(Def, RewriteMap);<br>
assert(NewCopy && "Should be able to always generate a new copy");<br>
-<br>
- // We extended the lifetime of Src and clear the kill flags to<br>
- // account for that.<br>
- MRI->clearKillFlags(Src.Reg);<br>
LocalMIs.insert(NewCopy);<br>
}<br>
+<br>
// MI is now dead.<br>
MI->eraseFromParent();<br>
++NumUncoalescableCopies;<br>
@@ -1523,6 +1704,26 @@ ValueTrackerResult ValueTracker::getNext<br>
Def->getOperand(3).getImm());<br>
}<br>
<br>
+/// \brief Explore each PHI incoming operand and return its sources<br>
+ValueTrackerResult ValueTracker::getNextSourceFromPHI() {<br>
+ assert(Def->isPHI() && "Invalid definition");<br>
+ ValueTrackerResult Res;<br>
+<br>
+ // If we look for a different subreg, bails as we do not<br>
+ // support composing subreg yet.<br>
+ if (Def->getOperand(0).getSubReg() != DefSubReg)<br>
+ return ValueTrackerResult();<br>
+<br>
+ // Return all register sources for PHI instructions.<br>
+ for (unsigned i = 1, e = Def->getNumOperands(); i < e; i += 2) {<br>
+ auto &MO = Def->getOperand(i);<br>
+ assert(MO.isReg() && "Invalid PHI instruction");<br>
+ Res.addSource(MO.getReg(), MO.getSubReg());<br>
+ }<br>
+<br>
+ return Res;<br>
+}<br>
+<br>
ValueTrackerResult ValueTracker::getNextSourceImpl() {<br>
assert(Def && "This method needs a valid definition");<br>
<br>
@@ -1545,6 +1746,8 @@ ValueTrackerResult ValueTracker::getNext<br>
return getNextSourceFromExtractSubreg();<br>
if (Def->isSubregToReg())<br>
return getNextSourceFromSubregToReg();<br>
+ if (Def->isPHI())<br>
+ return getNextSourceFromPHI();<br>
return ValueTrackerResult();<br>
}<br>
<br>
<br>
Modified: llvm/trunk/lib/Target/X86/X86InstrMMX.td<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_Target_X86_X86InstrMMX.td-3Frev-3D243486-26r1-3D243485-26r2-3D243486-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=1MpiP6GCkw15FZb9ChTwitIOkKv4D0hwJqUCkkPYZhw&s=yp6Xl_hMnzrqpP_uIYaPk9FP6oALId-m4ja9lJ70jyI&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86InstrMMX.td?rev=243486&r1=243485&r2=243486&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Target/X86/X86InstrMMX.td (original)<br>
+++ llvm/trunk/lib/Target/X86/X86InstrMMX.td Tue Jul 28 16:45:50 2015<br>
@@ -249,6 +249,7 @@ def MMX_MOVD64grr : MMXI<0x7E, MRMDestRe<br>
(MMX_X86movd2w (x86mmx VR64:$src)))],<br>
IIC_MMX_MOV_REG_MM>, Sched<[WriteMove]>;<br>
<br>
+let isBitcast = 1 in<br>
def MMX_MOVD64to64rr : MMXRI<0x6E, MRMSrcReg, (outs VR64:$dst), (ins GR64:$src),<br>
"movd\t{$src, $dst|$dst, $src}",<br>
[(set VR64:$dst, (bitconvert GR64:$src))],<br>
@@ -262,7 +263,7 @@ def MMX_MOVD64to64rm : MMXRI<0x6E, MRMSr<br>
// These are 64 bit moves, but since the OS X assembler doesn't<br>
// recognize a register-register movq, we write them as<br>
// movd.<br>
-let SchedRW = [WriteMove] in {<br>
+let SchedRW = [WriteMove], isBitcast = 1 in {<br>
def MMX_MOVD64from64rr : MMXRI<0x7E, MRMDestReg,<br>
(outs GR64:$dst), (ins VR64:$src),<br>
"movd\t{$src, $dst|$dst, $src}",<br>
<br>
Added: llvm/trunk/test/CodeGen/X86/mmx-coalescing.ll<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_test_CodeGen_X86_mmx-2Dcoalescing.ll-3Frev-3D243486-26view-3Dauto&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=1MpiP6GCkw15FZb9ChTwitIOkKv4D0hwJqUCkkPYZhw&s=5D8YUxBh5e3LS7V7iz28gzqcHe_EtEq7RSytZt8rFkw&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/mmx-coalescing.ll?rev=243486&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/CodeGen/X86/mmx-coalescing.ll (added)<br>
+++ llvm/trunk/test/CodeGen/X86/mmx-coalescing.ll Tue Jul 28 16:45:50 2015<br>
@@ -0,0 +1,84 @@<br>
+; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+mmx,+sse2 | FileCheck %s<br>
+<br>
+%SA = type <{ %union.anon, i32, [4 x i8], i8*, i8*, i8*, i32, [4 x i8] }><br>
+%union.anon = type { <1 x i64> }<br>
+<br>
+; Check that extra movd (copy) instructions aren't generated.<br>
+<br>
+define i32 @test(%SA* %pSA, i16* %A, i32 %B, i32 %C, i32 %D, i8* %E) {<br>
+entry:<br>
+; CHECK-LABEL: test<br>
+; CHECK: # BB#0:<br>
+; CHECK-NEXT: pshufw<br>
+; CHECK-NEXT: movd<br>
+; CHECK-NOT: movd<br>
+; CHECK-NEXT: testl<br>
+ %shl = shl i32 1, %B<br>
+ %shl1 = shl i32 %C, %B<br>
+ %shl2 = shl i32 1, %D<br>
+ %v = getelementptr inbounds %SA, %SA* %pSA, i64 0, i32 0, i32 0<br>
+ %v0 = load <1 x i64>, <1 x i64>* %v, align 8<br>
+ %SA0 = getelementptr inbounds %SA, %SA* %pSA, i64 0, i32 1<br>
+ %v1 = load i32, i32* %SA0, align 4<br>
+ %SA1 = getelementptr inbounds %SA, %SA* %pSA, i64 0, i32 3<br>
+ %v2 = load i8*, i8** %SA1, align 8<br>
+ %SA2 = getelementptr inbounds %SA, %SA* %pSA, i64 0, i32 4<br>
+ %v3 = load i8*, i8** %SA2, align 8<br>
+ %v4 = bitcast <1 x i64> %v0 to <4 x i16><br>
+ %v5 = bitcast <4 x i16> %v4 to x86_mmx<br>
+ %v6 = tail call x86_mmx @llvm.x86.sse.pshuf.w(x86_mmx %v5, i8 -18)<br>
+ %v7 = bitcast x86_mmx %v6 to <4 x i16><br>
+ %v8 = bitcast <4 x i16> %v7 to <1 x i64><br>
+ %v9 = extractelement <1 x i64> %v8, i32 0<br>
+ %v10 = bitcast i64 %v9 to <2 x i32><br>
+ %v11 = extractelement <2 x i32> %v10, i32 0<br>
+ %cmp = icmp eq i32 %v11, 0<br>
+ br i1 %cmp, label %if.A, label %if.B<br>
+<br>
+if.A:<br>
+; CHECK: %if.A<br>
+; CHECK-NEXT: movd<br>
+; CHECK-NEXT: psllq<br>
+ %pa = phi <1 x i64> [ %v8, %entry ], [ %vx, %if.C ]<br>
+ %v17 = extractelement <1 x i64> %pa, i32 0<br>
+ %v18 = bitcast i64 %v17 to x86_mmx<br>
+ %v19 = tail call x86_mmx @llvm.x86.mmx.pslli.q(x86_mmx %v18, i32 %B) #2<br>
+ %v20 = bitcast x86_mmx %v19 to i64<br>
+ %v21 = insertelement <1 x i64> undef, i64 %v20, i32 0<br>
+ %cmp3 = icmp eq i64 %v20, 0<br>
+ br i1 %cmp3, label %if.C, label %merge<br>
+<br>
+if.B:<br>
+ %v34 = bitcast <1 x i64> %v8 to <4 x i16><br>
+ %v35 = bitcast <4 x i16> %v34 to x86_mmx<br>
+ %v36 = tail call x86_mmx @llvm.x86.sse.pshuf.w(x86_mmx %v35, i8 -18)<br>
+ %v37 = bitcast x86_mmx %v36 to <4 x i16><br>
+ %v38 = bitcast <4 x i16> %v37 to <1 x i64><br>
+ br label %if.C<br>
+<br>
+if.C:<br>
+ %vx = phi <1 x i64> [ %v21, %if.A ], [ %v38, %if.B ]<br>
+ %cvt = bitcast <1 x i64> %vx to <2 x i32><br>
+ %ex = extractelement <2 x i32> %cvt, i32 0<br>
+ %cmp2 = icmp eq i32 %ex, 0<br>
+ br i1 %cmp2, label %if.A, label %merge<br>
+<br>
+merge:<br>
+; CHECK: %merge<br>
+; CHECK-NOT: movd<br>
+; CHECK-NEXT: pshufw<br>
+ %vy = phi <1 x i64> [ %v21, %if.A ], [ %vx, %if.C ]<br>
+ %v130 = bitcast <1 x i64> %vy to <4 x i16><br>
+ %v131 = bitcast <4 x i16> %v130 to x86_mmx<br>
+ %v132 = tail call x86_mmx @llvm.x86.sse.pshuf.w(x86_mmx %v131, i8 -18)<br>
+ %v133 = bitcast x86_mmx %v132 to <4 x i16><br>
+ %v134 = bitcast <4 x i16> %v133 to <1 x i64><br>
+ %v135 = extractelement <1 x i64> %v134, i32 0<br>
+ %v136 = bitcast i64 %v135 to <2 x i32><br>
+ %v137 = extractelement <2 x i32> %v136, i32 0<br>
+ ret i32 %v137<br>
+}<br>
+<br>
+<br>
+declare x86_mmx @llvm.x86.sse.pshuf.w(x86_mmx, i8)<br>
+declare x86_mmx @llvm.x86.mmx.pslli.q(x86_mmx, i32)<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>