<div dir="ltr">This added warnings to the build:<div>lib/CodeGen/MIRCanonicalizerPass.cpp:487:16: error: unused variable 'LastRenameReg' [-Werror,-Wunused-variable]<br></div></div><br><div class="gmail_quote"><div dir="ltr">On Wed, Apr 4, 2018 at 5:30 PM Puyan Lotfi via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: zer0<br>
Date: Wed Apr 4 17:27:15 2018<br>
New Revision: 329246<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=329246&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=329246&view=rev</a><br>
Log:<br>
[MIR-Canon] Improving performance by switching to named vregs.<br>
<br>
No more skipping thounsands of vregs. Much faster running time.<br>
<br>
Modified:<br>
llvm/trunk/lib/CodeGen/MIRCanonicalizerPass.cpp<br>
llvm/trunk/test/CodeGen/MIR/AArch64/mirCanonIdempotent.mir<br>
llvm/trunk/test/CodeGen/MIR/AMDGPU/mir-canon-multi.mir<br>
<br>
Modified: llvm/trunk/lib/CodeGen/MIRCanonicalizerPass.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MIRCanonicalizerPass.cpp?rev=329246&r1=329245&r2=329246&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MIRCanonicalizerPass.cpp?rev=329246&r1=329245&r2=329246&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/CodeGen/MIRCanonicalizerPass.cpp (original)<br>
+++ llvm/trunk/lib/CodeGen/MIRCanonicalizerPass.cpp Wed Apr 4 17:27:15 2018<br>
@@ -115,22 +115,6 @@ static std::vector<MachineBasicBlock *><br>
return RPOList;<br>
}<br>
<br>
-// Set a dummy vreg. We use this vregs register class to generate throw-away<br>
-// vregs that are used to skip vreg numbers so that vreg numbers line up.<br>
-static unsigned GetDummyVReg(const MachineFunction &MF) {<br>
- for (auto &MBB : MF) {<br>
- for (auto &MI : MBB) {<br>
- for (auto &MO : MI.operands()) {<br>
- if (!MO.isReg() || !TargetRegisterInfo::isVirtualRegister(MO.getReg()))<br>
- continue;<br>
- return MO.getReg();<br>
- }<br>
- }<br>
- }<br>
-<br>
- return ~0U;<br>
-}<br>
-<br>
static bool<br>
rescheduleLexographically(std::vector<MachineInstr *> instructions,<br>
MachineBasicBlock *MBB,<br>
@@ -437,33 +421,61 @@ static void doCandidateWalk(std::vector<<br>
}<br>
}<br>
<br>
-// TODO: Work to remove this in the future. One day when we have named vregs<br>
-// we should be able to form the canonical name based on some characteristic<br>
-// we see in that point of the expression tree (like if we were to name based<br>
-// on some sort of value numbering scheme).<br>
-static void SkipVRegs(unsigned &VRegGapIndex, MachineRegisterInfo &MRI,<br>
- const TargetRegisterClass *RC) {<br>
- const unsigned VR_GAP = (++VRegGapIndex * 1000);<br>
+class NamedVRegCursor {<br>
<br>
- DEBUG({<br>
- dbgs() << "Adjusting per-BB VR_GAP for BB" << VRegGapIndex << " to "<br>
- << VR_GAP << "\n";<br>
- });<br>
+ private:<br>
<br>
- unsigned I = MRI.createVirtualRegister(RC);<br>
- const unsigned E = (((I + VR_GAP) / VR_GAP) + 1) * VR_GAP;<br>
- while (I != E) {<br>
- I = MRI.createVirtualRegister(RC);<br>
- }<br>
-}<br>
+ MachineRegisterInfo &MRI;<br>
+ unsigned virtualVRegNumber;<br>
+<br>
+ public:<br>
+<br>
+ NamedVRegCursor(MachineRegisterInfo &MRI): MRI(MRI) {<br>
+ unsigned VRegGapIndex = 0;<br>
+ const unsigned VR_GAP = (++VRegGapIndex * 1000);<br>
+<br>
+ unsigned I = MRI.createIncompleteVirtualRegister();<br>
+ const unsigned E = (((I + VR_GAP) / VR_GAP) + 1) * VR_GAP;<br>
+<br>
+ virtualVRegNumber = E;<br>
+ }<br>
+<br>
+ void SkipVRegs() {<br>
+ unsigned VRegGapIndex = 1;<br>
+ const unsigned VR_GAP = (++VRegGapIndex * 1000);<br>
+<br>
+ unsigned I = virtualVRegNumber;<br>
+ const unsigned E = (((I + VR_GAP) / VR_GAP) + 1) * VR_GAP;<br>
+<br>
+ virtualVRegNumber = E;<br>
+ }<br>
+<br>
+ unsigned getVirtualVReg() const {<br>
+ return virtualVRegNumber;<br>
+ }<br>
+<br>
+ unsigned incrementVirtualVReg(unsigned incr = 1) {<br>
+ virtualVRegNumber += incr;<br>
+ return virtualVRegNumber;<br>
+ }<br>
+<br>
+ unsigned createVirtualRegister(const TargetRegisterClass *RC) {<br>
+ std::string S;<br>
+ raw_string_ostream OS(S);<br>
+ OS << "namedVReg" << (virtualVRegNumber & ~0x80000000);<br>
+ OS.flush();<br>
+ virtualVRegNumber++;<br>
+<br>
+ return MRI.createVirtualRegister(RC, OS.str());<br>
+ }<br>
+};<br>
<br>
static std::map<unsigned, unsigned><br>
GetVRegRenameMap(const std::vector<TypedVReg> &VRegs,<br>
const std::vector<unsigned> &renamedInOtherBB,<br>
MachineRegisterInfo &MRI,<br>
- const TargetRegisterClass *RC) {<br>
+ NamedVRegCursor &NVC) {<br>
std::map<unsigned, unsigned> VRegRenameMap;<br>
- unsigned LastRenameReg = MRI.createVirtualRegister(RC);<br>
bool FirstCandidate = true;<br>
<br>
for (auto &vreg : VRegs) {<br>
@@ -472,7 +484,7 @@ GetVRegRenameMap(const std::vector<Typed<br>
// (especially when comparing SelectionDAG to GlobalISel generated MIR)<br>
// that in the other file we are just getting an incoming vreg that comes<br>
// from a copy from a frame index. So it's safe to skip by one.<br>
- LastRenameReg = MRI.createVirtualRegister(RC);<br>
+ unsigned LastRenameReg = NVC.incrementVirtualVReg();<br>
DEBUG(dbgs() << "Skipping rename for FI " << LastRenameReg << "\n";);<br>
continue;<br>
} else if (vreg.isCandidate()) {<br>
@@ -482,19 +494,13 @@ GetVRegRenameMap(const std::vector<Typed<br>
// same vreg number making it more likely that the canonical walk from the<br>
// candidate insruction. We don't need to skip from the first candidate of<br>
// the BasicBlock because we already skip ahead several vregs for each BB.<br>
- while (LastRenameReg % 10) {<br>
- if (!FirstCandidate) break;<br>
- LastRenameReg = MRI.createVirtualRegister(RC);<br>
-<br>
- DEBUG({<br>
- dbgs() << "Skipping rename for new candidate " << LastRenameReg<br>
- << "\n";<br>
- });<br>
- }<br>
+ unsigned LastRenameReg = NVC.getVirtualVReg();<br>
+ if (FirstCandidate)<br>
+ NVC.incrementVirtualVReg(LastRenameReg % 10);<br>
FirstCandidate = false;<br>
continue;<br>
} else if (!TargetRegisterInfo::isVirtualRegister(vreg.getReg())) {<br>
- LastRenameReg = MRI.createVirtualRegister(RC);<br>
+ unsigned LastRenameReg = NVC.incrementVirtualVReg();<br>
DEBUG({<br>
dbgs() << "Skipping rename for Phys Reg " << LastRenameReg << "\n";<br>
});<br>
@@ -507,8 +513,7 @@ GetVRegRenameMap(const std::vector<Typed<br>
continue;<br>
}<br>
<br>
- auto Rename = MRI.createVirtualRegister(MRI.getRegClass(Reg));<br>
- LastRenameReg = Rename;<br>
+ auto Rename = NVC.createVirtualRegister(MRI.getRegClass(Reg));<br>
<br>
if (VRegRenameMap.find(Reg) == VRegRenameMap.end()) {<br>
DEBUG(dbgs() << "Mapping vreg ";);<br>
@@ -585,7 +590,8 @@ static bool doDefKillClear(MachineBasicB<br>
static bool runOnBasicBlock(MachineBasicBlock *MBB,<br>
std::vector<StringRef> &bbNames,<br>
std::vector<unsigned> &renamedInOtherBB,<br>
- unsigned &basicBlockNum, unsigned &VRegGapIndex) {<br>
+ unsigned &basicBlockNum, unsigned &VRegGapIndex,<br>
+ NamedVRegCursor &NVC) {<br>
<br>
if (CanonicalizeBasicBlockNumber != ~0U) {<br>
if (CanonicalizeBasicBlockNumber != basicBlockNum++)<br>
@@ -610,11 +616,6 @@ static bool runOnBasicBlock(MachineBasic<br>
MachineFunction &MF = *MBB->getParent();<br>
MachineRegisterInfo &MRI = MF.getRegInfo();<br>
<br>
- const unsigned DummyVReg = GetDummyVReg(MF);<br>
- const TargetRegisterClass *DummyRC =<br>
- (DummyVReg == ~0U) ? nullptr : MRI.getRegClass(DummyVReg);<br>
- if (!DummyRC) return false;<br>
-<br>
bbNames.push_back(MBB->getName());<br>
DEBUG(dbgs() << "\n\n NEW BASIC BLOCK: " << MBB->getName() << "\n\n";);<br>
<br>
@@ -677,25 +678,21 @@ static bool runOnBasicBlock(MachineBasic<br>
if (VRegs.size() == 0)<br>
return Changed;<br>
<br>
- // Skip some vregs, so we can recon where we'll land next.<br>
- SkipVRegs(VRegGapIndex, MRI, DummyRC);<br>
-<br>
- auto VRegRenameMap = GetVRegRenameMap(VRegs, renamedInOtherBB, MRI, DummyRC);<br>
+ auto VRegRenameMap = GetVRegRenameMap(VRegs, renamedInOtherBB, MRI, NVC);<br>
Changed |= doVRegRenaming(renamedInOtherBB, VRegRenameMap, MRI);<br>
<br>
// Here we renumber the def vregs for the idempotent instructions from the top<br>
// of the MachineBasicBlock so that they are named in the order that we sorted<br>
// them alphabetically. Eventually we wont need SkipVRegs because we will use<br>
// named vregs instead.<br>
- unsigned gap = 1;<br>
- SkipVRegs(gap, MRI, DummyRC);<br>
+ NVC.SkipVRegs();<br>
<br>
auto MII = MBB->begin();<br>
for (unsigned i = 0; i < IdempotentInstCount && MII != MBB->end(); ++i) {<br>
MachineInstr &MI = *MII++;<br>
Changed = true;<br>
unsigned vRegToRename = MI.getOperand(0).getReg();<br>
- auto Rename = MRI.createVirtualRegister(MRI.getRegClass(vRegToRename));<br>
+ auto Rename = NVC.createVirtualRegister(MRI.getRegClass(vRegToRename));<br>
<br>
std::vector<MachineOperand *> RenameMOs;<br>
for (auto &MO : MRI.reg_operands(vRegToRename)) {<br>
@@ -745,8 +742,10 @@ bool MIRCanonicalizer::runOnMachineFunct<br>
<br>
bool Changed = false;<br>
<br>
+ MachineRegisterInfo &MRI = MF.getRegInfo();<br>
+ NamedVRegCursor NVC(MRI);<br>
for (auto MBB : RPOList)<br>
- Changed |= runOnBasicBlock(MBB, BBNames, RenamedInOtherBB, BBNum, GapIdx);<br>
+ Changed |= runOnBasicBlock(MBB, BBNames, RenamedInOtherBB, BBNum, GapIdx, NVC);<br>
<br>
return Changed;<br>
}<br>
<br>
Modified: llvm/trunk/test/CodeGen/MIR/AArch64/mirCanonIdempotent.mir<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/MIR/AArch64/mirCanonIdempotent.mir?rev=329246&r1=329245&r2=329246&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/MIR/AArch64/mirCanonIdempotent.mir?rev=329246&r1=329245&r2=329246&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/CodeGen/MIR/AArch64/mirCanonIdempotent.mir (original)<br>
+++ llvm/trunk/test/CodeGen/MIR/AArch64/mirCanonIdempotent.mir Wed Apr 4 17:27:15 2018<br>
@@ -1,10 +1,10 @@<br>
# RUN: llc -mtriple=arm64-apple-ios11.0.0 -o - -run-pass mir-canonicalizer %s | FileCheck %s<br>
# These Idempotent instructions are sorted alphabetically (based on after the '=')<br>
-# CHECK: %4353:gpr64 = MOVi64imm 4617315517961601024<br>
-# CHECK: %4354:gpr32 = MOVi32imm 408<br>
-# CHECK: %4355:gpr64all = IMPLICIT_DEF<br>
-# CHECK: %4356:fpr64 = FMOVDi 20<br>
-# CHECK: %4357:fpr64 = FMOVDi 112<br>
+# CHECK: %namedVReg4352:gpr64 = MOVi64imm 4617315517961601024<br>
+# CHECK: %namedVReg4353:gpr32 = MOVi32imm 408<br>
+# CHECK: %namedVReg4354:gpr64all = IMPLICIT_DEF<br>
+# CHECK: %namedVReg4355:fpr64 = FMOVDi 20<br>
+# CHECK: %namedVReg4356:fpr64 = FMOVDi 112<br>
...<br>
---<br>
name: Proc8<br>
<br>
Modified: llvm/trunk/test/CodeGen/MIR/AMDGPU/mir-canon-multi.mir<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/MIR/AMDGPU/mir-canon-multi.mir?rev=329246&r1=329245&r2=329246&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/MIR/AMDGPU/mir-canon-multi.mir?rev=329246&r1=329245&r2=329246&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/CodeGen/MIR/AMDGPU/mir-canon-multi.mir (original)<br>
+++ llvm/trunk/test/CodeGen/MIR/AMDGPU/mir-canon-multi.mir Wed Apr 4 17:27:15 2018<br>
@@ -1,11 +1,11 @@<br>
# RUN: llc -o - -march=amdgcn -run-pass mir-canonicalizer -x mir %s | FileCheck %s<br>
<br>
-# CHECK: %1363:vgpr_32 = COPY %4354<br>
-# CHECK: %1368:vgpr_32 = COPY %4355<br>
-# CHECK: %1369:vgpr_32 = COPY %1372<br>
-# CHECK: %1370:vgpr_32 = COPY %1373<br>
-# CHECK: REG_SEQUENCE %1368, %subreg.sub0, %1363, %subreg.sub1<br>
-# CHECK: REG_SEQUENCE %1368, %subreg.sub0, %1363, %subreg.sub1, %1369, %subreg.sub2, %1370, %subreg.sub3<br>
+# CHECK: %namedVReg1352:vgpr_32 = COPY %namedVReg4353<br>
+# CHECK: %namedVReg1357:vgpr_32 = COPY %namedVReg4354<br>
+# CHECK: %namedVReg1358:vgpr_32 = COPY %namedVReg1361<br>
+# CHECK: %namedVReg1359:vgpr_32 = COPY %namedVReg1362<br>
+# CHECK: REG_SEQUENCE %namedVReg1357, %subreg.sub0, %namedVReg1352, %subreg.sub1<br>
+# CHECK: REG_SEQUENCE %namedVReg1357, %subreg.sub0, %namedVReg1352, %subreg.sub1, %namedVReg1358, %subreg.sub2, %namedVReg1359, %subreg.sub3<br>
<br>
...<br>
---<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>