Emit unwinding information for TurboFan code.

Emit unwinding information for TurboFan code.

This commit introduces support for writing unwinding tables in the
.eh_frame format, to be inserted in the jitdump read by Linux perf and
emitted with FLAG_perf_prof and FLAG_perf_prof_unwinding_info enabled.

x64 is fully implemented and tested, arm and arm64 are untested and the
unwinding information needs to be expanded, but the mechanism is ready.

BUG=v8:4899
LOG=N

Committed: https://crrev.com/cecded1c24bf3c3409d6556c11af2bd7b0ffea69
Cr-Commit-Position: refs/heads/master@{#37799}

[Stefano Sanfilippo, 2016-06-02 15:08:53]

Patchset #4 (id:60001) has been deleted

[Stefano Sanfilippo, 2016-06-02 18:06:01]

Patchset #4 (id:80001) has been deleted

[Stefano Sanfilippo, 2016-06-07 18:41:07]

Patchset #5 (id:120001) has been deleted

[Stefano Sanfilippo, 2016-06-07 19:14:30]

Patchset #7 (id:180001) has been deleted

[Stefano Sanfilippo, 2016-06-07 19:19:35]

Patchset #5 (id:140001) has been deleted

[Stefano Sanfilippo, 2016-06-07 19:20:18]

Patchset #5 (id:160001) has been deleted

[Stefano Sanfilippo, 2016-06-07 19:20:24]

Patchset #5 (id:200001) has been deleted

[Stefano Sanfilippo, 2016-06-07 19:22:02]

Patchset #5 (id:220001) has been deleted

[Stefano Sanfilippo, 2016-06-07 19:32:19]

Patchset #5 (id:240001) has been deleted

[Stefano Sanfilippo, 2016-06-21 12:55:04]

Patchset #1 (id:1) has been deleted

[Stefano Sanfilippo, 2016-06-21 12:55:12]

Patchset #1 (id:20001) has been deleted

[Stefano Sanfilippo, 2016-06-21 12:55:19]

Patchset #1 (id:40001) has been deleted

[Stefano Sanfilippo, 2016-06-21 12:55:27]

Patchset #1 (id:100001) has been deleted

[Stefano Sanfilippo, 2016-06-21 12:55:34]

Patchset #1 (id:260001) has been deleted

[Stefano Sanfilippo, 2016-06-21 12:55:41]

Patchset #1 (id:280001) has been deleted

[Stefano Sanfilippo, 2016-06-21 12:55:48]

Patchset #1 (id:300001) has been deleted

[Stefano Sanfilippo, 2016-06-21 12:55:55]

Patchset #1 (id:320001) has been deleted

[Stefano Sanfilippo, 2016-06-24 16:45:08]

Patchset #3 (id:380001) has been deleted

[Stefano Sanfilippo, 2016-06-24 16:45:41]

Description was changed from

==========
NOT FOR REVIEW Plug UnwindingInfoWriter in TurboFan.
==========

to

==========
NOT FOR REVIEW Write unwinding information in TurboFan.
==========

[Stefano Sanfilippo, 2016-06-29 15:10:06]

Patchset #9 (id:520001) has been deleted

[Stefano Sanfilippo, 2016-06-29 15:10:17]

Patchset #2 (id:360001) has been deleted

[Stefano Sanfilippo, 2016-06-29 15:10:25]

Patchset #2 (id:400001) has been deleted

[Stefano Sanfilippo, 2016-06-29 15:10:35]

Patchset #2 (id:420001) has been deleted

[Stefano Sanfilippo, 2016-06-29 15:10:48]

Patchset #2 (id:440001) has been deleted

[Stefano Sanfilippo, 2016-06-29 15:16:40]

The CQ bit was checked by ssanfilippo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-29 15:16:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-29 15:19:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-29 15:19:19]

Dry run: Try jobs failed on following builders:
  v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
  v8_linux64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/8251)
  v8_linux_gcc_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)
  v8_linux_mips64el_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...)

[Stefano Sanfilippo, 2016-06-29 15:23:13]

The CQ bit was checked by ssanfilippo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-29 15:23:18]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-29 15:25:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-29 15:25:50]

Dry run: Try jobs failed on following builders:
  v8_linux_arm_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/...)

[Stefano Sanfilippo, 2016-06-29 15:31:57]

