[PATCH] Emit lifetime start/end for unnamed objects --- take 3

Thu Sep 18 07:34:33 PDT 2014

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:dblaikie at gmail.com] 
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 <arnaud.degrandmaison at arm.com> 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:dblaikie at gmail.com] 
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 <arnaud.degrandmaison at arm.com> 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
cfe-commits at cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140918/43227cdb/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Emit-lifetime.start-lifetime.end-markers-for-unnamed.patch
Type: application/octet-stream
Size: 23212 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140918/43227cdb/attachment.obj>