Emit exception handler table in TurboFan.

Emit exception handler table in TurboFan.

R=bmeurer@chromium.org

Committed: https://crrev.com/88c4f52cb27474989a646d6c9433bbab3e55f8ae
Cr-Commit-Position: refs/heads/master@{#26783}

[Michael Starzinger, 2015-02-19 17:36:54]

First round of comments before I do the ports would be awesome. Thanks!

[Benedikt Meurer, 2015-02-19 18:31:19]

General direction is good.

One question that bugs me: Why don't we embed the handler table into the code
object?

https://codereview.chromium.org/943503003/diff/1/src/compiler/code-generator.cc
File src/compiler/code-generator.cc (right):

https://codereview.chromium.org/943503003/diff/1/src/compiler/code-generator....
src/compiler/code-generator.cc:136: if (handlers_.size() > 0) {
if (!handlers_.empty())

https://codereview.chromium.org/943503003/diff/1/src/compiler/code-generator.h
File src/compiler/code-generator.h (right):

https://codereview.chromium.org/943503003/diff/1/src/compiler/code-generator....
src/compiler/code-generator.h:30: struct HandlerInfo {
I think this should be a private member of CodeGenerator.

https://codereview.chromium.org/943503003/diff/1/src/compiler/ia32/instructio...
File src/compiler/ia32/instruction-selector-ia32.cc (right):

https://codereview.chromium.org/943503003/diff/1/src/compiler/ia32/instructio...
src/compiler/ia32/instruction-selector-ia32.cc:749: CallDescriptor::Flags flags;
how about initializing flags to descriptor->flags() here, and skipping the ORing
below?

https://codereview.chromium.org/943503003/diff/1/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/943503003/diff/1/src/objects.cc#newcode11724
src/objects.cc:11724: os << buf.start();
How about using ios formatting here instead of SNPrintF and temp vector?

[Michael Starzinger, 2015-02-19 18:50:04]

On 2015/02/19 18:31:19, Benedikt Meurer wrote:
> General direction is good.
> 
> One question that bugs me: Why don't we embed the handler table into the code
> object?

Thanks for the quick review, will address the detailed comments tomorrow. To
answer your question: No particular reason, the table is external for
full-codegen already so I just decided to go with an external table for TurboFan
as well without giving it much thought. I don't see a reason we couldn't embed
it into the code object. Maybe as a follow-up once everything is working? WDYT?

[Benedikt Meurer, 2015-02-19 18:51:37]

On 2015/02/19 18:50:04, Michael Starzinger wrote:
> On 2015/02/19 18:31:19, Benedikt Meurer wrote:
> > General direction is good.
> > 
> > One question that bugs me: Why don't we embed the handler table into the
code
> > object?
> 
> Thanks for the quick review, will address the detailed comments tomorrow. To
> answer your question: No particular reason, the table is external for
> full-codegen already so I just decided to go with an external table for
TurboFan
> as well without giving it much thought. I don't see a reason we couldn't embed
> it into the code object. Maybe as a follow-up once everything is working?
WDYT?

Sounds good, thanks.

[Michael Starzinger, 2015-02-20 10:04:54]

Comments addressed. Will do ports next.

https://codereview.chromium.org/943503003/diff/1/src/compiler/code-generator.cc
File src/compiler/code-generator.cc (right):

https://codereview.chromium.org/943503003/diff/1/src/compiler/code-generator....
src/compiler/code-generator.cc:136: if (handlers_.size() > 0) {
On 2015/02/19 18:31:19, Benedikt Meurer wrote:
> if (!handlers_.empty())

Done.

https://codereview.chromium.org/943503003/diff/1/src/compiler/code-generator.h
File src/compiler/code-generator.h (right):

https://codereview.chromium.org/943503003/diff/1/src/compiler/code-generator....
src/compiler/code-generator.h:30: struct HandlerInfo {
On 2015/02/19 18:31:19, Benedikt Meurer wrote:
> I think this should be a private member of CodeGenerator.

Done.

https://codereview.chromium.org/943503003/diff/1/src/compiler/ia32/instructio...
File src/compiler/ia32/instruction-selector-ia32.cc (right):

https://codereview.chromium.org/943503003/diff/1/src/compiler/ia32/instructio...
src/compiler/ia32/instruction-selector-ia32.cc:749: CallDescriptor::Flags flags;
On 2015/02/19 18:31:19, Benedikt Meurer wrote:
> how about initializing flags to descriptor->flags() here, and skipping the
ORing
> below?

Done.

https://codereview.chromium.org/943503003/diff/1/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/943503003/diff/1/src/objects.cc#newcode11724
src/objects.cc:11724: os << buf.start();
On 2015/02/19 18:31:19, Benedikt Meurer wrote:
> How about using ios formatting here instead of SNPrintF and temp vector?

Acknowledged. As discussed offline, we'll do that in one go over the codebase.

[Michael Starzinger, 2015-02-20 11:30:24]

Ports done. PTAL.

[Benedikt Meurer, 2015-02-20 12:09:36]

LGTM

[Michael Starzinger, 2015-02-20 14:52:22]

New patchsets have been uploaded after l-g-t-m from bmeurer@chromium.org

[Michael Starzinger, 2015-02-20 16:20:42]

The CQ bit was checked by mstarzinger@chromium.org

[commit-bot: I haz the power, 2015-02-20 16:21:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/943503003/60001

[commit-bot: I haz the power, 2015-02-20 16:22:45]

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

[commit-bot: I haz the power, 2015-02-20 16:22:46]

Failed to apply the patch.

[commit-bot: I haz the power, 2015-02-20 16:22:47]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-02-20 16:23:10]

Patchset 4 (id:??) landed as
https://crrev.com/88c4f52cb27474989a646d6c9433bbab3e55f8ae
Cr-Commit-Position: refs/heads/master@{#26783}