The CQ bit was checked by ssanfilippo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-29 15:32:11]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Stefano Sanfilippo, 2016-06-29 15:34:30]

Patchset #7 (id:570001) has been deleted

[Stefano Sanfilippo, 2016-06-29 15:34:37]

The CQ bit was checked by ssanfilippo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-29 15:34:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-29 15:39:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-29 15:39:27]

Dry run: Try jobs failed on following builders:
  v8_android_arm_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_android_arm_compile_rel/...)
  v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)

[Stefano Sanfilippo, 2016-06-29 16:44:25]

The CQ bit was checked by ssanfilippo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-29 16:44:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-29 17:10:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-29 17:10:23]

Dry run: This issue passed the CQ dry run.

[Stefano Sanfilippo, 2016-06-30 10:49:04]

The CQ bit was checked by ssanfilippo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-30 10:49:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Stefano Sanfilippo, 2016-06-30 10:52:15]

Description was changed from

==========
NOT FOR REVIEW Write unwinding information in TurboFan.
==========

to

==========
Emit unwinding information for TurboFan code.

BUG=v8:4899
LOG=N
==========

[commit-bot: I haz the power, 2016-06-30 11:15:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-30 11:15:03]

Dry run: This issue passed the CQ dry run.

[Stefano Sanfilippo, 2016-07-04 19:06:34]

The CQ bit was checked by ssanfilippo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-04 19:06:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-04 19:08:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-04 19:08:13]

Dry run: Try jobs failed on following builders:
  v8_android_arm_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_android_arm_compile_rel/...)
  v8_linux_mips64el_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...)
  v8_linux_mipsel_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mipsel_compile_rel...)
  v8_linux_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/8446)
  v8_presubmit on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/18730)

[Stefano Sanfilippo, 2016-07-04 19:56:49]

Description was changed from

==========
Emit unwinding information for TurboFan code.

BUG=v8:4899
LOG=N
==========

to

==========
Emit unwinding information in TurboFan codegen.

BUG=v8:4899
LOG=N
==========

[Stefano Sanfilippo, 2016-07-04 19:57:11]

Patchset #13 (id:700001) has been deleted

[Stefano Sanfilippo, 2016-07-04 19:57:18]

The CQ bit was checked by ssanfilippo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-04 19:57:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-04 20:22:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-04 20:22:32]

Dry run: This issue passed the CQ dry run.

[Stefano Sanfilippo, 2016-07-05 10:31:53]

Patchset #13 (id:720001) has been deleted

[Stefano Sanfilippo, 2016-07-05 10:32:03]

The CQ bit was checked by ssanfilippo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-05 10:32:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-05 10:35:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-05 10:35:08]

Dry run: Try jobs failed on following builders:
  v8_linux64_asan_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng/buil...)
  v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
  v8_linux64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/8525)
  v8_linux_arm64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...)

[Stefano Sanfilippo, 2016-07-05 16:13:11]

Description was changed from

==========
Emit unwinding information in TurboFan codegen.

BUG=v8:4899
LOG=N
==========

to

==========
Make TurboFan code generator emit unwinding information.

BUG=v8:4899
LOG=N
==========

[Stefano Sanfilippo, 2016-07-05 16:13:11]

ssanfilippo@chromium.org changed reviewers:
+ jarin@chromium.org

[Stefano Sanfilippo, 2016-07-05 16:13:23]

Patchset #13 (id:740001) has been deleted

[Stefano Sanfilippo, 2016-07-05 16:13:48]

Description was changed from

==========
Make TurboFan code generator emit unwinding information.

BUG=v8:4899
LOG=N
==========

to

==========
Make TurboFan emit unwinding information.

BUG=v8:4899
LOG=N
==========

[Stefano Sanfilippo, 2016-07-05 16:14:24]

This is the last piece in having TF emit unwinding information. PTAL.

[Stefano Sanfilippo, 2016-07-05 16:19:10]

Description was changed from

==========
Make TurboFan emit unwinding information.

BUG=v8:4899
LOG=N
==========

to

==========
Make TurboFan emit unwinding information.

This commit introduce support for writing x64, arm and arm64 unwinding 
tables in the .eh_frame format, to be inserted in the jitdump read by 
Linux perf. ia32 has been left behind for the moment.

