<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Sep 25, 2015 at 5:32 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:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span class="">On Fri, Sep 25, 2015 at 5:28 PM, escha 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:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">escha created this revision.<br>
escha added reviewers: MatzeB, arsenm, resistor.<br>
escha added a subscriber: llvm-commits.<br>
escha set the repository for this revision to rL LLVM.<br>
<br>
std::vector.resize() is actually about 2-3x slower than using unique_ptr, at least for me; it doesn't compile to bzero because the resize() in std:: consists of repeated appending (agh).</blockquote><div><br></div></span><div>Wait, what? Which standard library has that implementation? (is it necessary/implied by the standard, or just a poorly performing implementation that can/should be fixed?)</div></div></div></div></blockquote><div><br></div><div>For libcxx at least it boils down to:</div><div><br></div><div><a href="http://reviews.llvm.org/P126">http://reviews.llvm.org/P126</a><br></div><div><br></div><div>For __construct_at_end:</div><div>`// Precondition: size() + __n <= capacity()`<br></div><div><br></div><div>So it isn't repeated push_back's. We are just failing to optimize it (or maybe something is weird inside vector?). Marshall, any ideas?</div><div><br></div><div>We do end up with some really funky looking code though:</div><div><a href="http://reviews.llvm.org/P127">http://reviews.llvm.org/P127</a><br></div><div><br></div><div>```</div><div><div>#include <vector></div><div> </div><div>void blarg(std::vector<char *> &V) {</div><div> V.resize(1000, nullptr);</div><div>}</div></div><div>```</div><div>(one thing to note is that the "shrinking" case also has to be handled (i.e. if V.size() was >1000 on entry to the function))</div><div><br></div><div>The O3 IR is scary looking: <a href="http://reviews.llvm.org/P128">http://reviews.llvm.org/P128</a> </div><div><br></div><div>-- Sean Silva</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div class="h5"> On our out of tree target with a huge number of registers, this patch actually saves 1% of compile time. It should help significantly on AMDGPU as well, though likely a factor less due to them not having quite the same level of ridiculous register counts.<br>
<br>
In general, we probably shouldn't use the heavyweight-ness of std::vector for constant-size arrays. There's really no reason to.<br>
<br>
Repository:<br>
rL LLVM<br>
<br>
<a href="http://reviews.llvm.org/D13185" rel="noreferrer" target="_blank">http://reviews.llvm.org/D13185</a><br>
<br>
Files:<br>
include/llvm/CodeGen/MachineRegisterInfo.h<br>
lib/CodeGen/MachineRegisterInfo.cpp<br>
lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp<br>
<br>
Index: lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp<br>
===================================================================<br>
--- lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp<br>
+++ lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp<br>
@@ -141,8 +141,8 @@<br>
/// that are "live". These nodes must be scheduled before any other nodes that<br>
/// modifies the registers can be scheduled.<br>
unsigned NumLiveRegs;<br>
- std::vector<SUnit*> LiveRegDefs;<br>
- std::vector<SUnit*> LiveRegGens;<br>
+ std::unique_ptr<SUnit*[]> LiveRegDefs;<br>
+ std::unique_ptr<SUnit*[]> LiveRegGens;<br>
<br>
// Collect interferences between physical register use/defs.<br>
// Each interference is an SUnit and set of physical registers.<br>
@@ -328,8 +328,8 @@<br>
NumLiveRegs = 0;<br>
// Allocate slots for each physical register, plus one for a special register<br>
// to track the virtual resource of a calling sequence.<br>
- LiveRegDefs.resize(TRI->getNumRegs() + 1, nullptr);<br>
- LiveRegGens.resize(TRI->getNumRegs() + 1, nullptr);<br>
+ LiveRegDefs.reset(new SUnit*[TRI->getNumRegs() + 1]());<br>
+ LiveRegGens.reset(new SUnit*[TRI->getNumRegs() + 1]());<br>
CallSeqEndForStart.clear();<br>
assert(Interferences.empty() && LRegsMap.empty() && "stale Interferences");<br>
<br>
@@ -1218,7 +1218,7 @@<br>
/// CheckForLiveRegDef - Return true and update live register vector if the<br>
/// specified register def of the specified SUnit clobbers any "live" registers.<br>
static void CheckForLiveRegDef(SUnit *SU, unsigned Reg,<br>
- std::vector<SUnit*> &LiveRegDefs,<br>
+ std::unique_ptr<SUnit*[]> &LiveRegDefs,<br>
SmallSet<unsigned, 4> &RegAdded,<br>
SmallVectorImpl<unsigned> &LRegs,<br>
const TargetRegisterInfo *TRI) {<br>
@@ -1240,11 +1240,12 @@<br>
/// CheckForLiveRegDefMasked - Check for any live physregs that are clobbered<br>
/// by RegMask, and add them to LRegs.<br>
static void CheckForLiveRegDefMasked(SUnit *SU, const uint32_t *RegMask,<br>
- std::vector<SUnit*> &LiveRegDefs,<br>
+ std::unique_ptr<SUnit*[]> &LiveRegDefs,<br>
SmallSet<unsigned, 4> &RegAdded,<br>
- SmallVectorImpl<unsigned> &LRegs) {<br>
+ SmallVectorImpl<unsigned> &LRegs,<br>
+ unsigned NumRegs) {<br>
// Look at all live registers. Skip Reg0 and the special CallResource.<br>
- for (unsigned i = 1, e = LiveRegDefs.size()-1; i != e; ++i) {<br>
+ for (unsigned i = 1, e = NumRegs; i != e; ++i) {<br>
if (!LiveRegDefs[i]) continue;<br>
if (LiveRegDefs[i] == SU) continue;<br>
if (!MachineOperand::clobbersPhysReg(RegMask, i)) continue;<br>
@@ -1328,7 +1329,8 @@<br>
}<br>
}<br>
if (const uint32_t *RegMask = getNodeRegMask(Node))<br>
- CheckForLiveRegDefMasked(SU, RegMask, LiveRegDefs, RegAdded, LRegs);<br>
+ CheckForLiveRegDefMasked(SU, RegMask, LiveRegDefs, RegAdded, LRegs,<br>
+ TRI->getNumRegs());<br>
<br>
const MCInstrDesc &MCID = TII->get(Node->getMachineOpcode());<br>
if (!MCID.ImplicitDefs)<br>
Index: lib/CodeGen/MachineRegisterInfo.cpp<br>
===================================================================<br>
--- lib/CodeGen/MachineRegisterInfo.cpp<br>
+++ lib/CodeGen/MachineRegisterInfo.cpp<br>
@@ -27,12 +27,11 @@<br>
MachineRegisterInfo::MachineRegisterInfo(const MachineFunction *MF)<br>
: MF(MF), TheDelegate(nullptr), IsSSA(true), TracksLiveness(true),<br>
TracksSubRegLiveness(false) {<br>
+ unsigned NumRegs = getTargetRegisterInfo()->getNumRegs();<br>
VRegInfo.reserve(256);<br>
RegAllocHints.reserve(256);<br>
- UsedPhysRegMask.resize(getTargetRegisterInfo()->getNumRegs());<br>
-<br>
- // Create the physreg use/def lists.<br>
- PhysRegUseDefLists.resize(getTargetRegisterInfo()->getNumRegs(), nullptr);<br>
+ UsedPhysRegMask.resize(NumRegs);<br>
+ PhysRegUseDefLists.reset(new MachineOperand*[NumRegs]());<br>
}<br>
<br>
/// setRegClass - Set the register class of the specified virtual register.<br>
Index: include/llvm/CodeGen/MachineRegisterInfo.h<br>
===================================================================<br>
--- include/llvm/CodeGen/MachineRegisterInfo.h<br>
+++ include/llvm/CodeGen/MachineRegisterInfo.h<br>
@@ -73,7 +73,7 @@<br>
<br>
/// PhysRegUseDefLists - This is an array of the head of the use/def list for<br>
/// physical registers.<br>
- std::vector<MachineOperand *> PhysRegUseDefLists;<br>
+ std::unique_ptr<MachineOperand *[]> PhysRegUseDefLists;<br>
<br>
/// getRegUseDefListHead - Return the head pointer for the register use/def<br>
/// list for the specified virtual or physical register.<br>
<br>
<br>
<br></div></div>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">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/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div></div>
<br>_______________________________________________<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/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div></div>