<div dir="ltr">Ping.<div><br></div><div>Thanks,</div><div>Teresa</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Feb 16, 2016 at 10:24 AM, Peter Collingbourne <span dir="ltr"><<a href="mailto:peter@pcc.me.uk" target="_blank">peter@pcc.me.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Mehdi,<br>
<br>
Can we resolve this by stipulating that Teresa should implement in whichever<br>
way she finds easiest?<br>
<br>
Thanks,<br>
Peter<br>
<div class="HOEnZb"><div class="h5"><br>
On Tue, Feb 16, 2016 at 05:45:06PM +0000, Teresa Johnson wrote:<br>
> I can see both sides to this, and will implement either. Can we close on a<br>
> decision? I don't want to hold up the thinlto side much longer. Out of town<br>
> this week, but if we can get consensus so I can get this in when I'm back,<br>
> that would be great! Thanks, Teresa<br>
><br>
> On Fri, Feb 12, 2016, 1:53 PM Peter Collingbourne <<a href="mailto:peter@pcc.me.uk">peter@pcc.me.uk</a>> wrote:<br>
><br>
> > On Fri, Feb 12, 2016 at 03:36:19PM -0800, Mehdi Amini wrote:<br>
> > ><br>
> > ><br>
> > > Sent from my iPhone<br>
> > ><br>
> > > > On Feb 12, 2016, at 3:09 PM, Peter Collingbourne <<a href="mailto:peter@pcc.me.uk">peter@pcc.me.uk</a>><br>
> > wrote:<br>
> > > ><br>
> > > >> On Fri, Feb 12, 2016 at 02:53:04PM -0800, Mehdi Amini wrote:<br>
> > > >><br>
> > > >><br>
> > > >> Sent from my iPhone<br>
> > > >><br>
> > > >>> On Feb 12, 2016, at 2:22 PM, Peter Collingbourne <<a href="mailto:peter@pcc.me.uk">peter@pcc.me.uk</a>><br>
> > wrote:<br>
> > > >>><br>
> > > >>> pcc added inline comments.<br>
> > > >>><br>
> > > >>> ================<br>
> > > >>> Comment at: tools/gold/gold-plugin.cpp:1019<br>
> > > >>> @@ +1018,3 @@<br>
> > > >>> +    raw_svector_ostream BCOS(BCs.back());<br>
> > > >>> +    WriteBitcodeToFile(MPart.get(), BCOS);<br>
> > > >>> +  });<br>
> > > >>> ----------------<br>
> > > >>> joker.eph wrote:<br>
> > > >>>> joker.eph wrote:<br>
> > > >>>>> pcc wrote:<br>
> > > >>>>>> I don't think thread pools are necessary for split code gen, as<br>
> > we can already perfectly assign the right amount of work to individual<br>
> > threads. Also, this implementation loses the pipelining feature from the<br>
> > original code (i.e. worker threads can work on codegen'ing while the main<br>
> > thread is still splitting). I would prefer you to use the existing<br>
> > implementation in `llvm/CodeGen/ParallelCG.h`.<br>
> > > >>>>> The thread that is doing the splitting can issue other jobs to the<br>
> > thread pool, providing the desired pipeline.<br>
> > > >>>>><br>
> > > >>>>> Just fuse the loop body below within the lambda...<br>
> > > >>>> I'll add that while the pooling is not necessary if you only queue<br>
> > as many jobs as you have threads, it is not a reason by itself not to use<br>
> > it: the paradigm is fairly clear, and it decouples the actual splitting<br>
> > granularity from the number of actual worker threads, allowing to<br>
> > experiment with different numbers for each (providing better pipelining for<br>
> > instance).<br>
> > > >>> Yes, but the current implementation doesn't need any of that. If we<br>
> > experimentally find that decoupling would provide some benefit, then by all<br>
> > means we can start using thread pools here.<br>
> > > >>><br>
> > > >>> In any case, if there is a compelling reason to use thread pools,<br>
> > the right place to make the change is in `lib/CodeGen/ParallelCG.cpp`<br>
> > rather than in a duplicate implementation here. We can defer what the<br>
> > design for that should look like simply by not using thread pools yet.<br>
> > > >>><br>
> > > >><br>
> > > >> I don't understand why should we defer what is clearly identified as<br>
> > a decoupled implementation, for which you haven't articulate any drawbacks<br>
> > (unless I missed one point).<br>
> > > ><br>
> > > > The drawback is that it adds unnecessary complexity to the codebase.<br>
> > ><br>
> > > Having two places that issue thread in a different way is also useless<br>
> > complexity.<br>
> ><br>
> > I have to disagree with you there. Each place uses concurrency in a way<br>
> > that<br>
> > is appropriate to its use case. Trying to shoehorn things into<br>
> > inappropriate<br>
> > concurrency models just makes the codebase harder to understand. There's a<br>
> > reason why we use different data structures in different places, etc.<br>
> ><br>
> > (Right now I'm battling the DWARF writer implementation, which has built up<br>
> > 10 years of technical debt as a result of using a poorly designed, overly<br>
> > generic data structure.)<br>
> ><br>
> > > Rewriting the split code gen is on my list of cleanup for some time and<br>
> > if I actually had to use it on my cod path I would already have decoupled<br>
> > it and moved it to the thread pool.<br>
> > ><br>
> > > ><br>
> > > >> The fact that you want it implemented in a different place is not a<br>
> > reason to not implement it now.<br>
> > > ><br>
> > > > My view is that *if* we implement it, it should not be here. I am<br>
> > against<br>
> > > > adding it now, anywhere.<br>
> > ><br>
> > > Well we'll need to continue talking about it because I have a totally<br>
> > different opinion on this topic.<br>
> > ><br>
> > ><br>
> > > ><br>
> > > >> I'm against building any technical debt when we can easily avoid it.<br>
> > > ><br>
> > > > So am I. My view is that adding the thread pool implementation<br>
> > > The thread pool implementation is already in the code base.<br>
> ><br>
> > I meant the split code gen implementation that uses thread pools.<br>
> ><br>
> > > > adds to the<br>
> > > > technical debt because it would mean more code and therefore more<br>
> > complexity<br>
> > ><br>
> > > I don't buy that and unless we're actually looking at a diff I'm not<br>
> > sure we'll be able to make progress on the tradeoff at stance here.<br>
> ><br>
> > Fair enough. If you still hold that view I think we'll have to agree to<br>
> > disagree here, I'll drop my argument and we can get back to coding :)<br>
> ><br>
> > Thanks,<br>
> > --<br>
> > Peter<br>
> ><br>
<br>
</div></div><span class="HOEnZb"><font color="#888888">--<br>
Peter<br>
</font></span></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature"><span style="font-family:Times;font-size:medium"><table cellspacing="0" cellpadding="0"><tbody><tr style="color:rgb(85,85,85);font-family:sans-serif;font-size:small"><td nowrap style="border-top-style:solid;border-top-color:rgb(213,15,37);border-top-width:2px">Teresa Johnson |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(51,105,232);border-top-width:2px"> Software Engineer |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(0,153,57);border-top-width:2px"> <a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a> |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(238,178,17);border-top-width:2px"> 408-460-2413</td></tr></tbody></table></span></div>
</div>