BUG=v8:4899
LOG=N
==========

[Stefano Sanfilippo, 2016-07-05 16:19:20]

Description was changed from

==========
Make TurboFan emit unwinding information.

This commit introduce support for writing x64, arm and arm64 unwinding 
tables in the .eh_frame format, to be inserted in the jitdump read by 
Linux perf. ia32 has been left behind for the moment.

BUG=v8:4899
LOG=N
==========

to

==========
Make TurboFan emit unwinding information.

This commit introduces support for writing x64, arm and arm64 unwinding 
tables in the .eh_frame format, to be inserted in the jitdump read by 
Linux perf. ia32 has been left behind for the moment.

BUG=v8:4899
LOG=N
==========

[Stefano Sanfilippo, 2016-07-05 16:19:58]

Description was changed from

==========
Make TurboFan emit unwinding information.

This commit introduces support for writing x64, arm and arm64 unwinding 
tables in the .eh_frame format, to be inserted in the jitdump read by 
Linux perf. ia32 has been left behind for the moment.

BUG=v8:4899
LOG=N
==========

to

==========
Make TurboFan emit unwinding information.

This commit introduces support for writing x64, arm and arm64 unwinding 
tables in the .eh_frame format, to be inserted in the jitdump read by 
Linux perf.

BUG=v8:4899
LOG=N
==========

[rmcilroy, 2016-07-05 16:39:15]

rmcilroy@chromium.org changed reviewers:
+ rmcilroy@chromium.org

[rmcilroy, 2016-07-05 16:39:16]

Jaro: could you do the main review for this since it impacts Turbofan? Stefano's
internship ends next week, so if you could have a look soon that'd be great,
thank.

[Stefano Sanfilippo, 2016-07-05 17:49:51]

Description was changed from

==========
Make TurboFan emit unwinding information.

This commit introduces support for writing x64, arm and arm64 unwinding 
tables in the .eh_frame format, to be inserted in the jitdump read by 
Linux perf.

BUG=v8:4899
LOG=N
==========

to

==========
Make TurboFan emit unwinding information.

This commit introduces support for writing x64, arm and arm64 unwinding 
tables in the .eh_frame format, to be inserted in the jitdump read by 
Linux perf and emitted with FLAG_perf_prof and 
FLAG_perf_prof_unwinding_info enabled.

BUG=v8:4899
LOG=N
==========

[Stefano Sanfilippo, 2016-07-05 19:56:16]

Patchset #13 (id:760001) has been deleted

[Stefano Sanfilippo, 2016-07-05 20:24:07]

Patchset #13 (id:780001) has been deleted

[Stefano Sanfilippo, 2016-07-05 20:24:42]

The CQ bit was checked by ssanfilippo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-05 20:24:52]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Jarin, 2016-07-06 07:09:42]

Mostly looking good, I have just a few suggestions.

https://codereview.chromium.org/2026313002/diff/820001/src/compiler/arm/code-...
File src/compiler/arm/code-generator-arm.cc (right):

