Subzero: Fix stats collection and output for multithreading.

Subzero: Fix stats collection and output for multithreading.

Updates of current-function and cumulative stats are done entirely in TLS.  At the end, cumulative stats are merged across all threads' TLS into the global cumulative stats.

Printing of cumulative stats after every function is removed, since there's very little value from that.  It was probably done in the first place just to give partial cumulative information in the face of crashes or assertion failures.

BUG= none
R=jfb@chromium.org

Committed: https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-subzero.git;a=commit;h=a1dd3cc8fb88cddc7ce823747cba507c8f283b70

[Jim Stichnoth, 2015-01-31 02:25:44]

stichnot@chromium.org changed reviewers:
+ jfb@chromium.org, jvoung@chromium.org, kschimpf@google.com

[JF, 2015-01-31 05:08:24]

https://codereview.chromium.org/887213002/diff/1/src/IceGlobalContext.h
File src/IceGlobalContext.h (right):

https://codereview.chromium.org/887213002/diff/1/src/IceGlobalContext.h#newco...
src/IceGlobalContext.h:245:
ICE_TLS_GET_FIELD(TLS)->StatsCumulative.updateFills();
Can you get the field only once per function instead of twice? I'm not sure the
compiler can eliminate redundant TLS fetches in all circumstances.

[Jim Stichnoth, 2015-01-31 05:36:59]

https://codereview.chromium.org/887213002/diff/1/src/IceGlobalContext.h
File src/IceGlobalContext.h (right):

https://codereview.chromium.org/887213002/diff/1/src/IceGlobalContext.h#newco...
src/IceGlobalContext.h:245:
ICE_TLS_GET_FIELD(TLS)->StatsCumulative.updateFills();
On 2015/01/31 05:08:24, JF wrote:
> Can you get the field only once per function instead of twice? I'm not sure
the
> compiler can eliminate redundant TLS fetches in all circumstances.

Nice, done.

[JF, 2015-01-31 05:48:38]

lgtm after the `auto *TLS` fix. x-macros can be separate, if you think it would
be good.

https://codereview.chromium.org/887213002/diff/20001/src/IceGlobalContext.h
File src/IceGlobalContext.h (right):

https://codereview.chromium.org/887213002/diff/20001/src/IceGlobalContext.h#n...
src/IceGlobalContext.h:90: uint32_t Fills;
X-macros for these, and add(), and the update*() methods?

https://codereview.chromium.org/887213002/diff/20001/src/IceGlobalContext.h#n...
src/IceGlobalContext.h:220: auto TLS = ICE_TLS_GET_FIELD(TLS);
auto *TLS = ...;

[Jim Stichnoth, 2015-01-31 16:42:28]

Clang-format seems to be doing a better job these days of x-macros formatting. 
I should consider moving some of the .def files back into their .h files.

https://codereview.chromium.org/887213002/diff/20001/src/IceGlobalContext.h
File src/IceGlobalContext.h (right):

https://codereview.chromium.org/887213002/diff/20001/src/IceGlobalContext.h#n...
src/IceGlobalContext.h:90: uint32_t Fills;
On 2015/01/31 05:48:38, JF wrote:
> X-macros for these, and add(), and the update*() methods?

Done.

https://codereview.chromium.org/887213002/diff/20001/src/IceGlobalContext.h#n...
src/IceGlobalContext.h:220: auto TLS = ICE_TLS_GET_FIELD(TLS);
On 2015/01/31 05:48:38, JF wrote:
> auto *TLS = ...;

Done, took it one step further...

My original idea was that when we eventually get rid of these ICE_TLS macros,
this initialization would reduce to "auto TLS = TLS;" and ultimately be removed
entirely without any surrounding code modifications, so using the simplest
possible "auto" initialization would make it easier to come to that conclusion. 
But, that's probably overthinking it so I put back the full declaration.

[JF, 2015-01-31 17:46:25]

https://codereview.chromium.org/887213002/diff/60001/src/IceGlobalContext.h
File src/IceGlobalContext.h (right):

https://codereview.chromium.org/887213002/diff/60001/src/IceGlobalContext.h#n...
src/IceGlobalContext.h:72: //#define X(str, tag)
Leftover?

https://codereview.chromium.org/887213002/diff/60001/src/IceGlobalContext.h#n...
src/IceGlobalContext.h:97: uint32_t Stats[CS_NUM];
std::array, and then you can use the std algorithms above :-D

[Jim Stichnoth, 2015-01-31 18:32:29]

https://codereview.chromium.org/887213002/diff/60001/src/IceGlobalContext.h
File src/IceGlobalContext.h (right):

https://codereview.chromium.org/887213002/diff/60001/src/IceGlobalContext.h#n...
src/IceGlobalContext.h:72: //#define X(str, tag)
On 2015/01/31 17:46:25, JF wrote:
> Leftover?

No - our x-macros style when defining the table is to leave the expected
"#define X(...)" usage in a comment.  I just wish clang-format wouldn't indent
it...

https://codereview.chromium.org/887213002/diff/60001/src/IceGlobalContext.h#n...
src/IceGlobalContext.h:97: uint32_t Stats[CS_NUM];
On 2015/01/31 17:46:25, JF wrote:
> std::array, and then you can use the std algorithms above :-D

Done.

[JF, 2015-01-31 18:38:51]

lgtm

[Jim Stichnoth, 2015-01-31 18:48:16]

Committed patchset #5 (id:80001) manually as
a1dd3cc8fb88cddc7ce823747cba507c8f283b70 (presubmit successful).