<div dir="ltr">Yeah, the optimizer seems to be failing somewhere. One of the first things is that we don't actually fully inline the previous example (so we can't constant fold the store of 0), also we have code for the "shrinking" cases. So to simplify, this code (using libc++ <vector>)<div><br></div><div>```</div><div><div>#include <vector></div><div><br></div><div>void blarg(std::vector<char *> &V) {</div><div> std::vector<char*> VNew(1000, nullptr); </div><div> V.swap(VNew);</div><div>}</div></div><div>```</div><div><br></div><div>We get the attached code from:</div><div>~/pg/release/bin/clang++ -I ~/pg/libcxx/include -fno-exceptions -fno-unroll-loops -c -O2 testvectorresize.cpp -mllvm -inline-threshold=10000 -emit-llvm -o - | ~/pg/release/bin/opt -S -view-cfg</div><div><br></div><div>I've also attached O0 IR in case somebody wants to take a look at it.</div><div><br></div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Sep 25, 2015 at 6:03 PM, Sean Silva <span dir="ltr"><<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</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">Attached is the resulting -view-cfg of the -O2 IR.</div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Sep 25, 2015 at 5:58 PM, Sean Silva <span dir="ltr"><<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</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><div class="gmail_extra"><br><div class="gmail_quote"><span>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>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></span><div>For libcxx at least it boils down to:</div><div><br></div><div><a href="http://reviews.llvm.org/P126" target="_blank">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" target="_blank">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" target="_blank">http://reviews.llvm.org/P128</a> </div><span><font color="#888888"><div><br></div><div>-- Sean Silva</div></font></span><div><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> 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" 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></div></div><br></div></div>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div>