https://codereview.chromium.org/2026313002/diff/820001/src/compiler/arm/code-...
src/compiler/arm/code-generator-arm.cc:429: if
(!unwinding_info_writer_.is_empty()) {
I am wondering whether you do not want to fold this is_empty logic into
UnwindingInforWriter - the writer would internally know whether it is active,
and it would only do its thing if it is active. That way, you could embed the
writer into the code generator, so here you would just say

unwinding_info_writer_.MarkFrameDeconstructed(__ pc_offset());

without the is_empty check. WDYT?

https://codereview.chromium.org/2026313002/diff/820001/src/compiler/arm/unwin...
File src/compiler/arm/unwinding-info-writer-arm.cc (right):

https://codereview.chromium.org/2026313002/diff/820001/src/compiler/arm/unwin...
src/compiler/arm/unwinding-info-writer-arm.cc:21:
eh_frame_writer_.RecordRegisterSavedToStack(lr, -4);
-4 ==> -kPointerSize? (Here and below.)

https://codereview.chromium.org/2026313002/diff/820001/src/compiler/arm/unwin...
File src/compiler/arm/unwinding-info-writer-arm.h (right):

https://codereview.chromium.org/2026313002/diff/820001/src/compiler/arm/unwin...
src/compiler/arm/unwinding-info-writer-arm.h:46: std::unordered_map<int,
BlockInitialState> block_initial_states_;
Nit: Could not you use a zone vector here? It seems to me you have entries for
most BBs, so the vector would be pretty dense. Or am I missing something?

(Also, using naked STL hash table makes me slightly uneasy. We usually do not
allocate on C++ heap directly, we tend to use zones. This is partly to have
better memory accounting, but also because using an STL containers directly
relies on destructors being called, and I am not sure we always guarantee that.)

https://codereview.chromium.org/2026313002/diff/820001/src/compiler/arm/unwin...
src/compiler/arm/unwinding-info-writer-arm.h:47: static_assert(kIntSize >=
kInt32Size, "int too small to encode a RpoNumber");
Normally, we use STATIC_ASSERT.

https://codereview.chromium.org/2026313002/diff/820001/src/compiler/arm64/unw...
File src/compiler/arm64/unwinding-info-writer-arm64.cc (right):

https://codereview.chromium.org/2026313002/diff/820001/src/compiler/arm64/unw...
src/compiler/arm64/unwinding-info-writer-arm64.cc:21:
eh_frame_writer_.RecordRegisterSavedToStack(lr, -8);
8 ==> kPointerSize?

https://codereview.chromium.org/2026313002/diff/820001/src/compiler/unwinding...
File src/compiler/unwinding-info-writer.h (right):

https://codereview.chromium.org/2026313002/diff/820001/src/compiler/unwinding...
src/compiler/unwinding-info-writer.h:13: #include
"src/compiler/ia32/unwinding-info-writer-ia32.h"
Instead of including the same unimplemented unwinding info writers here, could
not you just put the unimplemented version of the class here and only include
the headers with the real implementations? Or do you intend to implement
all/most of the remaining architectures soon?

https://codereview.chromium.org/2026313002/diff/820001/src/compiler/x64/code-...
File src/compiler/x64/code-generator-x64.cc (right):

https://codereview.chromium.org/2026313002/diff/820001/src/compiler/x64/code-...
src/compiler/x64/code-generator-x64.cc:1887: kInt64Size);
kInt64Size ==> kPointerSize (here and below)

[Stefano Sanfilippo, 2016-07-06 10:06:09]

The CQ bit was checked by ssanfilippo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-06 10:06:20]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-06 10:31:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-06 10:31:13]

Dry run: This issue passed the CQ dry run.

[Stefano Sanfilippo, 2016-07-06 13:25:10]

Done with the first round of comments, PTAL.

I still have to think a bit about merging the optional logic into the
UnwindingInfoWriter and how to integrate the ZoneVector. Another patchset will
follow ASAP.

https://codereview.chromium.org/2026313002/diff/820001/src/compiler/arm/unwin...
File src/compiler/arm/unwinding-info-writer-arm.cc (right):

https://codereview.chromium.org/2026313002/diff/820001/src/compiler/arm/unwin...
src/compiler/arm/unwinding-info-writer-arm.cc:21:
eh_frame_writer_.RecordRegisterSavedToStack(lr, -4);
On 2016/07/06 07:09:42, Jarin wrote:
> -4 ==> -kPointerSize? (Here and below.)

Done.

https://codereview.chromium.org/2026313002/diff/820001/src/compiler/arm/unwin...
File src/compiler/arm/unwinding-info-writer-arm.h (right):

https://codereview.chromium.org/2026313002/diff/820001/src/compiler/arm/unwin...
src/compiler/arm/unwinding-info-writer-arm.h:47: static_assert(kIntSize >=
kInt32Size, "int too small to encode a RpoNumber");
On 2016/07/06 07:09:42, Jarin wrote:
> Normally, we use STATIC_ASSERT.

I followed the comment at line 216 of src/base/macros.h:

// TODO(all) Replace all uses of this macro with static_assert, remove macro.
#define STATIC_ASSERT(test) static_assert(test, #test)

https://codereview.chromium.org/2026313002/diff/820001/src/compiler/arm64/unw...
File src/compiler/arm64/unwinding-info-writer-arm64.cc (right):

https://codereview.chromium.org/2026313002/diff/820001/src/compiler/arm64/unw...
src/compiler/arm64/unwinding-info-writer-arm64.cc:21:
eh_frame_writer_.RecordRegisterSavedToStack(lr, -8);
On 2016/07/06 07:09:42, Jarin wrote:
> 8 ==> kPointerSize? 

