Skip to content

Commit

Permalink
Add peephole optimization simplifying rethrowing catch blocks.
Browse files Browse the repository at this point in the history
The optimization turns this:

```
function void foo() {
     try {
        ...
     } catch {
         throw;
     }
 }
```

into

```
function void foo() {
     {
        ...
     }
 }
```

It would be even nicer if we didn't need the braces around the
remaining block, but it's generally not safe to remove them because if
the block declares any locals their life-times and visibility would
change.
  • Loading branch information
rsmmr committed Sep 25, 2024
1 parent 6fdeb10 commit e64facf
Show file tree
Hide file tree
Showing 13 changed files with 76 additions and 34 deletions.
2 changes: 1 addition & 1 deletion hilti/toolchain/include/ast/node-range.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ class Range {
auto begin() const { return const_iterator(_begin); }
auto end() const { return const_iterator(_end); }
size_t size() const { return static_cast<size_t>(_end - _begin); }
const T& front() const { return *_begin; }
T* front() const { return *_begin; }
bool empty() const { return _begin == _end; }

operator NodeVector<T>() const {
Expand Down
16 changes: 16 additions & 0 deletions hilti/toolchain/src/compiler/optimizer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1000,6 +1000,22 @@ struct PeepholeOptimizer : visitor::MutatingPostOrder {
}
}
}

void operator()(statement::Try* n) final {
// If a there's only a single catch block that just rethrows, replace
// the whole try/catch with the block inside.
if ( auto catches = n->catches(); catches.size() == 1 ) {
const auto& catch_ = catches.front();
if ( auto catch_body = catch_->body()->as<statement::Block>(); catch_body->statements().size() == 1 ) {
if ( auto throw_ = catch_body->statements().front()->tryAs<statement::Throw>();
throw_ && ! throw_->expression() ) {
recordChange(n, "replacing rethrowing try/catch with just the block");
replaceNode(n, n->body());
return;
}
}
}
}
};

