emit better code for try-finally when function does not throw #7333

WalterBright · 2017-11-18T09:04:29Z

Even if a function is marked nothrow, the exception unwind tables must still be there because a try-catch could still be there that absorbs the exception. This PR marks a function internally if it does not throw and does not have a try-catch. This knowledge is then used to omit building exception unwind tables, and the code gen does not have to assume that an external unwinder is calling the finally blocks. The code emitted (at least for Windows) is substantially less.

dlang-bot · 2017-11-18T09:04:30Z

Thanks for your pull request, @WalterBright!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

andralex · 2017-11-20T18:18:12Z

awesome possum!

andralex · 2017-11-20T18:58:24Z

@WalterBright check the autotester

WalterBright · 2017-11-20T20:40:04Z

Yeah, I know. It's not ready yet. Turned off automerge.

WalterBright · 2017-11-21T06:29:29Z

This has an odd failure on Win32. I get two failures:

Makefile:163: recipe for target 'test_results/compilable/test16031.d.out' failed
gmake[1]: *** [test_results/compilable/test16031.d.out] Error 127
Makefile:201: recipe for target 'start_compilable_tests' failed
gmake: *** [start_compilable_tests] Error 2

and:

Makefile:170: recipe for target 'test_results/fail_compilation/cppeh2.d.out' failed
gmake[1]: *** [test_results/fail_compilation/cppeh2.d.out] Error 127
Makefile:207: recipe for target 'start_fail_compilation_tests' failed
gmake: *** [start_fail_compilation_tests] Error 2
gmake: *** Waiting for unfinished jobs....

In test16031.d it says:

// REQUIRED_ARGS: -fPIC -lib
// PERMUTE_ARGS:
// DISABLED: win32 win64

and in cppeh2.d it says:

// DISABLED: win32 win64

How can the tests fail on Win32 when they are disabled on Win32?

rainers · 2017-11-21T07:53:50Z

How can the tests fail on Win32 when they are disabled on Win32?

The tests are still run even if disabled, but the result is ignored (and shows DISABLED but PASSES if successful). As there is no output for running those two tests (supposed to show up with [DISABLED]), I suspect d_do_test is crashing itself. The unusual return code 127 also hints at an abnormal termination.

WalterBright · 2017-11-21T08:56:43Z

Thanks for the tip. I retrieved and tried running d_do_test:

test>d_do_test fail_compilation cppeh2 d
 ... fail_compilation\cppeh2.d      ()!!! [DISABLED on win32]

I.e. it works normally. Not sure where to go with this now.

WalterBright · 2017-11-21T10:48:42Z

I wound up disabling the optimization for Win32 and filing a bug report for it:
https://issues.dlang.org/show_bug.cgi?id=17997

WalterBright · 2017-11-21T11:18:26Z

@CyberShadow I can't tell if the DAutoTest failure is the fault of this PR or not?

MartinNowak

Otherwise looks good.
Would be great if we could generally improve the non-exception code-path, currently dmd uses a call to jump to the finally handler, which trashes the return prediction.

MartinNowak · 2017-11-21T11:12:26Z

src/ddmd/declaration.h

@@ -523,6 +524,8 @@ class FuncDeclaration : public Declaration
    CompiledCtfeFunction *ctfeCode;     // Compiled code for interpreter
    int inlineNest;                     // !=0 if nested inline
    bool isArrayOp;                     // true if array operation
+    bool eh_none;                       /// true if no exception unwinding is needed


Mmh, that doesn't belong in the frontend, any other place you could store or retrieve this flag?

The flag is per-function, and is detected by the font end. It is later transferred to the backend's symbol for that same function.

MartinNowak · 2017-11-21T11:28:53Z

@CyberShadow I can't tell if the DAutoTest failure is the fault of this PR or not?

Root package dpl-docs references unknown package ddox

It's a temporarily code.dlang.org issue, the dub version used doesn't yet know about https://code-mirror.dlang.io/. Unfortunately DAutoTest still has no retry button, so simply push a new commit to retest.

PetarKirov · 2017-11-21T11:32:39Z

@WalterBright

From the build log, it looks like an intermittent network failure:

.generated/stable_dmd-2.072.2/dmd2/linux/bin64/dub upgrade --missing-only --root=dpl-docs
Upgrading project in /dev/shm/dtest/work/repo/dlang.org/dpl-docs
Selected package ddox 0.15.18 doesn't exist. Using latest matching version instead.
Selected package libev 5.0.0+4.04 doesn't exist. Using latest matching version instead.
Selected package libevent 2.0.2+2.0.16 doesn't exist. Using latest matching version instead.
Selected package experimental_allocator 2.70.0-b1 doesn't exist. Using latest matching version instead.
Selected package libdparse 0.7.0 doesn't exist. Using latest matching version instead.
Selected package memutils 0.4.9 doesn't exist. Using latest matching version instead.
Selected package vibe-d 0.7.31 doesn't exist. Using latest matching version instead.
Selected package hyphenate 1.1.1 doesn't exist. Using latest matching version instead.
Selected package libasync 0.8.3 doesn't exist. Using latest matching version instead.
Root package dpl-docs references unknown package ddox
posix.mak:583: recipe for target 'dpl-docs' failed
make: *** [dpl-docs] Error 2

As clearly ddox is a known package: http://code.dlang.org/packages/ddox. The error message is trying to say that dub has tried to contact the registry in order to find ddox, but it failed and fell back to searching the package on the local computer and after it failed it assumed that such package is not known to exist.

The error message is quite obscure, but has since been improved (along with the addition of registry mirrors to remedy the network failures) in later versions of Dub, but dlang.org's build uses DMD 2.072.2, and hence why the the dub version used is probably quite old.