Done.

https://codereview.chromium.org/2026313002/diff/820001/src/compiler/unwinding...
File src/compiler/unwinding-info-writer.h (right):

https://codereview.chromium.org/2026313002/diff/820001/src/compiler/unwinding...
src/compiler/unwinding-info-writer.h:13: #include
"src/compiler/ia32/unwinding-info-writer-ia32.h"
On 2016/07/06 07:09:42, Jarin wrote:
> Instead of including the same unimplemented unwinding info writers here, could
> not you just put the unimplemented version of the class here and only include
> the headers with the real implementations? Or do you intend to implement
> all/most of the remaining architectures soon?

This was initially meant to be a placeholder, but on a second thought it's
better to follow your suggestion, since Linux perf presently appears to only
support x64, arm, arm64 and ia32 anyway.

https://codereview.chromium.org/2026313002/diff/820001/src/compiler/x64/code-...
File src/compiler/x64/code-generator-x64.cc (right):

https://codereview.chromium.org/2026313002/diff/820001/src/compiler/x64/code-...
src/compiler/x64/code-generator-x64.cc:1887: kInt64Size);
On 2016/07/06 07:09:42, Jarin wrote:
> kInt64Size ==> kPointerSize (here and below)

Done.

[Stefano Sanfilippo, 2016-07-06 13:37:20]

Patchset #17 (id:880001) has been deleted

[Stefano Sanfilippo, 2016-07-06 17:45:36]

Patchset #19 (id:940001) has been deleted

[Stefano Sanfilippo, 2016-07-06 17:46:23]

The CQ bit was checked by ssanfilippo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-06 17:46:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-06 18:10:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-06 18:10:10]

Dry run: This issue passed the CQ dry run.

[Stefano Sanfilippo, 2016-07-06 18:45:24]

Patchset #20 (id:980001) has been deleted

[Jarin, 2016-07-07 07:31:40]

So far looking good.

(Since UnwindingInfoWriter does not really need a destructor to be called on it
anymore, you could make it a zone object. That way, you would not have to use
the smart pointer. Obviously, if you fold the conditional logic into
UnwindingInfoWriter, then you can embed it into code generator and not to worry
about allocation at all.)

https://codereview.chromium.org/2026313002/diff/820001/src/compiler/arm/unwin...
File src/compiler/arm/unwinding-info-writer-arm.h (right):

https://codereview.chromium.org/2026313002/diff/820001/src/compiler/arm/unwin...
src/compiler/arm/unwinding-info-writer-arm.h:47: static_assert(kIntSize >=
kInt32Size, "int too small to encode a RpoNumber");
On 2016/07/06 13:25:10, Stefano Sanfilippo wrote:
> On 2016/07/06 07:09:42, Jarin wrote:
> > Normally, we use STATIC_ASSERT.
> 
> I followed the comment at line 216 of src/base/macros.h:
> 
> // TODO(all) Replace all uses of this macro with static_assert, remove macro.
> #define STATIC_ASSERT(test) static_assert(test, #test)

Ah, good to know. Thanks.

[rmcilroy, 2016-07-07 09:55:17]

https://codereview.chromium.org/2026313002/diff/820001/src/compiler/arm/code-...
File src/compiler/arm/code-generator-arm.cc (right):

https://codereview.chromium.org/2026313002/diff/820001/src/compiler/arm/code-...
src/compiler/arm/code-generator-arm.cc:429: if
(!unwinding_info_writer_.is_empty()) {
On 2016/07/06 07:09:42, Jarin wrote:
> I am wondering whether you do not want to fold this is_empty logic into
> UnwindingInforWriter - the writer would internally know whether it is active,
> and it would only do its thing if it is active. That way, you could embed the
> writer into the code generator, so here you would just say
> 
> unwinding_info_writer_.MarkFrameDeconstructed(__ pc_offset());
> 
> without the is_empty check. WDYT?

+1, I think this would make things clearer in the code generators.

[Stefano Sanfilippo, 2016-07-07 12:46:46]

The CQ bit was checked by ssanfilippo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-07 12:46:51]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-07 12:50:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-07 12:50:47]

Dry run: Try jobs failed on following builders:
  v8_linux_arm64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...)
  v8_linux_arm_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/...)