// This visitor collects requirement attributes in the AST and toggles unused features.
Expand Down
11 changes: 11 additions & 0 deletions tests/Baseline/hilti.optimization.peephole-try-catch/opt.hlt
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
module Test {


public function void foo() {

assert True;

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,8 @@
[debug/optimizer] [<no location>] statement::If "if ( False ) { }" -> null
[debug/optimizer] [<no location>] statement::If "if ( False ) { }" -> null
[debug/optimizer] [<no location>] statement::If "if ( False ) { }" -> null
[debug/optimizer] [<no location>] statement::Try "try { hilti::debugIndent("spicy"); local iterator<stream> __begin = begin(__cur); local strong_ref<stream> filtered = Null; if ( ! filtered ) __result = (*self).__parse_foo__P1_stage2(__data, __begin, __cur, __trim, __lah, __lahe, __error); } catch ( hilti::SystemException __except ) { throw; }" -> replacing rethrowing try/catch with just the block
[debug/optimizer] [<no location>] statement::Try "try { hilti::debugIndent("spicy"); local iterator<stream> __begin = begin(__cur); local strong_ref<stream> filtered = Null; if ( ! filtered ) __result = (*self).__parse_foo__P1_stage2(__data, __begin, __cur, __trim, __lah, __lahe, __error); } catch ( hilti::SystemException __except ) { throw; }" -> statement::Block "{ hilti::debugIndent("spicy"); local iterator<stream> __begin = begin(__cur); local strong_ref<stream> filtered = Null; if ( ! filtered ) __result = (*self).__parse_foo__P1_stage2(__data, __begin, __cur, __trim, __lah, __lahe, __error); }"
[debug/optimizer] [default-parser-functions.spicy:10:1-21:2] declaration::Constant "const bool __feat%foo@@P0%supports_filters = False;" -> disabled feature 'supports_filters' of type 'foo::P0' since it is not used
[debug/optimizer] [default-parser-functions.spicy:10:1-21:2] declaration::Constant "const bool __feat%foo@@P0%supports_sinks = False;" -> disabled feature 'supports_sinks' of type 'foo::P0' since it is not used
[debug/optimizer] [default-parser-functions.spicy:10:1-21:2] declaration::Constant "const bool __feat%foo@@P0%synchronization = False;" -> disabled feature 'synchronization' of type 'foo::P0' since it is not used
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ init function void __register_foo_P0() {
method method tuple<const view<stream>, int<64>, const iterator<stream>, optional<hilti::RecoverableFailure>> foo::P1::__parse_stage1(inout value_ref<stream> __data, iterator<stream> __begin, copy view<stream> __cur, copy bool __trim, copy int<64> __lah, copy iterator<stream> __lahe, copy optional<hilti::RecoverableFailure> __error) {
# "<...>/default-parser-functions.spicy:14:18-14:24"
local tuple<view<stream>, int<64>, const iterator<stream>, optional<hilti::RecoverableFailure>> __result;
try {
{
hilti::debugIndent("spicy");
local iterator<stream> __begin = begin(__cur);
local strong_ref<stream> filtered = Null;
Expand All @@ -67,8 +67,6 @@ method method tuple<const view<stream>, int<64>, const iterator<stream>, optiona
__result = (*self).__parse_foo__P1_stage2(__data, __begin, __cur, __trim, __lah, __lahe, __error);

}
catch ( hilti::SystemException __except )
throw;

return __result;
}
Expand Down
6 changes: 6 additions & 0 deletions tests/Baseline/spicy.optimization.unused-functions/log
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,12 @@
[debug/optimizer] [<no location>] statement::If "if ( False ) { }" -> null
[debug/optimizer] [<no location>] statement::If "if ( False ) { }" -> null
[debug/optimizer] [<no location>] statement::If "if ( False ) { }" -> null
[debug/optimizer] [<no location>] statement::Try "try { hilti::debugIndent("spicy"); local iterator<stream> __begin = begin(__cur); local strong_ref<stream> filtered = Null; if ( ! filtered ) __result = (*self).__parse_foo__B_stage2(__data, __begin, __cur, __trim, __lah, __lahe, __error); } catch ( hilti::SystemException __except ) { throw; }" -> replacing rethrowing try/catch with just the block
[debug/optimizer] [<no location>] statement::Try "try { hilti::debugIndent("spicy"); local iterator<stream> __begin = begin(__cur); local strong_ref<stream> filtered = Null; if ( ! filtered ) __result = (*self).__parse_foo__B_stage2(__data, __begin, __cur, __trim, __lah, __lahe, __error); } catch ( hilti::SystemException __except ) { throw; }" -> statement::Block "{ hilti::debugIndent("spicy"); local iterator<stream> __begin = begin(__cur); local strong_ref<stream> filtered = Null; if ( ! filtered ) __result = (*self).__parse_foo__B_stage2(__data, __begin, __cur, __trim, __lah, __lahe, __error); }"
[debug/optimizer] [<no location>] statement::Try "try { hilti::debugIndent("spicy"); local iterator<stream> __begin = begin(__cur); local strong_ref<stream> filtered = Null; if ( ! filtered ) __result = (*self).__parse_foo__C_stage2(__data, __begin, __cur, __trim, __lah, __lahe, __error); } catch ( hilti::SystemException __except ) { throw; }" -> replacing rethrowing try/catch with just the block
[debug/optimizer] [<no location>] statement::Try "try { hilti::debugIndent("spicy"); local iterator<stream> __begin = begin(__cur); local strong_ref<stream> filtered = Null; if ( ! filtered ) __result = (*self).__parse_foo__C_stage2(__data, __begin, __cur, __trim, __lah, __lahe, __error); } catch ( hilti::SystemException __except ) { throw; }" -> statement::Block "{ hilti::debugIndent("spicy"); local iterator<stream> __begin = begin(__cur); local strong_ref<stream> filtered = Null; if ( ! filtered ) __result = (*self).__parse_foo__C_stage2(__data, __begin, __cur, __trim, __lah, __lahe, __error); }"
[debug/optimizer] [<no location>] statement::Try "try { hilti::debugIndent("spicy"); local iterator<stream> __begin = begin(__cur); local strong_ref<stream> filtered = Null; if ( ! filtered ) __result = (*self).__parse_foo__D_stage2(__data, __begin, __cur, __trim, __lah, __lahe, __error); } catch ( hilti::SystemException __except ) { throw; }" -> replacing rethrowing try/catch with just the block
[debug/optimizer] [<no location>] statement::Try "try { hilti::debugIndent("spicy"); local iterator<stream> __begin = begin(__cur); local strong_ref<stream> filtered = Null; if ( ! filtered ) __result = (*self).__parse_foo__D_stage2(__data, __begin, __cur, __trim, __lah, __lahe, __error); } catch ( hilti::SystemException __except ) { throw; }" -> statement::Block "{ hilti::debugIndent("spicy"); local iterator<stream> __begin = begin(__cur); local strong_ref<stream> filtered = Null; if ( ! filtered ) __result = (*self).__parse_foo__D_stage2(__data, __begin, __cur, __trim, __lah, __lahe, __error); }"
[debug/optimizer] [spicy_rt.hlt:28:5-28:76] declaration::Field "method void connect_mime_type(string mime_type, string scope) &internal;" -> null (removing unused member)
[debug/optimizer] [spicy_rt.hlt:29:5-29:75] declaration::Field "method void connect_mime_type(bytes mime_type, string scope) &internal;" -> null (removing unused member)
[debug/optimizer] [unused-functions.spicy:10:1-32:2] declaration::Constant "const bool __feat%foo@@A%supports_filters = False;" -> disabled feature 'supports_filters' of type 'foo::A' since it is not used
Expand Down
12 changes: 3 additions & 9 deletions tests/Baseline/spicy.optimization.unused-functions/opt.hlt
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ init function void __register_foo_A() {
method method tuple<const view<stream>, int<64>, const iterator<stream>, optional<hilti::RecoverableFailure>> foo::B::__parse_stage1(inout value_ref<stream> __data, iterator<stream> __begin, copy view<stream> __cur, copy bool __trim, copy int<64> __lah, copy iterator<stream> __lahe, copy optional<hilti::RecoverableFailure> __error) {
# "<...>/unused-functions.spicy:21:17-21:23"
local tuple<view<stream>, int<64>, const iterator<stream>, optional<hilti::RecoverableFailure>> __result;
try {
{
hilti::debugIndent("spicy");
local iterator<stream> __begin = begin(__cur);
local strong_ref<stream> filtered = Null;
Expand All @@ -88,8 +88,6 @@ method method tuple<const view<stream>, int<64>, const iterator<stream>, optiona
__result = (*self).__parse_foo__B_stage2(__data, __begin, __cur, __trim, __lah, __lahe, __error);

}
catch ( hilti::SystemException __except )
throw;

return __result;
}
Expand Down Expand Up @@ -167,7 +165,7 @@ init function void __register_foo_B() {
method method tuple<const view<stream>, int<64>, const iterator<stream>, optional<hilti::RecoverableFailure>> foo::C::__parse_stage1(inout value_ref<stream> __data, iterator<stream> __begin, copy view<stream> __cur, copy bool __trim, copy int<64> __lah, copy iterator<stream> __lahe, copy optional<hilti::RecoverableFailure> __error) {
# "<...>/unused-functions.spicy:24:10-24:16"
local tuple<view<stream>, int<64>, const iterator<stream>, optional<hilti::RecoverableFailure>> __result;
try {
{
hilti::debugIndent("spicy");
local iterator<stream> __begin = begin(__cur);
local strong_ref<stream> filtered = Null;
Expand All @@ -176,8 +174,6 @@ method method tuple<const view<stream>, int<64>, const iterator<stream>, optiona
__result = (*self).__parse_foo__C_stage2(__data, __begin, __cur, __trim, __lah, __lahe, __error);

}
catch ( hilti::SystemException __except )
throw;

return __result;
}
Expand All @@ -196,7 +192,7 @@ init function void __register_foo_C() {
method method tuple<const view<stream>, int<64>, const iterator<stream>, optional<hilti::RecoverableFailure>> foo::D::__parse_stage1(inout value_ref<stream> __data, iterator<stream> __begin, copy view<stream> __cur, copy bool __trim, copy int<64> __lah, copy iterator<stream> __lahe, copy optional<hilti::RecoverableFailure> __error) {
# "<...>/unused-functions.spicy:25:17-27:1"
local tuple<view<stream>, int<64>, const iterator<stream>, optional<hilti::RecoverableFailure>> __result;
try {
{
hilti::debugIndent("spicy");
local iterator<stream> __begin = begin(__cur);
local strong_ref<stream> filtered = Null;
Expand All @@ -205,8 +201,6 @@ method method tuple<const view<stream>, int<64>, const iterator<stream>, optiona
__result = (*self).__parse_foo__D_stage2(__data, __begin, __cur, __trim, __lah, __lahe, __error);

}
catch ( hilti::SystemException __except )
throw;

return __result;
}
Expand Down
10 changes: 10 additions & 0 deletions tests/Baseline/spicy.optimization.unused-types/log
Original file line number Diff line number Diff line change
Expand Up @@ -788,6 +788,16 @@
[debug/optimizer] [<no location>] statement::If "if ( False ) { }" -> null
[debug/optimizer] [<no location>] statement::If "if ( False ) { }" -> null
[debug/optimizer] [<no location>] statement::If "if ( False ) { }" -> null
[debug/optimizer] [<no location>] statement::Try "try { hilti::debugIndent("spicy"); local iterator<stream> __begin = begin(__cur); local strong_ref<stream> filtered = Null; if ( ! filtered ) __result = (*self).__parse_foo__Priv10_stage2(__data, __begin, __cur, __trim, __lah, __lahe, __error); } catch ( hilti::SystemException __except ) { throw; }" -> replacing rethrowing try/catch with just the block
[debug/optimizer] [<no location>] statement::Try "try { hilti::debugIndent("spicy"); local iterator<stream> __begin = begin(__cur); local strong_ref<stream> filtered = Null; if ( ! filtered ) __result = (*self).__parse_foo__Priv10_stage2(__data, __begin, __cur, __trim, __lah, __lahe, __error); } catch ( hilti::SystemException __except ) { throw; }" -> statement::Block "{ hilti::debugIndent("spicy"); local iterator<stream> __begin = begin(__cur); local strong_ref<stream> filtered = Null; if ( ! filtered ) __result = (*self).__parse_foo__Priv10_stage2(__data, __begin, __cur, __trim, __lah, __lahe, __error); }"
[debug/optimizer] [<no location>] statement::Try "try { hilti::debugIndent("spicy"); local iterator<stream> __begin = begin(__cur); local strong_ref<stream> filtered = Null; if ( ! filtered ) __result = (*self).__parse_foo__Priv5_stage2(__data, __begin, __cur, __trim, __lah, __lahe, __error); } catch ( hilti::SystemException __except ) { throw; }" -> replacing rethrowing try/catch with just the block
[debug/optimizer] [<no location>] statement::Try "try { hilti::debugIndent("spicy"); local iterator<stream> __begin = begin(__cur); local strong_ref<stream> filtered = Null; if ( ! filtered ) __result = (*self).__parse_foo__Priv5_stage2(__data, __begin, __cur, __trim, __lah, __lahe, __error); } catch ( hilti::SystemException __except ) { throw; }" -> statement::Block "{ hilti::debugIndent("spicy"); local iterator<stream> __begin = begin(__cur); local strong_ref<stream> filtered = Null; if ( ! filtered ) __result = (*self).__parse_foo__Priv5_stage2(__data, __begin, __cur, __trim, __lah, __lahe, __error); }"
[debug/optimizer] [<no location>] statement::Try "try { hilti::debugIndent("spicy"); local iterator<stream> __begin = begin(__cur); local strong_ref<stream> filtered = Null; if ( ! filtered ) __result = (*self).__parse_foo__Priv6_stage2(__data, __begin, __cur, __trim, __lah, __lahe, __error); } catch ( hilti::SystemException __except ) { throw; }" -> replacing rethrowing try/catch with just the block
[debug/optimizer] [<no location>] statement::Try "try { hilti::debugIndent("spicy"); local iterator<stream> __begin = begin(__cur); local strong_ref<stream> filtered = Null; if ( ! filtered ) __result = (*self).__parse_foo__Priv6_stage2(__data, __begin, __cur, __trim, __lah, __lahe, __error); } catch ( hilti::SystemException __except ) { throw; }" -> statement::Block "{ hilti::debugIndent("spicy"); local iterator<stream> __begin = begin(__cur); local strong_ref<stream> filtered = Null; if ( ! filtered ) __result = (*self).__parse_foo__Priv6_stage2(__data, __begin, __cur, __trim, __lah, __lahe, __error); }"
[debug/optimizer] [<no location>] statement::Try "try { hilti::debugIndent("spicy"); local iterator<stream> __begin = begin(__cur); local strong_ref<stream> filtered = Null; if ( ! filtered ) __result = (*self).__parse_foo__Pub2_stage2(__data, __begin, __cur, __trim, __lah, __lahe, __error); } catch ( hilti::SystemException __except ) { throw; }" -> replacing rethrowing try/catch with just the block
[debug/optimizer] [<no location>] statement::Try "try { hilti::debugIndent("spicy"); local iterator<stream> __begin = begin(__cur); local strong_ref<stream> filtered = Null; if ( ! filtered ) __result = (*self).__parse_foo__Pub2_stage2(__data, __begin, __cur, __trim, __lah, __lahe, __error); } catch ( hilti::SystemException __except ) { throw; }" -> statement::Block "{ hilti::debugIndent("spicy"); local iterator<stream> __begin = begin(__cur); local strong_ref<stream> filtered = Null; if ( ! filtered ) __result = (*self).__parse_foo__Pub2_stage2(__data, __begin, __cur, __trim, __lah, __lahe, __error); }"
[debug/optimizer] [<no location>] statement::Try "try { hilti::debugIndent("spicy"); local iterator<stream> __begin = begin(__cur); local strong_ref<stream> filtered = Null; if ( ! filtered ) __result = (*self).__parse_foo__Pub3_stage2(__data, __begin, __cur, __trim, __lah, __lahe, __error); } catch ( hilti::SystemException __except ) { throw; }" -> replacing rethrowing try/catch with just the block
[debug/optimizer] [<no location>] statement::Try "try { hilti::debugIndent("spicy"); local iterator<stream> __begin = begin(__cur); local strong_ref<stream> filtered = Null; if ( ! filtered ) __result = (*self).__parse_foo__Pub3_stage2(__data, __begin, __cur, __trim, __lah, __lahe, __error); } catch ( hilti::SystemException __except ) { throw; }" -> statement::Block "{ hilti::debugIndent("spicy"); local iterator<stream> __begin = begin(__cur); local strong_ref<stream> filtered = Null; if ( ! filtered ) __result = (*self).__parse_foo__Pub3_stage2(__data, __begin, __cur, __trim, __lah, __lahe, __error); }"
[debug/optimizer] [spicy_rt.hlt:28:5-28:76] declaration::Field "method void connect_mime_type(string mime_type, string scope) &internal;" -> null (removing unused member)
[debug/optimizer] [spicy_rt.hlt:29:5-29:75] declaration::Field "method void connect_mime_type(bytes mime_type, string scope) &internal;" -> null (removing unused member)
[debug/optimizer] [unused-types.spicy:10:1-53:30] declaration::Constant "const bool __feat%foo@@Priv1%supports_filters = False;" -> disabled feature 'supports_filters' of type 'foo::Priv1' since it is not used
Expand Down
Loading

0 comments on commit e64facf

Please sign in to comment.