<div dir="ltr">Would ArrayRef be better so we're not leaking the fact that its a std::vector vs a SmallVector?</div><div class="gmail_extra"><br clear="all"><div><div class="gmail_signature" data-smartmail="gmail_signature">~Craig</div></div>
<br><div class="gmail_quote">On Mon, Oct 16, 2017 at 2:01 PM, David Blaikie via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><br><div class="gmail_quote"><span class=""><div dir="ltr">On Mon, Oct 16, 2017 at 1:54 PM Krzysztof Parzyszek <<a href="mailto:kparzysz@codeaurora.org" target="_blank">kparzysz@codeaurora.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I think that make_range, after inlining should evaluate to almost<br>
nothing, especially when the underlying structure has begin/end already.<br>
<br>
I'm not sure what mistake we're making with make_range. Could you explain?<br>
<br>
I guess we can expose the underlying vector, as you're saying. One<br>
advantage of using make_range here is that we only expose the begin/end<br>
without saying what specific container they came from.<br></blockquote></span><div><br>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.<br> </div><span class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Would you prefer to change liveins to return LiveIns?<br></blockquote></span><div><br>That's what I'm suggesting/asking, yeah. (a const ref to LiveIns)<br> </div><div><div class="h5"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
-Krzysztof<br>
<br>
<br>
On 10/16/2017 3:45 PM, David Blaikie wrote:<br>
> If LiveIns is already a container, should 'liveins()' return a const ref<br>
> to that container instead? (or is it that we're expecting thees kind of<br>
> ranges to be cheap to copy? That seems like a mistake in how we treat<br>
> ranges if we've already created an assumption around cheap-to-copy<br>
> ranges... might be best to avoid that)<br>
><br>
> Having an iterator_range for cases where there is no underlying<br>
> container (custom iterators iterating over some portion of the data<br>
> structure, filtering, etc) makes sense to me - but I don't know that we<br>
> need it/should use it when there is such a range.<br>
><br>
> On Mon, Oct 16, 2017 at 12:08 PM Krzysztof Parzyszek via llvm-commits<br>
> <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a> <mailto:<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.<wbr>llvm.org</a>>> wrote:<br>
><br>
> Author: kparzysz<br>
> Date: Mon Oct 16 12:08:41 2017<br>
> New Revision: 315927<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=315927&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=315927&view=rev</a><br>
> Log:<br>
> Add iterator range MachineRegisterInfo::liveins()<wbr>, adopt users, NFC<br>
><br>
> Modified:<br>
> llvm/trunk/include/llvm/<wbr>CodeGen/MachineRegisterInfo.h<br>
> llvm/trunk/lib/CodeGen/<wbr>MIRPrinter.cpp<br>
> llvm/trunk/lib/CodeGen/<wbr>SelectionDAG/SelectionDAGISel.<wbr>cpp<br>
> llvm/trunk/lib/Target/AMDGPU/<wbr>R600InstrInfo.cpp<br>
> llvm/trunk/lib/Target/Hexagon/<wbr>HexagonBitTracker.cpp<br>
> llvm/trunk/lib/Target/Hexagon/<wbr>RDFGraph.cpp<br>
> llvm/trunk/lib/Target/PowerPC/<wbr>PPCFrameLowering.cpp<br>
> llvm/trunk/lib/Target/X86/<wbr>X86ISelLowering.cpp<br>
> llvm/trunk/lib/Target/X86/<wbr>X86VZeroUpper.cpp<br>
><br>
> Modified: llvm/trunk/include/llvm/<wbr>CodeGen/MachineRegisterInfo.h<br>
> URL:<br>
> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/MachineRegisterInfo.h?rev=315927&r1=315926&r2=315927&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/include/<wbr>llvm/CodeGen/<wbr>MachineRegisterInfo.h?rev=<wbr>315927&r1=315926&r2=315927&<wbr>view=diff</a><br>
> =============================<wbr>==============================<wbr>===================<br>
> --- llvm/trunk/include/llvm/<wbr>CodeGen/MachineRegisterInfo.h (original)<br>
> +++ llvm/trunk/include/llvm/<wbr>CodeGen/MachineRegisterInfo.h Mon Oct 16<br>
> 12:08:41 2017<br>
> @@ -841,6 +841,9 @@ public:<br>
> livein_iterator livein_begin() const { return LiveIns.begin(); }<br>
> livein_iterator livein_end() const { return LiveIns.end(); }<br>
> bool livein_empty() const { return LiveIns.empty(); }<br>
> + iterator_range<livein_<wbr>iterator> liveins() const {<br>
> + return make_range(livein_begin(), livein_end());<br>
> + }<br>
><br>
> bool isLiveIn(unsigned Reg) const;<br>
><br>
><br>
> Modified: llvm/trunk/lib/CodeGen/<wbr>MIRPrinter.cpp<br>
> URL:<br>
> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MIRPrinter.cpp?rev=315927&r1=315926&r2=315927&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/lib/<wbr>CodeGen/MIRPrinter.cpp?rev=<wbr>315927&r1=315926&r2=315927&<wbr>view=diff</a><br>
> =============================<wbr>==============================<wbr>===================<br>
> --- llvm/trunk/lib/CodeGen/<wbr>MIRPrinter.cpp (original)<br>
> +++ llvm/trunk/lib/CodeGen/<wbr>MIRPrinter.cpp Mon Oct 16 12:08:41 2017<br>
> @@ -297,11 +297,11 @@ void MIRPrinter::convert(yaml::<wbr>MachineFu<br>
> }<br>
><br>
> // Print the live ins.<br>
> - for (auto I = RegInfo.livein_begin(), E = RegInfo.livein_end(); I<br>
> != E; ++I) {<br>
> + for (std::pair<unsigned, unsigned> LI : RegInfo.liveins()) {<br>
> yaml::MachineFunctionLiveIn LiveIn;<br>
> - printReg(I->first, LiveIn.Register, TRI);<br>
> - if (I->second)<br>
> - printReg(I->second, LiveIn.VirtualRegister, TRI);<br>
> + printReg(LI.first, LiveIn.Register, TRI);<br>
> + if (LI.second)<br>
> + printReg(LI.second, LiveIn.VirtualRegister, TRI);<br>
> MF.LiveIns.push_back(LiveIn);<br>
> }<br>
><br>
><br>
> Modified: llvm/trunk/lib/CodeGen/<wbr>SelectionDAG/SelectionDAGISel.<wbr>cpp<br>
> URL:<br>
> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp?rev=315927&r1=315926&r2=315927&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/lib/<wbr>CodeGen/SelectionDAG/<wbr>SelectionDAGISel.cpp?rev=<wbr>315927&r1=315926&r2=315927&<wbr>view=diff</a><br>
> =============================<wbr>==============================<wbr>===================<br>
> --- llvm/trunk/lib/CodeGen/<wbr>SelectionDAG/SelectionDAGISel.<wbr>cpp (original)<br>
> +++ llvm/trunk/lib/CodeGen/<wbr>SelectionDAG/SelectionDAGISel.<wbr>cpp Mon Oct<br>
> 16 12:08:41 2017<br>
> @@ -494,10 +494,9 @@ bool SelectionDAGISel::<wbr>runOnMachineFunct<br>
><br>
> DenseMap<unsigned, unsigned> LiveInMap;<br>
> if (!FuncInfo->ArgDbgValues.<wbr>empty())<br>
> - for (MachineRegisterInfo::livein_<wbr>iterator LI =<br>
> RegInfo->livein_begin(),<br>
> - E = RegInfo->livein_end(); LI != E; ++LI)<br>
> - if (LI->second)<br>
> - LiveInMap.insert(std::make_<wbr>pair(LI->first, LI->second));<br>
> + for (std::pair<unsigned, unsigned> LI : RegInfo->liveins())<br>
> + if (LI.second)<br>
> + LiveInMap.insert(LI);<br>
><br>
> // Insert DBG_VALUE instructions for function arguments to the<br>
> entry block.<br>
> for (unsigned i = 0, e = FuncInfo->ArgDbgValues.size(); i != e;<br>
> ++i) {<br>
><br>
> Modified: llvm/trunk/lib/Target/AMDGPU/<wbr>R600InstrInfo.cpp<br>
> URL:<br>
> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AMDGPU/R600InstrInfo.cpp?rev=315927&r1=315926&r2=315927&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/lib/Target/<wbr>AMDGPU/R600InstrInfo.cpp?rev=<wbr>315927&r1=315926&r2=315927&<wbr>view=diff</a><br>
> =============================<wbr>==============================<wbr>===================<br>
> --- llvm/trunk/lib/Target/AMDGPU/<wbr>R600InstrInfo.cpp (original)<br>
> +++ llvm/trunk/lib/Target/AMDGPU/<wbr>R600InstrInfo.cpp Mon Oct 16<br>
> 12:08:41 2017<br>
> @@ -1186,10 +1186,8 @@ int R600InstrInfo::<wbr>getIndirectIndexBegin<br>
> }<br>
><br>
> const TargetRegisterClass *IndirectRC = getIndirectAddrRegClass();<br>
> - for (MachineRegisterInfo::livein_<wbr>iterator LI = MRI.livein_begin(),<br>
> - LE = MRI.livein_end();<br>
> - LI != LE; ++LI) {<br>
> - unsigned Reg = LI->first;<br>
> + for (std::pair<unsigned, unsigned> LI : MRI.liveins()) {<br>
> + unsigned Reg = LI.first;<br>
> if (TargetRegisterInfo::<wbr>isVirtualRegister(Reg) ||<br>
> !IndirectRC->contains(Reg))<br>
> continue;<br>
><br>
> Modified: llvm/trunk/lib/Target/Hexagon/<wbr>HexagonBitTracker.cpp<br>
> URL:<br>
> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Hexagon/HexagonBitTracker.cpp?rev=315927&r1=315926&r2=315927&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/lib/Target/<wbr>Hexagon/HexagonBitTracker.cpp?<wbr>rev=315927&r1=315926&r2=<wbr>315927&view=diff</a><br>
> =============================<wbr>==============================<wbr>===================<br>
> --- llvm/trunk/lib/Target/Hexagon/<wbr>HexagonBitTracker.cpp (original)<br>
> +++ llvm/trunk/lib/Target/Hexagon/<wbr>HexagonBitTracker.cpp Mon Oct 16<br>
> 12:08:41 2017<br>
> @@ -1248,11 +1248,8 @@ unsigned HexagonEvaluator::<wbr>getNextPhysRe<br>
> }<br>
><br>
> unsigned HexagonEvaluator::<wbr>getVirtRegFor(unsigned PReg) const {<br>
> - using iterator = MachineRegisterInfo::livein_<wbr>iterator;<br>
> -<br>
> - for (iterator I = MRI.livein_begin(), E = MRI.livein_end(); I !=<br>
> E; ++I) {<br>
> - if (I->first == PReg)<br>
> - return I->second;<br>
> - }<br>
> + for (std::pair<unsigned,unsigned> P : MRI.liveins())<br>
> + if (P.first == PReg)<br>
> + return P.second;<br>
> return 0;<br>
> }<br>
><br>
> Modified: llvm/trunk/lib/Target/Hexagon/<wbr>RDFGraph.cpp<br>
> URL:<br>
> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Hexagon/RDFGraph.cpp?rev=315927&r1=315926&r2=315927&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/lib/Target/<wbr>Hexagon/RDFGraph.cpp?rev=<wbr>315927&r1=315926&r2=315927&<wbr>view=diff</a><br>
> =============================<wbr>==============================<wbr>===================<br>
> --- llvm/trunk/lib/Target/Hexagon/<wbr>RDFGraph.cpp (original)<br>
> +++ llvm/trunk/lib/Target/Hexagon/<wbr>RDFGraph.cpp Mon Oct 16 12:08:41 2017<br>
> @@ -913,8 +913,8 @@ void DataFlowGraph::build(unsigned Optio<br>
> MachineRegisterInfo &MRI = MF.getRegInfo();<br>
> MachineBasicBlock &EntryB = *EA.Addr->getCode();<br>
> assert(EntryB.pred_empty() && "Function entry block has<br>
> predecessors");<br>
> - for (auto I = MRI.livein_begin(), E = MRI.livein_end(); I != E; ++I)<br>
> - LiveIns.insert(RegisterRef(I-><wbr>first));<br>
> + for (std::pair<unsigned,unsigned> P : MRI.liveins())<br>
> + LiveIns.insert(RegisterRef(P.<wbr>first));<br>
> if (MRI.tracksLiveness()) {<br>
> for (auto I : EntryB.liveins())<br>
> LiveIns.insert(RegisterRef(I.<wbr>PhysReg, I.LaneMask));<br>
><br>
> Modified: llvm/trunk/lib/Target/PowerPC/<wbr>PPCFrameLowering.cpp<br>
> URL:<br>
> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/PowerPC/PPCFrameLowering.cpp?rev=315927&r1=315926&r2=315927&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/lib/Target/<wbr>PowerPC/PPCFrameLowering.cpp?<wbr>rev=315927&r1=315926&r2=<wbr>315927&view=diff</a><br>
> =============================<wbr>==============================<wbr>===================<br>
> --- llvm/trunk/lib/Target/PowerPC/<wbr>PPCFrameLowering.cpp (original)<br>
> +++ llvm/trunk/lib/Target/PowerPC/<wbr>PPCFrameLowering.cpp Mon Oct 16<br>
> 12:08:41 2017<br>
> @@ -312,11 +312,9 @@ static void HandleVRSaveUpdate(MachineIn<br>
><br>
> // Live in and live out values already must be in the mask, so<br>
> don't bother<br>
> // marking them.<br>
> - for (MachineRegisterInfo::livein_<wbr>iterator<br>
> - I = MF->getRegInfo().livein_begin(<wbr>),<br>
> - E = MF->getRegInfo().livein_end(); I != E; ++I) {<br>
> - unsigned RegNo = TRI->getEncodingValue(I-><wbr>first);<br>
> - if (VRRegNo[RegNo] == I->first) // If this really is a<br>
> vector reg.<br>
> + for (std::pair<unsigned, unsigned> LI : MF->getRegInfo().liveins()) {<br>
> + unsigned RegNo = TRI->getEncodingValue(LI.<wbr>first);<br>
> + if (VRRegNo[RegNo] == LI.first) // If this really is a<br>
> vector reg.<br>
> UsedRegMask &= ~(1 << (31-RegNo)); // Doesn't need to be<br>
> marked.<br>
> }<br>
><br>
><br>
> Modified: llvm/trunk/lib/Target/X86/<wbr>X86ISelLowering.cpp<br>
> URL:<br>
> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=315927&r1=315926&r2=315927&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/lib/Target/<wbr>X86/X86ISelLowering.cpp?rev=<wbr>315927&r1=315926&r2=315927&<wbr>view=diff</a><br>
> =============================<wbr>==============================<wbr>===================<br>
> --- llvm/trunk/lib/Target/X86/<wbr>X86ISelLowering.cpp (original)<br>
> +++ llvm/trunk/lib/Target/X86/<wbr>X86ISelLowering.cpp Mon Oct 16<br>
> 12:08:41 2017<br>
> @@ -3235,9 +3235,9 @@ SDValue X86TargetLowering::<wbr>LowerFormalAr<br>
><br>
> if (CallConv == CallingConv::X86_RegCall ||<br>
> Fn->hasFnAttribute("no_<wbr>caller_saved_registers")) {<br>
> - const MachineRegisterInfo &MRI = MF.getRegInfo();<br>
> - for (const auto &Pair : make_range(MRI.livein_begin(),<br>
> MRI.livein_end()))<br>
> - MF.getRegInfo().<wbr>disableCalleeSavedRegister(<wbr>Pair.first);<br>
> + MachineRegisterInfo &MRI = MF.getRegInfo();<br>
> + for (std::pair<unsigned, unsigned> Pair : MRI.liveins())<br>
> + MRI.<wbr>disableCalleeSavedRegister(<wbr>Pair.first);<br>
> }<br>
><br>
> return Chain;<br>
><br>
> Modified: llvm/trunk/lib/Target/X86/<wbr>X86VZeroUpper.cpp<br>
> URL:<br>
> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86VZeroUpper.cpp?rev=315927&r1=315926&r2=315927&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/lib/Target/<wbr>X86/X86VZeroUpper.cpp?rev=<wbr>315927&r1=315926&r2=315927&<wbr>view=diff</a><br>
> =============================<wbr>==============================<wbr>===================<br>
> --- llvm/trunk/lib/Target/X86/<wbr>X86VZeroUpper.cpp (original)<br>
> +++ llvm/trunk/lib/Target/X86/<wbr>X86VZeroUpper.cpp Mon Oct 16 12:08:41 2017<br>
> @@ -132,9 +132,8 @@ static bool isYmmOrZmmReg(unsigned Reg)<br>
> }<br>
><br>
> static bool checkFnHasLiveInYmmOrZmm(<wbr>MachineRegisterInfo &MRI) {<br>
> - for (MachineRegisterInfo::livein_<wbr>iterator I = MRI.livein_begin(),<br>
> - E = MRI.livein_end(); I != E; ++I)<br>
> - if (isYmmOrZmmReg(I->first))<br>
> + for (std::pair<unsigned, unsigned> LI : MRI.liveins())<br>
> + if (isYmmOrZmmReg(LI.first))<br>
> return true;<br>
><br>
> return false;<br>
><br>
><br>
> _____________________________<wbr>__________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a> <mailto:<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.<wbr>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-<wbr>bin/mailman/listinfo/llvm-<wbr>commits</a><br>
><br>
<br>
--<br>
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,<br>
hosted by The Linux Foundation<br>
</blockquote></div></div></div></div>
<br>______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">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/<wbr>mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div>