Hey Reid, you're neck deep in cleanups and EH stuff, could you take a look at this?
It seems really important to have several folks familiar with this as its been tricky to land w/o regressing things but lays the groundwork for *really* important stack usage reductions, something we're all really sensitive to... On Tue, Sep 23, 2014 at 9:06 AM, Arnaud A. de Grandmaison < [email protected]> wrote: > Ping > > > > *From:* [email protected] [mailto: > [email protected]] *On Behalf Of *Arnaud A. de Grandmaison > *Sent:* 18 September 2014 16:35 > *To:* llvm cfe > *Subject:* RE: [PATCH] Emit lifetime start/end for unnamed objects --- > take 3 > > > > Gentle ping: could someone familiar with cleanup scopes and lifetime > extended temporaries have a look at the patch (attached again to this mail > for convenience)? > > > > Cheers, > > Arnaud > > > > *From:* David Blaikie [mailto:[email protected] <[email protected]>] > *Sent:* 16 September 2014 20:50 > *To:* Arnaud De Grandmaison > *Cc:* llvm cfe > *Subject:* Re: [PATCH] Emit lifetime start/end for unnamed objects --- > take 3 > > > > > > > > On Tue, Sep 16, 2014 at 1:29 AM, Arnaud A. de Grandmaison < > [email protected]> wrote: > > Hi David, > > > > In principal, giving more lifetime info can only improve stack slot > sharing and reduce runtime stack usage, which is critical for us in the > embedded world. > > > > I did not test that on a wide code base yet, but we had a customer > reporting an issue where llvm/clang was producing code with a stack usage > significantly worse than gcc. To make things worse, in their case their > code was heavily recursive, to the point where using clang was simply not > an option: they are forced to use gcc L > > > > This patch only adds lifetime markers to big enough (>32 bytes) objects, > consistent with what is done for named temporaries. I do not know how this > 32 bytes threshold has been choosen, but there is for sure a compile time / > stack size gain trade-off to be made. My experiments have shown that for > our customer case, the threshold should be lower: 16-bytes. But changing > this threshold would require a separate thread on this list, as well as > much more measurements. > > > > The improvements I have been able to get, by visual inspection of the > generated assembly code, for a single call of the hot functions were: > > > > | GCC | Clang | LT-32 | LT-16 | > > ===+=====+=======+=======+=======+ > > F1 | 432 | 608 | 608 | 400 | > > F2 | 432 | 640 | 640 | 432 | > > F3 | 384 | 368 | 368 | 192 | > > F4 | 320 | 400 | 400 | 224 | > > > > Stack size is expressed in bytes. > > GCC version 4.8 > > LT-32 is clang with this patch (default 32 bytes threshold for all > temporaries). > > LT-16 is clang with this patch and a 16 bytes threshold for all > temporaries. > > > > I believe bootstrapping clang could be a good testcase and will be needed > when we will address the real problem in a separate discussion: what > threshold should we use ? > > > > Very strangely to me coming from the embedded world, I have not found how > to measure a program stack usage on linux, so if you have any idea, I am > glad to hear about it. > > > > I'm not sure what the nicest way to do it for the running program or > examining a binary, but I would /imagine/ that LLVM might have a > counter/statistic for stack usage. I believe LLVM has some way to record > statistics about optimizations, etc, for debugging the compiler. So if it > doesn't have a "stack size" stat counter, it could hopefully be added. > > But you're right, for now - adding more should be generally better. I'm > not sure if there's a concern that adding too many (given that there's a > threshold, I assume someone tried it without a threshold and found that it > created too much metadata) intrinsics like this - so there might be some > need to show cost/benefit... (maybe looking at the commit archive to find > the original commits, those that added the threshold, etc) > > (this is way out of my depth/area of interest - but just replying with > suggestions/ideas to both make the conversation more visible, give you some > ideas that might pre-empt ideas from other more knowledgeable reviewers, > etc) > > - David > > > > > > Cheers, > > Arnaud > > > > > > *From:* David Blaikie [mailto:[email protected]] > *Sent:* 16 September 2014 01:25 > *To:* Arnaud De Grandmaison > *Cc:* llvm cfe > *Subject:* Re: [PATCH] Emit lifetime start/end for unnamed objects --- > take 3 > > > > I'm hardly an expert on this stuff - but just curious: what sort of > testing did you put this through? Bootstrap Clang? Were you able to gather > any stats on reduced stack usage with this improvement to lifetime markers? > > > > On Mon, Sep 15, 2014 at 4:05 PM, Arnaud A. de Grandmaison < > [email protected]> wrote: > > Hi All, > > > > Please find attached a patch which teaches clang to emit lifetime.start / > lifetime.end markers for unnamed temporary objects. > > > > This patch can greatly reduce the stack usage of some C++ code, where it > is so easy to have short lived unnamed temporaries. > > > > As noted in the subject, this is my third attempt: my previous attempts > failed to handle correctly the lifetime extended temporaries, and I have > had a hard time to understand the CleanupScope. It all boiled down to the > fact that the body of a function is not considered a full CleanupScope (for > debug information reasons), so in the case of lifetime extended objects at > the top level of the function body, with a trivial destructor + > lifetime.end marker, the lifetime markers were simply not considered, > firing an assert in ~CodeGenFunction. All cases are now covered by > testcases. > > > > I would appreciate if someone knowledgeable with the lifetime extended > temporaries & cleanup scopes could give a look to this patch. > > > > Cheers, > > -- > > Arnaud A. de Grandmaison > > > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > > > > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
