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