[Stefano Sanfilippo, 2016-07-07 14:47:06]

The CQ bit was checked by ssanfilippo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-07 14:47:17]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Stefano Sanfilippo, 2016-07-07 14:47:48]

Optional logic merged. PTAL.

[commit-bot: I haz the power, 2016-07-07 15:05:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-07 15:05:11]

Dry run: Try jobs failed on following builders:
  v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
  v8_linux64_avx2_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng_trig...)

[Stefano Sanfilippo, 2016-07-07 16:47:07]

Patchset #23 (id:1060001) has been deleted

[Stefano Sanfilippo, 2016-07-07 17:37:42]

The CQ bit was checked by ssanfilippo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-07 17:37:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Stefano Sanfilippo, 2016-07-07 17:42:29]

Patchset #24 (id:1100001) has been deleted

[Stefano Sanfilippo, 2016-07-07 17:42:51]

The CQ bit was checked by ssanfilippo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-07 17:42:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Stefano Sanfilippo, 2016-07-07 17:46:48]

Patchset #24 (id:1120001) has been deleted

[Stefano Sanfilippo, 2016-07-07 17:46:57]

The CQ bit was checked by ssanfilippo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-07 17:47:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-07 18:14:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-07 18:14:59]

Dry run: This issue passed the CQ dry run.

[Stefano Sanfilippo, 2016-07-08 09:31:32]

Patchset #25 (id:1160001) has been deleted

[Stefano Sanfilippo, 2016-07-08 18:18:36]

The CQ bit was checked by ssanfilippo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-08 18:18:45]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-08 19:29:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-08 19:29:49]

Dry run: This issue passed the CQ dry run.

[Stefano Sanfilippo, 2016-07-11 10:03:55]

Patchset #27 (id:1220001) has been deleted

[Jarin, 2016-07-11 18:52:13]

lgtm

[Stefano Sanfilippo, 2016-07-12 11:05:43]

The CQ bit was checked by ssanfilippo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-12 11:05:49]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-12 11:07:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-12 11:07:42]

Dry run: Try jobs failed on following builders:
  v8_linux64_asan_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng/buil...)
  v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
  v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...)
  v8_linux64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/8925)
  v8_linux_arm64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...)
  v8_linux_arm_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/...)
  v8_linux_mipsel_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mipsel_compile_rel...)
  v8_linux_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/8866)
  v8_presubmit on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/19241)

[Stefano Sanfilippo, 2016-07-12 11:08:18]

Patchset #28 (id:1260001) has been deleted

[rmcilroy, 2016-07-13 10:14:52]

I'd still like some simple tests for this if it's possible.

[Stefano Sanfilippo, 2016-07-13 11:24:25]

Patchset #29 (id:1300001) has been deleted

[Stefano Sanfilippo, 2016-07-13 11:26:17]

The CQ bit was checked by ssanfilippo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-13 11:26:22]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-13 11:53:46]

The CQ bit was unchecked by commit-bot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-13 11:53:48]

Dry run: This issue passed the CQ dry run.

[Stefano Sanfilippo, 2016-07-13 14:04:46]

Patchset #31 (id:1360001) has been deleted

[Stefano Sanfilippo, 2016-07-13 14:15:17]

Patchset #31 (id:1380001) has been deleted

[Stefano Sanfilippo, 2016-07-13 14:36:00]

Patchset #30 (id:1340001) has been deleted

[Stefano Sanfilippo, 2016-07-13 14:36:14]

Patchset #30 (id:1400001) has been deleted

[Stefano Sanfilippo, 2016-07-14 10:40:10]

The CQ bit was checked by ssanfilippo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-14 10:40:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-14 11:04:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-14 11:04:36]

Dry run: This issue passed the CQ dry run.

[Stefano Sanfilippo, 2016-07-14 16:35:34]

Patchset #32 (id:1460001) has been deleted

[Stefano Sanfilippo, 2016-07-14 18:49:17]

The CQ bit was checked by ssanfilippo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-14 18:49:22]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-14 19:20:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-14 19:20:18]

Dry run: This issue passed the CQ dry run.

[Stefano Sanfilippo, 2016-07-15 13:42:58]

The CQ bit was checked by ssanfilippo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-15 13:43:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Stefano Sanfilippo, 2016-07-15 13:53:59]

