[llvm] r315927 - Add iterator range MachineRegisterInfo::liveins(), adopt users, NFC
David Blaikie via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 16 14:06:41 PDT 2017
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> 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> wrote:
>
>>
>>
>> On Mon, Oct 16, 2017 at 1:54 PM Krzysztof Parzyszek <
>> 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>>
>>> 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>
>>> > 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
>>
> 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/20171016/b6a2ac48/attachment.html>
More information about the llvm-commits
mailing list