Skip to content

Commit

Permalink
Fix filters consuming too much data.
Browse files Browse the repository at this point in the history 
We would previously assume that a filter would consume all available
data. This only holds if the filter is attached to a top-level unit, but
in general not if some sub-unit uses a filter. With this patch we
explicitly compute how much data is consumed.

Closes #1652.
  • Loading branch information
@bbannier
bbannier committed Feb 7, 2024
1 parent 041a072 commit e4704de
Show file tree
Hide file tree
Showing 10 changed files with 267 additions and 110 deletions.
27 changes: 20 additions & 7 deletions spicy/toolchain/src/compiler/codegen/parser-builder.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,11 @@ struct ProductionVisitor
if ( unit ) {
pb->guardFeatureCode(*unit->id(), {"supports_filters"}, [&]() {
// If we have a filter attached, we initialize it and change to parse from its output.
auto offset1 =
builder()->addTmp("offset1",
builder::memberCall(builder::begin(builder::deref(state().data)),
"offset", {}));

auto filtered = builder::assign(builder::id("filtered"),
builder::call("spicy_rt::filter_init",
{state().self, state().data, state().cur}));
Expand All @@ -335,15 +340,23 @@ struct ProductionVisitor
pushBuilder(have_filter);

auto args2 = args;
builder()->addLocal("filtered_data", type::ValueReference(type::Stream()),
builder::id("filtered"));
args2[0] = builder::id("filtered_data");
args2[1] = builder::begin(builder::deref(args2[0]));
args2[2] = builder::deref(args2[0]);

auto filtered_data =
builder()->addTmp("filtered_data", type::ValueReference(type::Stream()),
builder::id("filtered"));
args2[0] = filtered_data;
args2[1] = builder::begin(builder::deref(filtered_data));
args2[2] = builder::deref(filtered_data);

builder()->addExpression(builder::memberCall(state().self, id_stage2, args2));

// Assume the filter consumed the full input.
pb->advanceInput(builder::size(state().cur));
auto offset2 =
builder()->addTmp("offset2",
builder::memberCall(builder::begin(builder::deref(state().data)),
"offset", {}));

auto advance = builder::difference(offset2, offset1);
pb->advanceInput(advance);

auto result =
builder::tuple({state().cur, state().lahead, state().lahead_end, state().error});
Expand Down
33 changes: 21 additions & 12 deletions tests/Baseline/spicy.optimization.default-parser-functions/noopt.hlt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,19 +142,22 @@ method method tuple<view<stream>, int<64>, iterator<stream>, optional<hilti::Rec
__error = (*self).__error;
local strong_ref<stream> filtered = Null;

if ( __feat%foo@@P0%supports_filters )
if ( __feat%foo@@P0%supports_filters ) {
local uint<64> __offset1 = begin((*__data)).offset();

if ( filtered = spicy_rt::filter_init(self, __data, __cur) ) {
local value_ref<stream> filtered_data = filtered;
(*self).__parse_foo_P0_stage2(filtered_data, begin((*filtered_data)), (*filtered_data), __trim, __lah, __lahe, __error);
__cur = __cur.advance(|__cur|);
local value_ref<stream> __filtered_data = filtered;
(*self).__parse_foo_P0_stage2(__filtered_data, begin((*__filtered_data)), (*__filtered_data), __trim, __lah, __lahe, __error);
local uint<64> __offset2 = begin((*__data)).offset();
__cur = __cur.advance(__offset2 - __offset1);

if ( __trim )
(*__data).trim(begin(__cur));

__result = (__cur, __lah, __lahe, __error);
}

}


if ( ! filtered )
Expand Down Expand Up @@ -375,19 +378,22 @@ method method tuple<view<stream>, int<64>, iterator<stream>, optional<hilti::Rec
__error = (*self).__error;
local strong_ref<stream> filtered = Null;

if ( __feat%foo@@P1%supports_filters )
if ( __feat%foo@@P1%supports_filters ) {
local uint<64> __offset1 = begin((*__data)).offset();

if ( filtered = spicy_rt::filter_init(self, __data, __cur) ) {
local value_ref<stream> filtered_data = filtered;
(*self).__parse_foo_P1_stage2(filtered_data, begin((*filtered_data)), (*filtered_data), __trim, __lah, __lahe, __error);
__cur = __cur.advance(|__cur|);
local value_ref<stream> __filtered_data = filtered;
(*self).__parse_foo_P1_stage2(__filtered_data, begin((*__filtered_data)), (*__filtered_data), __trim, __lah, __lahe, __error);
local uint<64> __offset2 = begin((*__data)).offset();
__cur = __cur.advance(__offset2 - __offset1);

if ( __trim )
(*__data).trim(begin(__cur));

__result = (__cur, __lah, __lahe, __error);
}

}


if ( ! filtered )
Expand Down Expand Up @@ -610,19 +616,22 @@ method method tuple<view<stream>, int<64>, iterator<stream>, optional<hilti::Rec
__error = (*self).__error;
local strong_ref<stream> filtered = Null;

if ( __feat%foo@@P2%supports_filters )
if ( __feat%foo@@P2%supports_filters ) {
local uint<64> __offset1 = begin((*__data)).offset();

if ( filtered = spicy_rt::filter_init(self, __data, __cur) ) {
local value_ref<stream> filtered_data = filtered;
(*self).__parse_foo_P2_stage2(filtered_data, begin((*filtered_data)), (*filtered_data), __trim, __lah, __lahe, __error);
__cur = __cur.advance(|__cur|);
local value_ref<stream> __filtered_data = filtered;
(*self).__parse_foo_P2_stage2(__filtered_data, begin((*__filtered_data)), (*__filtered_data), __trim, __lah, __lahe, __error);
local uint<64> __offset2 = begin((*__data)).offset();
__cur = __cur.advance(__offset2 - __offset1);

if ( __trim )
(*__data).trim(begin(__cur));

__result = (__cur, __lah, __lahe, __error);
}

}


if ( ! filtered )
Expand Down
77 changes: 49 additions & 28 deletions tests/Baseline/spicy.optimization.feature_requirements/noopt.hlt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,19 +273,22 @@ method method tuple<view<stream>, int<64>, iterator<stream>, optional<hilti::Rec
__error = (*self).__error;
local strong_ref<stream> filtered = Null;

if ( __feat%foo@@X0%supports_filters )
if ( __feat%foo@@X0%supports_filters ) {
local uint<64> __offset1 = begin((*__data)).offset();

if ( filtered = spicy_rt::filter_init(self, __data, __cur) ) {
local value_ref<stream> filtered_data = filtered;
(*self).__parse_foo_X0_stage2(filtered_data, begin((*filtered_data)), (*filtered_data), __trim, __lah, __lahe, __error);
__cur = __cur.advance(|__cur|);
local value_ref<stream> __filtered_data = filtered;
(*self).__parse_foo_X0_stage2(__filtered_data, begin((*__filtered_data)), (*__filtered_data), __trim, __lah, __lahe, __error);
local uint<64> __offset2 = begin((*__data)).offset();
__cur = __cur.advance(__offset2 - __offset1);

if ( __trim )
(*__data).trim(begin(__cur));

__result = (__cur, __lah, __lahe, __error);
}

}


if ( ! filtered )
Expand Down Expand Up @@ -510,19 +513,22 @@ method method tuple<view<stream>, int<64>, iterator<stream>, optional<hilti::Rec
__error = (*self).__error;
local strong_ref<stream> filtered = Null;

if ( __feat%foo@@X1%supports_filters )
if ( __feat%foo@@X1%supports_filters ) {
local uint<64> __offset1 = begin((*__data)).offset();

if ( filtered = spicy_rt::filter_init(self, __data, __cur) ) {
local value_ref<stream> filtered_data = filtered;
(*self).__parse_foo_X1_stage2(filtered_data, begin((*filtered_data)), (*filtered_data), __trim, __lah, __lahe, __error);
__cur = __cur.advance(|__cur|);
local value_ref<stream> __filtered_data = filtered;
(*self).__parse_foo_X1_stage2(__filtered_data, begin((*__filtered_data)), (*__filtered_data), __trim, __lah, __lahe, __error);
local uint<64> __offset2 = begin((*__data)).offset();
__cur = __cur.advance(__offset2 - __offset1);

if ( __trim )
(*__data).trim(begin(__cur));

__result = (__cur, __lah, __lahe, __error);
}

}


if ( ! filtered )
Expand Down Expand Up @@ -743,19 +749,22 @@ method method tuple<view<stream>, int<64>, iterator<stream>, optional<hilti::Rec
__error = (*self).__error;
local strong_ref<stream> filtered = Null;

if ( __feat%foo@@X2%supports_filters )
if ( __feat%foo@@X2%supports_filters ) {
local uint<64> __offset1 = begin((*__data)).offset();

if ( filtered = spicy_rt::filter_init(self, __data, __cur) ) {
local value_ref<stream> filtered_data = filtered;
(*self).__parse_foo_X2_stage2(filtered_data, begin((*filtered_data)), (*filtered_data), __trim, __lah, __lahe, __error);
__cur = __cur.advance(|__cur|);
local value_ref<stream> __filtered_data = filtered;
(*self).__parse_foo_X2_stage2(__filtered_data, begin((*__filtered_data)), (*__filtered_data), __trim, __lah, __lahe, __error);
local uint<64> __offset2 = begin((*__data)).offset();
__cur = __cur.advance(__offset2 - __offset1);

if ( __trim )
(*__data).trim(begin(__cur));

__result = (__cur, __lah, __lahe, __error);
}

}


if ( ! filtered )
Expand Down Expand Up @@ -976,19 +985,22 @@ method method tuple<view<stream>, int<64>, iterator<stream>, optional<hilti::Rec
__error = (*self).__error;
local strong_ref<stream> filtered = Null;

if ( __feat%foo@@X3%supports_filters )
if ( __feat%foo@@X3%supports_filters ) {
local uint<64> __offset1 = begin((*__data)).offset();

if ( filtered = spicy_rt::filter_init(self, __data, __cur) ) {
local value_ref<stream> filtered_data = filtered;
(*self).__parse_foo_X3_stage2(filtered_data, begin((*filtered_data)), (*filtered_data), __trim, __lah, __lahe, __error);
__cur = __cur.advance(|__cur|);
local value_ref<stream> __filtered_data = filtered;
(*self).__parse_foo_X3_stage2(__filtered_data, begin((*__filtered_data)), (*__filtered_data), __trim, __lah, __lahe, __error);
local uint<64> __offset2 = begin((*__data)).offset();
__cur = __cur.advance(__offset2 - __offset1);

if ( __trim )
(*__data).trim(begin(__cur));

__result = (__cur, __lah, __lahe, __error);
}

}


if ( ! filtered )
Expand Down Expand Up @@ -1217,19 +1229,22 @@ method method tuple<view<stream>, int<64>, iterator<stream>, optional<hilti::Rec
__error = (*self).__error;
local strong_ref<stream> filtered = Null;

if ( __feat%foo@@X4%supports_filters )
if ( __feat%foo@@X4%supports_filters ) {
local uint<64> __offset1 = begin((*__data)).offset();

if ( filtered = spicy_rt::filter_init(self, __data, __cur) ) {
local value_ref<stream> filtered_data = filtered;
(*self).__parse_foo_X4_stage2(filtered_data, begin((*filtered_data)), (*filtered_data), __trim, __lah, __lahe, __error);
__cur = __cur.advance(|__cur|);
local value_ref<stream> __filtered_data = filtered;
(*self).__parse_foo_X4_stage2(__filtered_data, begin((*__filtered_data)), (*__filtered_data), __trim, __lah, __lahe, __error);
local uint<64> __offset2 = begin((*__data)).offset();
__cur = __cur.advance(__offset2 - __offset1);

if ( __trim )
(*__data).trim(begin(__cur));

__result = (__cur, __lah, __lahe, __error);
}

}


if ( ! filtered )
Expand Down Expand Up @@ -1462,19 +1477,22 @@ method method tuple<view<stream>, int<64>, iterator<stream>, optional<hilti::Rec
__error = (*self).__error;
local strong_ref<stream> filtered = Null;

if ( __feat%foo@@X5%supports_filters )
if ( __feat%foo@@X5%supports_filters ) {
local uint<64> __offset1 = begin((*__data)).offset();

if ( filtered = spicy_rt::filter_init(self, __data, __cur) ) {
local value_ref<stream> filtered_data = filtered;
(*self).__parse_foo_X5_stage2(filtered_data, begin((*filtered_data)), (*filtered_data), __trim, __lah, __lahe, __error);
__cur = __cur.advance(|__cur|);
local value_ref<stream> __filtered_data = filtered;
(*self).__parse_foo_X5_stage2(__filtered_data, begin((*__filtered_data)), (*__filtered_data), __trim, __lah, __lahe, __error);
local uint<64> __offset2 = begin((*__data)).offset();
__cur = __cur.advance(__offset2 - __offset1);

if ( __trim )
(*__data).trim(begin(__cur));

__result = (__cur, __lah, __lahe, __error);
}

}


if ( ! filtered )
Expand Down Expand Up @@ -1695,19 +1713,22 @@ method method tuple<view<stream>, int<64>, iterator<stream>, optional<hilti::Rec
__error = (*self).__error;
local strong_ref<stream> filtered = Null;

if ( __feat%foo@@X6%supports_filters )
if ( __feat%foo@@X6%supports_filters ) {
local uint<64> __offset1 = begin((*__data)).offset();

if ( filtered = spicy_rt::filter_init(self, __data, __cur) ) {
local value_ref<stream> filtered_data = filtered;
(*self).__parse_foo_X6_stage2(filtered_data, begin((*filtered_data)), (*filtered_data), __trim, __lah, __lahe, __error);
__cur = __cur.advance(|__cur|);
local value_ref<stream> __filtered_data = filtered;
(*self).__parse_foo_X6_stage2(__filtered_data, begin((*__filtered_data)), (*__filtered_data), __trim, __lah, __lahe, __error);
local uint<64> __offset2 = begin((*__data)).offset();
__cur = __cur.advance(__offset2 - __offset1);

if ( __trim )
(*__data).trim(begin(__cur));

__result = (__cur, __lah, __lahe, __error);
}

}


if ( ! filtered )
Expand Down
11 changes: 7 additions & 4 deletions tests/Baseline/spicy.optimization.feature_requirements/opt.hlt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,19 +247,22 @@ method method tuple<view<stream>, int<64>, iterator<stream>, optional<hilti::Rec
(*self).__on_0x25_init();
__error = (*self).__error;
local strong_ref<stream> filtered = Null;

{
local uint<64> __offset1 = begin((*__data)).offset();

if ( filtered = spicy_rt::filter_init(self, __data, __cur) ) {
local value_ref<stream> filtered_data = filtered;
(*self).__parse_foo_X5_stage2(filtered_data, begin((*filtered_data)), (*filtered_data), __trim, __lah, __lahe, __error);
__cur = __cur.advance(|__cur|);
local value_ref<stream> __filtered_data = filtered;
(*self).__parse_foo_X5_stage2(__filtered_data, begin((*__filtered_data)), (*__filtered_data), __trim, __lah, __lahe, __error);
local uint<64> __offset2 = begin((*__data)).offset();
__cur = __cur.advance(__offset2 - __offset1);

if ( __trim )
(*__data).trim(begin(__cur));

__result = (__cur, __lah, __lahe, __error);
}

}


if ( ! filtered )
Expand Down
Loading

0 comments on commit e4704de

Please # to comment.