I will address this as soon as I can.
<div>
<br />
</div>
<div>PL
<br />
<div>
<br />
</div>
<div>
<br />
</div>
<div id="protonmail_mobile_signature_block">Sent from ProtonMail Mobile</div>
<div>
<br />
<div>
<div>
<br />
</div>On Wed, Apr 4, 2018 at 7:59 PM, Chandler Carruth via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a>> wrote:</div>
<blockquote class="protonmail_quote" type="cite">
<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: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204);">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 << " ";
<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 << " ";);
<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 />- << " ";
<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 << " ";
<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() << " NEW BASIC BLOCK: " << MBB->getName() << " ";);
<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>
</blockquote>
</div>
</div>