[llvm] f26ff8c - [TargetRegisterInfo] Make the heuristic to skip region split overridable by the target
Quentin Colombet via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 3 12:04:59 PST 2020
Hi Wei,
I've refactored the heuristic you added in 40c4aa76 to make it more tweak-able by the backends.
I noticed something odd when doing the refactoring and checked back at the review (https://reviews.llvm.org/D49353) to see how this was discussed back then and whether this was the intended behavior. Basically, I was wondering if `LiveInterval::size` was what you meant to check.
Like Matthias said, this returns the number of segments and AFAIU, you used that as a proxy for live-intervals that span a lot of basic block.
If that’s truly the intent, I believe the current heuristic could be improved because having a lot of segments means that the live-interval covers a lot of basic blocks but the converse is not true: covering a lot of basic blocks doesn’t necessarily means having a lot of segments.
If you want to check for that, instead I would recommend that you use SplitAnalysis::getNumLiveBlocks. Right now I did not put SA in this hook, but we could if we want to go down that road. (I might just do that actually.)
Anyway, just a heads-up in case you wanted to double check.
Cheers,
-Quentin
> On Feb 3, 2020, at 11:30 AM, Quentin Colombet via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>
>
> Author: Quentin Colombet
> Date: 2020-02-03T11:30:35-08:00
> New Revision: f26ff8c9df79a6905903be58864fb27c52374ab8
>
> URL: https://github.com/llvm/llvm-project/commit/f26ff8c9df79a6905903be58864fb27c52374ab8
> DIFF: https://github.com/llvm/llvm-project/commit/f26ff8c9df79a6905903be58864fb27c52374ab8.diff
>
> LOG: [TargetRegisterInfo] Make the heuristic to skip region split overridable by the target
>
> RegAllocGreedy uses a fairly compile time intensive splitting heuristic
> called region splitting. This heuristic was disabled via another heuristic
> when it is likely that it won't be worth the compile time. The only way
> to control this other heuristic was via a command line option (huge-size-for-split).
>
> This commit gives more control on this heuristic by making it overridable
> by the target using a target hook in TargetRegisterInfo called
> shouldRegionSplitForVirtReg.
>
> The default implementation of this hook keeps the heuristic as it was
> before this patch.
>
> Added:
>
>
> Modified:
> llvm/include/llvm/CodeGen/TargetRegisterInfo.h
> llvm/lib/CodeGen/RegAllocGreedy.cpp
> llvm/lib/CodeGen/TargetRegisterInfo.cpp
>
> Removed:
>
>
>
> ################################################################################
> diff --git a/llvm/include/llvm/CodeGen/TargetRegisterInfo.h b/llvm/include/llvm/CodeGen/TargetRegisterInfo.h
> index ae46c2971457..658e3263559a 100644
> --- a/llvm/include/llvm/CodeGen/TargetRegisterInfo.h
> +++ b/llvm/include/llvm/CodeGen/TargetRegisterInfo.h
> @@ -40,6 +40,7 @@ class MachineInstr;
> class RegScavenger;
> class VirtRegMap;
> class LiveIntervals;
> +class LiveInterval;
>
> class TargetRegisterClass {
> public:
> @@ -952,6 +953,12 @@ class TargetRegisterInfo : public MCRegisterInfo {
> LiveIntervals &LIS) const
> { return true; }
>
> + /// Region split has a high compile time cost especially for large live range.
> + /// This method is used to decide whether or not \p VirtReg should
> + /// go through this expensive splitting heuristic.
> + virtual bool shouldRegionSplitForVirtReg(const MachineFunction &MF,
> + const LiveInterval &VirtReg) const;
> +
> //===--------------------------------------------------------------------===//
> /// Debug information queries.
>
>
> diff --git a/llvm/lib/CodeGen/RegAllocGreedy.cpp b/llvm/lib/CodeGen/RegAllocGreedy.cpp
> index 27de7fe45887..9573b91acd6d 100644
> --- a/llvm/lib/CodeGen/RegAllocGreedy.cpp
> +++ b/llvm/lib/CodeGen/RegAllocGreedy.cpp
> @@ -124,12 +124,6 @@ static cl::opt<bool> EnableDeferredSpilling(
> "variable because of other evicted variables."),
> cl::init(false));
>
> -static cl::opt<unsigned>
> - HugeSizeForSplit("huge-size-for-split", cl::Hidden,
> - cl::desc("A threshold of live range size which may cause "
> - "high compile time cost in global splitting."),
> - cl::init(5000));
> -
> // FIXME: Find a good default for this flag and remove the flag.
> static cl::opt<unsigned>
> CSRFirstTimeCost("regalloc-csr-first-time-cost",
> @@ -486,7 +480,6 @@ class RAGreedy : public MachineFunctionPass,
> const SmallVirtRegSet&);
> unsigned tryRegionSplit(LiveInterval&, AllocationOrder&,
> SmallVectorImpl<unsigned>&);
> - unsigned isSplitBenefitWorthCost(LiveInterval &VirtReg);
> /// Calculate cost of region splitting.
> unsigned calculateRegionSplitCost(LiveInterval &VirtReg,
> AllocationOrder &Order,
> @@ -1815,20 +1808,9 @@ void RAGreedy::splitAroundRegion(LiveRangeEdit &LREdit,
> MF->verify(this, "After splitting live range around region");
> }
>
> -// Global split has high compile time cost especially for large live range.
> -// Return false for the case here where the potential benefit will never
> -// worth the cost.
> -unsigned RAGreedy::isSplitBenefitWorthCost(LiveInterval &VirtReg) {
> - MachineInstr *MI = MRI->getUniqueVRegDef(VirtReg.reg);
> - if (MI && TII->isTriviallyReMaterializable(*MI, AA) &&
> - VirtReg.size() > HugeSizeForSplit)
> - return false;
> - return true;
> -}
> -
> unsigned RAGreedy::tryRegionSplit(LiveInterval &VirtReg, AllocationOrder &Order,
> SmallVectorImpl<unsigned> &NewVRegs) {
> - if (!isSplitBenefitWorthCost(VirtReg))
> + if (!TRI->shouldRegionSplitForVirtReg(*MF, VirtReg))
> return 0;
> unsigned NumCands = 0;
> BlockFrequency SpillCost = calcSpillCost();
>
> diff --git a/llvm/lib/CodeGen/TargetRegisterInfo.cpp b/llvm/lib/CodeGen/TargetRegisterInfo.cpp
> index e5592c31098a..1c582ff06560 100644
> --- a/llvm/lib/CodeGen/TargetRegisterInfo.cpp
> +++ b/llvm/lib/CodeGen/TargetRegisterInfo.cpp
> @@ -13,19 +13,22 @@
> #include "llvm/CodeGen/TargetRegisterInfo.h"
> #include "llvm/ADT/ArrayRef.h"
> #include "llvm/ADT/BitVector.h"
> -#include "llvm/ADT/SmallSet.h"
> #include "llvm/ADT/STLExtras.h"
> +#include "llvm/ADT/SmallSet.h"
> #include "llvm/ADT/StringExtras.h"
> #include "llvm/CodeGen/MachineFrameInfo.h"
> #include "llvm/CodeGen/MachineFunction.h"
> #include "llvm/CodeGen/MachineRegisterInfo.h"
> +#include "llvm/CodeGen/LiveInterval.h"
> #include "llvm/CodeGen/TargetFrameLowering.h"
> +#include "llvm/CodeGen/TargetInstrInfo.h"
> #include "llvm/CodeGen/TargetSubtargetInfo.h"
> #include "llvm/CodeGen/VirtRegMap.h"
> #include "llvm/Config/llvm-config.h"
> #include "llvm/IR/Attributes.h"
> #include "llvm/IR/Function.h"
> #include "llvm/MC/MCRegisterInfo.h"
> +#include "llvm/Support/CommandLine.h"
> #include "llvm/Support/Compiler.h"
> #include "llvm/Support/Debug.h"
> #include "llvm/Support/MachineValueType.h"
> @@ -39,6 +42,12 @@
>
> using namespace llvm;
>
> +static cl::opt<unsigned>
> + HugeSizeForSplit("huge-size-for-split", cl::Hidden,
> + cl::desc("A threshold of live range size which may cause "
> + "high compile time cost in global splitting."),
> + cl::init(5000));
> +
> TargetRegisterInfo::TargetRegisterInfo(const TargetRegisterInfoDesc *ID,
> regclass_iterator RCB, regclass_iterator RCE,
> const char *const *SRINames,
> @@ -55,6 +64,17 @@ TargetRegisterInfo::TargetRegisterInfo(const TargetRegisterInfoDesc *ID,
>
> TargetRegisterInfo::~TargetRegisterInfo() = default;
>
> +bool TargetRegisterInfo::shouldRegionSplitForVirtReg(
> + const MachineFunction &MF, const LiveInterval &VirtReg) const {
> + const TargetInstrInfo *TII = MF.getSubtarget().getInstrInfo();
> + const MachineRegisterInfo &MRI = MF.getRegInfo();
> + MachineInstr *MI = MRI.getUniqueVRegDef(VirtReg.reg);
> + if (MI && TII->isTriviallyReMaterializable(*MI) &&
> + VirtReg.size() > HugeSizeForSplit)
> + return false;
> + return true;
> +}
> +
> void TargetRegisterInfo::markSuperRegs(BitVector &RegisterSet, unsigned Reg)
> const {
> for (MCSuperRegIterator AI(Reg, this, true); AI.isValid(); ++AI)
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://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/20200203/df132835/attachment.html>
More information about the llvm-commits
mailing list