[llvm] r315927 - Add iterator range MachineRegisterInfo::liveins(), adopt users, NFC
Krzysztof Parzyszek via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 16 14:21:00 PDT 2017
Replaced it with ArrayRef in r315938.
-Krzysztof
On 10/16/2017 4:06 PM, David Blaikie wrote:
> Generally I'd vote for ArrayRef over iterator_range - again, reduced
> proliferation of distinct templates, etc.
>
> But usually I figure returning the specific type is relatively harmless.
> It's more important to use ArrayRef/StringRef/etc on input to functions
> so they're as general as possible.
>
> But I'm open to other ideas here - I get the benefit of not exposing
> implementation details.
>
> On Mon, Oct 16, 2017 at 2:04 PM Craig Topper <craig.topper at gmail.com
> <mailto:craig.topper at gmail.com>> wrote:
>
> Would ArrayRef be better so we're not leaking the fact that its a
> std::vector vs a SmallVector?
>
> ~Craig
>
> On Mon, Oct 16, 2017 at 2:01 PM, David Blaikie via llvm-commits
> <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>>
> wrote:
>
>
>
> On Mon, Oct 16, 2017 at 1:54 PM Krzysztof Parzyszek
> <kparzysz at codeaurora.org <mailto:kparzysz at codeaurora.org>> wrote:
>
> I think that make_range, after inlining should evaluate to
> almost
> nothing, especially when the underlying structure has
> begin/end already.
>
> I'm not sure what mistake we're making with make_range.
> Could you explain?
>
> I guess we can expose the underlying vector, as you're
> saying. One
> advantage of using make_range here is that we only expose
> the begin/end
> without saying what specific container they came from.
>
>
> Yep, though I'm not sure that's too much of a big
> improvement/issue & yeah, inlining probably folds away most of
> anything that matters here - just a thought. Would reduce some
> of the layers of template/compile-time indirection, maybe save
> some compile time and developer understanding about what the
> code's doing, what container it's operating on, etc.
>
> Would you prefer to change liveins to return LiveIns?
>
>
> That's what I'm suggesting/asking, yeah. (a const ref to LiveIns)
>
>
> -Krzysztof
>
>
> On 10/16/2017 3:45 PM, David Blaikie wrote:
> > If LiveIns is already a container, should 'liveins()'
> return a const ref
> > to that container instead? (or is it that we're expecting
> thees kind of
> > ranges to be cheap to copy? That seems like a mistake in
> how we treat
> > ranges if we've already created an assumption around
> cheap-to-copy
> > ranges... might be best to avoid that)
> >
> > Having an iterator_range for cases where there is no
> underlying
> > container (custom iterators iterating over some portion
> of the data
> > structure, filtering, etc) makes sense to me - but I
> don't know that we
> > need it/should use it when there is such a range.
> >
> > On Mon, Oct 16, 2017 at 12:08 PM Krzysztof Parzyszek via
> llvm-commits
> > <llvm-commits at lists.llvm.org
> <mailto:llvm-commits at lists.llvm.org>
> <mailto:llvm-commits at lists.llvm.org
> <mailto:llvm-commits at lists.llvm.org>>> wrote:
> >
> > Author: kparzysz
> > Date: Mon Oct 16 12:08:41 2017
> > New Revision: 315927
> >
> > URL:
> http://llvm.org/viewvc/llvm-project?rev=315927&view=rev
> > Log:
> > Add iterator range MachineRegisterInfo::liveins(),
> adopt users, NFC
> >
> > Modified:
> >
> llvm/trunk/include/llvm/CodeGen/MachineRegisterInfo.h
> > llvm/trunk/lib/CodeGen/MIRPrinter.cpp
> >
> llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
> > llvm/trunk/lib/Target/AMDGPU/R600InstrInfo.cpp
> > llvm/trunk/lib/Target/Hexagon/HexagonBitTracker.cpp
> > llvm/trunk/lib/Target/Hexagon/RDFGraph.cpp
> > llvm/trunk/lib/Target/PowerPC/PPCFrameLowering.cpp
> > llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
> > llvm/trunk/lib/Target/X86/X86VZeroUpper.cpp
> >
> > Modified:
> llvm/trunk/include/llvm/CodeGen/MachineRegisterInfo.h
> > URL:
> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/MachineRegisterInfo.h?rev=315927&r1=315926&r2=315927&view=diff
> >
> ==============================================================================
> > ---
> llvm/trunk/include/llvm/CodeGen/MachineRegisterInfo.h (original)
> > +++
> llvm/trunk/include/llvm/CodeGen/MachineRegisterInfo.h Mon Oct 16
> > 12:08:41 2017
> > @@ -841,6 +841,9 @@ public:
> > livein_iterator livein_begin() const { return
> LiveIns.begin(); }
> > livein_iterator livein_end() const { return
> LiveIns.end(); }
> > bool livein_empty() const { return
> LiveIns.empty(); }
> > + iterator_range<livein_iterator> liveins() const {
> > + return make_range(livein_begin(), livein_end());
> > + }
> >
> > bool isLiveIn(unsigned Reg) const;
> >
> >
> > Modified: llvm/trunk/lib/CodeGen/MIRPrinter.cpp
> > URL:
> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MIRPrinter.cpp?rev=315927&r1=315926&r2=315927&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/CodeGen/MIRPrinter.cpp (original)
> > +++ llvm/trunk/lib/CodeGen/MIRPrinter.cpp Mon Oct 16
> 12:08:41 2017
> > @@ -297,11 +297,11 @@ void
> MIRPrinter::convert(yaml::MachineFu
> > }
> >
> > // Print the live ins.
> > - for (auto I = RegInfo.livein_begin(), E =
> RegInfo.livein_end(); I
> > != E; ++I) {
> > + for (std::pair<unsigned, unsigned> LI :
> RegInfo.liveins()) {
> > yaml::MachineFunctionLiveIn LiveIn;
> > - printReg(I->first, LiveIn.Register, TRI);
> > - if (I->second)
> > - printReg(I->second, LiveIn.VirtualRegister, TRI);
> > + printReg(LI.first, LiveIn.Register, TRI);
> > + if (LI.second)
> > + printReg(LI.second, LiveIn.VirtualRegister, TRI);
> > MF.LiveIns.push_back(LiveIn);
> > }
> >
> >
> > Modified:
> llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
> > URL:
> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp?rev=315927&r1=315926&r2=315927&view=diff
> >
> ==============================================================================
> > ---
> llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
> (original)
> > +++
> llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp Mon Oct
> > 16 12:08:41 2017
> > @@ -494,10 +494,9 @@ bool
> SelectionDAGISel::runOnMachineFunct
> >
> > DenseMap<unsigned, unsigned> LiveInMap;
> > if (!FuncInfo->ArgDbgValues.empty())
> > - for (MachineRegisterInfo::livein_iterator LI =
> > RegInfo->livein_begin(),
> > - E = RegInfo->livein_end(); LI != E; ++LI)
> > - if (LI->second)
> > - LiveInMap.insert(std::make_pair(LI->first,
> LI->second));
> > + for (std::pair<unsigned, unsigned> LI :
> RegInfo->liveins())
> > + if (LI.second)
> > + LiveInMap.insert(LI);
> >
> > // Insert DBG_VALUE instructions for function
> arguments to the
> > entry block.
> > for (unsigned i = 0, e =
> FuncInfo->ArgDbgValues.size(); i != e;
> > ++i) {
> >
> > Modified: llvm/trunk/lib/Target/AMDGPU/R600InstrInfo.cpp
> > URL:
> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AMDGPU/R600InstrInfo.cpp?rev=315927&r1=315926&r2=315927&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/Target/AMDGPU/R600InstrInfo.cpp
> (original)
> > +++ llvm/trunk/lib/Target/AMDGPU/R600InstrInfo.cpp
> Mon Oct 16
> > 12:08:41 2017
> > @@ -1186,10 +1186,8 @@ int
> R600InstrInfo::getIndirectIndexBegin
> > }
> >
> > const TargetRegisterClass *IndirectRC =
> getIndirectAddrRegClass();
> > - for (MachineRegisterInfo::livein_iterator LI =
> MRI.livein_begin(),
> > - LE =
> MRI.livein_end();
> > - LI !=
> LE; ++LI) {
> > - unsigned Reg = LI->first;
> > + for (std::pair<unsigned, unsigned> LI :
> MRI.liveins()) {
> > + unsigned Reg = LI.first;
> > if (TargetRegisterInfo::isVirtualRegister(Reg) ||
> > !IndirectRC->contains(Reg))
> > continue;
> >
> > Modified:
> llvm/trunk/lib/Target/Hexagon/HexagonBitTracker.cpp
> > URL:
> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Hexagon/HexagonBitTracker.cpp?rev=315927&r1=315926&r2=315927&view=diff
> >
> ==============================================================================
> > ---
> llvm/trunk/lib/Target/Hexagon/HexagonBitTracker.cpp (original)
> > +++
> llvm/trunk/lib/Target/Hexagon/HexagonBitTracker.cpp Mon Oct 16
> > 12:08:41 2017
> > @@ -1248,11 +1248,8 @@ unsigned
> HexagonEvaluator::getNextPhysRe
> > }
> >
> > unsigned HexagonEvaluator::getVirtRegFor(unsigned
> PReg) const {
> > - using iterator = MachineRegisterInfo::livein_iterator;
> > -
> > - for (iterator I = MRI.livein_begin(), E =
> MRI.livein_end(); I !=
> > E; ++I) {
> > - if (I->first == PReg)
> > - return I->second;
> > - }
> > + for (std::pair<unsigned,unsigned> P : MRI.liveins())
> > + if (P.first == PReg)
> > + return P.second;
> > return 0;
> > }
> >
> > Modified: llvm/trunk/lib/Target/Hexagon/RDFGraph.cpp
> > URL:
> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Hexagon/RDFGraph.cpp?rev=315927&r1=315926&r2=315927&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/Target/Hexagon/RDFGraph.cpp (original)
> > +++ llvm/trunk/lib/Target/Hexagon/RDFGraph.cpp Mon
> Oct 16 12:08:41 2017
> > @@ -913,8 +913,8 @@ void
> DataFlowGraph::build(unsigned Optio
> > MachineRegisterInfo &MRI = MF.getRegInfo();
> > MachineBasicBlock &EntryB = *EA.Addr->getCode();
> > assert(EntryB.pred_empty() && "Function entry
> block has
> > predecessors");
> > - for (auto I = MRI.livein_begin(), E =
> MRI.livein_end(); I != E; ++I)
> > - LiveIns.insert(RegisterRef(I->first));
> > + for (std::pair<unsigned,unsigned> P : MRI.liveins())
> > + LiveIns.insert(RegisterRef(P.first));
> > if (MRI.tracksLiveness()) {
> > for (auto I : EntryB.liveins())
> > LiveIns.insert(RegisterRef(I.PhysReg,
> I.LaneMask));
> >
> > Modified:
> llvm/trunk/lib/Target/PowerPC/PPCFrameLowering.cpp
> > URL:
> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/PowerPC/PPCFrameLowering.cpp?rev=315927&r1=315926&r2=315927&view=diff
> >
> ==============================================================================
> > ---
> llvm/trunk/lib/Target/PowerPC/PPCFrameLowering.cpp (original)
> > +++
> llvm/trunk/lib/Target/PowerPC/PPCFrameLowering.cpp Mon Oct 16
> > 12:08:41 2017
> > @@ -312,11 +312,9 @@ static void
> HandleVRSaveUpdate(MachineIn
> >
> > // Live in and live out values already must be in
> the mask, so
> > don't bother
> > // marking them.
> > - for (MachineRegisterInfo::livein_iterator
> > - I = MF->getRegInfo().livein_begin(),
> > - E = MF->getRegInfo().livein_end(); I != E; ++I) {
> > - unsigned RegNo = TRI->getEncodingValue(I->first);
> > - if (VRRegNo[RegNo] == I->first) // If
> this really is a
> > vector reg.
> > + for (std::pair<unsigned, unsigned> LI :
> MF->getRegInfo().liveins()) {
> > + unsigned RegNo = TRI->getEncodingValue(LI.first);
> > + if (VRRegNo[RegNo] == LI.first) // If
> this really is a
> > vector reg.
> > UsedRegMask &= ~(1 << (31-RegNo)); //
> Doesn't need to be
> > marked.
> > }
> >
> >
> > Modified: llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
> > URL:
> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=315927&r1=315926&r2=315927&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
> (original)
> > +++ llvm/trunk/lib/Target/X86/X86ISelLowering.cpp Mon
> Oct 16
> > 12:08:41 2017
> > @@ -3235,9 +3235,9 @@ SDValue
> X86TargetLowering::LowerFormalAr
> >
> > if (CallConv == CallingConv::X86_RegCall ||
> >
> Fn->hasFnAttribute("no_caller_saved_registers")) {
> > - const MachineRegisterInfo &MRI = MF.getRegInfo();
> > - for (const auto &Pair :
> make_range(MRI.livein_begin(),
> > MRI.livein_end()))
> > -
> MF.getRegInfo().disableCalleeSavedRegister(Pair.first);
> > + MachineRegisterInfo &MRI = MF.getRegInfo();
> > + for (std::pair<unsigned, unsigned> Pair :
> MRI.liveins())
> > + MRI.disableCalleeSavedRegister(Pair.first);
> > }
> >
> > return Chain;
> >
> > Modified: llvm/trunk/lib/Target/X86/X86VZeroUpper.cpp
> > URL:
> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86VZeroUpper.cpp?rev=315927&r1=315926&r2=315927&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/Target/X86/X86VZeroUpper.cpp
> (original)
> > +++ llvm/trunk/lib/Target/X86/X86VZeroUpper.cpp Mon
> Oct 16 12:08:41 2017
> > @@ -132,9 +132,8 @@ static bool
> isYmmOrZmmReg(unsigned Reg)
> > }
> >
> > static bool
> checkFnHasLiveInYmmOrZmm(MachineRegisterInfo &MRI) {
> > - for (MachineRegisterInfo::livein_iterator I =
> MRI.livein_begin(),
> > - E = MRI.livein_end(); I != E; ++I)
> > - if (isYmmOrZmmReg(I->first))
> > + for (std::pair<unsigned, unsigned> LI : MRI.liveins())
> > + if (isYmmOrZmmReg(LI.first))
> > return true;
> >
> > return false;
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> <mailto:llvm-commits at lists.llvm.org>
> <mailto:llvm-commits at lists.llvm.org
> <mailto:llvm-commits at lists.llvm.org>>
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> >
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora
> Forum,
> hosted by The Linux Foundation
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
More information about the llvm-commits
mailing list