To fix this, you need to restart the build and at the moment the only way is to rebase the branch of your pull request and force push it.

WalterBright · 2017-11-21T11:39:03Z

Would be great if we could generally improve the non-exception code-path, currently dmd uses a call to jump to the finally handler, which trashes the return prediction.

I know, but this is just one more incremental step to get there. I spent 3 days trying to get just this to work, and it still fails on Win32 for mysterious reasons. Debugging eh code is stupidly hard. Having d_do_test itself fail makes for even worse problems. Most everything has to work in order for that program to work, which is backwards.

I lost at least one day because asserts are disabled in the front end. I need to fix that, too. The asserts had found the problem, but being disabled, things just went merrily onwards and died randomly somewhere else.

WalterBright · 2017-11-21T11:40:22Z

Ok, changed a comment to get the DAutoTest to run again.

WalterBright · 2017-11-21T11:43:34Z

d_do_test.d should be written in Python or something, so the test runner itself does not depend on the compiler ecosystem working. Or maybe just have it built with the last release of the compiler, not the current one under test.

MartinNowak · 2017-11-21T11:44:15Z

I lost at least one day because asserts are disabled in the front end. I need to fix that, too. The asserts had found the problem, but being disabled, things just went merrily onwards and died randomly somewhere else.

Let's give it a try then #7344.

MartinNowak · 2017-11-21T11:45:00Z

Or maybe just have it built with the last release of the compiler, not the current one under test.

That's an idiom that we use everywhere else, a bit more tricky due to the unclear mingw/cygwin environment.

WalterBright · 2017-11-21T11:56:25Z

BTW, getting all these EH optimizations in should substantially reduce the cost of "zero cost" RAII exceptions (haha). I had forgotten how much better code my pre-exceptions C++ compiler generated for RAII.

MartinNowak · 2017-11-21T12:09:00Z

Or maybe just have it built with the last release of the compiler, not the current one under test.

See #7345

WalterBright force-pushed the func-no-eh branch 25 times, most recently from b6a8322 to a995bb9 Compare November 20, 2017 11:27

andralex approved these changes Nov 20, 2017

View reviewed changes

andralex added the auto-merge label Nov 20, 2017

WalterBright force-pushed the func-no-eh branch 2 times, most recently from bace0ee to 8181759 Compare November 21, 2017 05:33

WalterBright force-pushed the func-no-eh branch from 8181759 to 7ab71d4 Compare November 21, 2017 07:49

WalterBright force-pushed the func-no-eh branch 3 times, most recently from cad9c11 to c02fa7b Compare November 21, 2017 10:31

WalterBright added the auto-merge label Nov 21, 2017

MartinNowak reviewed Nov 21, 2017

View reviewed changes

emit better code for try-finally when function does not throw

9327d15

WalterBright force-pushed the func-no-eh branch from c02fa7b to 9327d15 Compare November 21, 2017 11:39

dlang-bot removed the auto-merge label Nov 21, 2017

WalterBright added the auto-merge label Nov 21, 2017

dlang-bot merged commit 00d179f into dlang:master Nov 21, 2017

WalterBright deleted the func-no-eh branch November 21, 2017 12:26

LemonBoy mentioned this pull request Mar 13, 2018

Fix issue 18504 - Generate unwind tables for halting functions #8027

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

emit better code for try-finally when function does not throw #7333

emit better code for try-finally when function does not throw #7333

WalterBright commented Nov 18, 2017

dlang-bot commented Nov 18, 2017

andralex commented Nov 20, 2017

andralex commented Nov 20, 2017

WalterBright commented Nov 20, 2017

WalterBright commented Nov 21, 2017

rainers commented Nov 21, 2017

WalterBright commented Nov 21, 2017 •

edited

WalterBright commented Nov 21, 2017

WalterBright commented Nov 21, 2017

MartinNowak left a comment

MartinNowak Nov 21, 2017

WalterBright Nov 21, 2017

MartinNowak commented Nov 21, 2017

PetarKirov commented Nov 21, 2017 •

edited

WalterBright commented Nov 21, 2017

WalterBright commented Nov 21, 2017

WalterBright commented Nov 21, 2017 •

edited

MartinNowak commented Nov 21, 2017

MartinNowak commented Nov 21, 2017 •

edited

WalterBright commented Nov 21, 2017

MartinNowak commented Nov 21, 2017

emit better code for try-finally when function does not throw #7333

emit better code for try-finally when function does not throw #7333

Conversation

WalterBright commented Nov 18, 2017

dlang-bot commented Nov 18, 2017

Bugzilla references

andralex commented Nov 20, 2017

andralex commented Nov 20, 2017

WalterBright commented Nov 20, 2017

WalterBright commented Nov 21, 2017

rainers commented Nov 21, 2017

WalterBright commented Nov 21, 2017 • edited

WalterBright commented Nov 21, 2017

WalterBright commented Nov 21, 2017

MartinNowak left a comment

Choose a reason for hiding this comment

MartinNowak Nov 21, 2017

Choose a reason for hiding this comment

WalterBright Nov 21, 2017

Choose a reason for hiding this comment

MartinNowak commented Nov 21, 2017

PetarKirov commented Nov 21, 2017 • edited

WalterBright commented Nov 21, 2017

WalterBright commented Nov 21, 2017

WalterBright commented Nov 21, 2017 • edited

MartinNowak commented Nov 21, 2017

MartinNowak commented Nov 21, 2017 • edited

WalterBright commented Nov 21, 2017

MartinNowak commented Nov 21, 2017

WalterBright commented Nov 21, 2017 •

edited

PetarKirov commented Nov 21, 2017 •

edited

WalterBright commented Nov 21, 2017 •

edited

MartinNowak commented Nov 21, 2017 •

edited