<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Sep 25, 2015 at 5:42 PM, Matthias Braun 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">MatzeB added a comment.<br>
<br>
Given that the vectors are fixed size I don't see why we would need the additional complexity of std::vector (it has to manage the size and capacity, ...). </blockquote><div><br></div><div>I'm often on the fence about this - in this case it seems like at least the size is known/shared elsewhere.<br><br>I'd probably still have used std::dynarray, if it existed, which it doesn't - and since the standards committee decided it wasn't worth having, I tend to tip towards std::vector, rather than towards a raw array. But that's just me.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">To me this change looks good with the nitpicks below addressed.<br>
<br>
Of course it's bad that llvm doesn't get the vector initialisation optimized, but this is a separate issue, would you agree David?<br></blockquote><div><br>Sure, to a degree - maybe SmallVector would get the right benefits/have better resize performance & or if it's a sufficiently obscure standard library implementation that it's not worth optimizing for (perhaps someone's fixed it/it's an old version/etc).<br><br>But I'm just rambling - certainly not going to stop the patch going in, happy to leave it to other people to decide. <br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
================<br>
Comment at: lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp:1221<br>
@@ -1220,3 +1220,3 @@<br>
<span class=""> static void CheckForLiveRegDef(SUnit *SU, unsigned Reg,<br>
- std::vector<SUnit*> &LiveRegDefs,<br>
+ std::unique_ptr<SUnit*[]> &LiveRegDefs,<br>
SmallSet<unsigned, 4> &RegAdded,<br>
</span>----------------<br>
This can be a simple SUnit * now.<br>
<br>
================<br>
Comment at: lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp:1243-1246<br>
@@ -1242,5 +1242,6 @@<br>
<span class=""> 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>
</span>----------------<br>
This could use an ArrayRef<SUnit*> instead of std::unique_ptr reference + NumRegs.<br>
<span class="im HOEnZb"><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>
<br>
<br>
</span><div class="HOEnZb"><div class="h5">_______________________________________________<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>
</div></div></blockquote></div><br></div></div>