Description was changed from

==========
Make TurboFan emit unwinding information.

This commit introduces support for writing x64, arm and arm64 unwinding 
tables in the .eh_frame format, to be inserted in the jitdump read by 
Linux perf and emitted with FLAG_perf_prof and 
FLAG_perf_prof_unwinding_info enabled.

BUG=v8:4899
LOG=N
==========

to

==========
Emit unwinding information for TurboFan code.

This commit introduces support for writing unwinding tables in the
.eh_frame format, to be inserted in the jitdump read by Linux perf and 
emitted with FLAG_perf_prof and FLAG_perf_prof_unwinding_info enabled.

x64 is fully implemented and tested, arm and arm64 are untested and the
unwinding information needs to be expanded, but the mechanism is ready.

BUG=v8:4899
LOG=N
==========

[Stefano Sanfilippo, 2016-07-15 14:01:38]

On 2016/07/13 10:14:52, rmcilroy wrote:
> I'd still like some simple tests for this if it's possible.

I have added a simple test that will exercise that code and check that the
unwinding info has been written.

I also left some TODOs (assigned to me) for tests I would like to see
implemented.

[commit-bot: I haz the power, 2016-07-15 14:52:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-15 14:52:56]

Dry run: This issue passed the CQ dry run.

[Stefano Sanfilippo, 2016-07-15 15:02:25]

The CQ bit was checked by ssanfilippo@chromium.org

[Stefano Sanfilippo, 2016-07-15 15:02:25]

The patchset sent to the CQ was uploaded after l-g-t-m from jarin@chromium.org
Link to the patchset: https://codereview.chromium.org/2026313002/#ps1520001
(title: "MarkFrameConstructed inside has_frame branch on ARM.")

[commit-bot: I haz the power, 2016-07-15 15:02:35]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-15 15:04:39]

Description was changed from

==========
Emit unwinding information for TurboFan code.

This commit introduces support for writing unwinding tables in the
.eh_frame format, to be inserted in the jitdump read by Linux perf and 
emitted with FLAG_perf_prof and FLAG_perf_prof_unwinding_info enabled.

x64 is fully implemented and tested, arm and arm64 are untested and the
unwinding information needs to be expanded, but the mechanism is ready.

BUG=v8:4899
LOG=N
==========

to

==========
Emit unwinding information for TurboFan code.

This commit introduces support for writing unwinding tables in the
.eh_frame format, to be inserted in the jitdump read by Linux perf and 
emitted with FLAG_perf_prof and FLAG_perf_prof_unwinding_info enabled.

x64 is fully implemented and tested, arm and arm64 are untested and the
unwinding information needs to be expanded, but the mechanism is ready.

BUG=v8:4899
LOG=N
==========

[commit-bot: I haz the power, 2016-07-15 15:04:41]

Committed patchset #34 (id:1520001)

[commit-bot: I haz the power, 2016-07-15 15:06:04]

Description was changed from

==========
Emit unwinding information for TurboFan code.

This commit introduces support for writing unwinding tables in the
.eh_frame format, to be inserted in the jitdump read by Linux perf and 
emitted with FLAG_perf_prof and FLAG_perf_prof_unwinding_info enabled.

x64 is fully implemented and tested, arm and arm64 are untested and the
unwinding information needs to be expanded, but the mechanism is ready.

BUG=v8:4899
LOG=N
==========

to

==========
Emit unwinding information for TurboFan code.

This commit introduces support for writing unwinding tables in the
.eh_frame format, to be inserted in the jitdump read by Linux perf and
emitted with FLAG_perf_prof and FLAG_perf_prof_unwinding_info enabled.

x64 is fully implemented and tested, arm and arm64 are untested and the
unwinding information needs to be expanded, but the mechanism is ready.

BUG=v8:4899
LOG=N

Committed: https://crrev.com/cecded1c24bf3c3409d6556c11af2bd7b0ffea69
Cr-Commit-Position: refs/heads/master@{#37799}
==========

[commit-bot: I haz the power, 2016-07-15 15:06:07]

Patchset 34 (id:??) landed as
https://crrev.com/cecded1c24bf3c3409d6556c11af2bd7b0ffea69
Cr-Commit-Position: refs/heads/master@{#37799}