Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[WIP] Apply &on-heap selectively #1781

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

[WIP] Apply &on-heap selectively #1781

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b937fd1
Let optional's deref operator pass through "LHSideness".
rsmmr Jul 8, 2024
90d6e8a
Prepare for moving parser temporaries to the stack.
rsmmr Jul 8, 2024
d93f6fd
Apply `&on-heap` selectively only where needed.
rsmmr Jul 5, 2024
35aa3c0
Change a `set` to an `unordered_set`.
rsmmr Jul 8, 2024
1acdf78
Attempt to reduce stack size again.
rsmmr Jul 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion hilti/toolchain/src/ast/operators/optional.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@ class Deref : public Operator {
}

QualifiedType* result(Builder* builder, const Expressions& operands, const Meta& meta) const final {
return operands[0]->type()->type()->as<type::Optional>()->dereferencedType();
if ( auto t = operands[0]->type()->type()->as<type::Optional>()->dereferencedType(); t->side() == Side::LHS )
return t->recreateAsLhs(builder->context());
else
return t;
}

HILTI_OPERATOR(hilti, optional::Deref)
Expand Down
48 changes: 46 additions & 2 deletions spicy/toolchain/src/compiler/codegen/codegen.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ namespace spicy::detail::codegen {

// Information collected from the AST in an initial pass for any code generation.
struct ASTInfo {
std::set<ID> uses_sync_advance; // type ID of units implementing %sync_advance
std::unordered_set<ID> uses_sync_advance; // type ID of units implementing %sync_advance
std::unordered_set<ID>
units_with_references; // type IDs if any unit types that are wrapped into a reference somewhere
};

} // namespace spicy::detail::codegen
Expand Down Expand Up @@ -69,6 +71,31 @@ struct VisitorASTInfo : public visitor::PreOrder {
info->uses_sync_advance.insert(unit->typeID());
}
}

void operator()(hilti::type::StrongReference* n) final {
if ( auto t = n->dereferencedType()->type(); t->isA<type::Unit>() )
info->units_with_references.insert(t->canonicalID());
}

void operator()(hilti::type::ValueReference* n) final {
if ( auto t = n->dereferencedType()->type(); t->isA<type::Unit>() )
info->units_with_references.insert(t->canonicalID());
}

void operator()(hilti::type::WeakReference* n) final {
if ( auto t = n->dereferencedType()->type(); t->isA<type::Unit>() )
info->units_with_references.insert(t->canonicalID());
}

void operator()(hilti::declaration::Parameter* n) final {
if ( n->kind() == hilti::parameter::Kind::InOut ) {
if ( auto t = n->type()->type(); t->isA<type::Unit>() )
// For historical reasons, `inout` unit parameters are expected
// to be wrapped into a reference, so mark them as such so that
// they will gain a `value_ref` wrapping.
info->units_with_references.insert(t->canonicalID());
}
}
};

// Visitor that runs over each module's AST at the beginning of their
Expand Down Expand Up @@ -125,7 +152,6 @@ struct VisitorPass1 : public visitor::MutatingPostOrder {
auto qstruct = builder()->qualifiedType(struct_, n->type()->constness());

n->setType(context(), qstruct);
n->addAttribute(context(), builder()->attribute("&on-heap"));

if ( info->uses_sync_advance.find(u->typeID()) != info->uses_sync_advance.end() )
// Unit has an implementation of `%sync_advance`, so add feature
Expand All @@ -136,6 +162,24 @@ struct VisitorPass1 : public visitor::MutatingPostOrder {

cg->recordTypeMapping(u, struct_);

// Add &on-hep attribute to struct types that need it for correct functioning.
bool add_on_heap = false;

if ( info->units_with_references.find(n->canonicalID()) != info->units_with_references.end() )
// Add &on-heap attribute to units types that are wrapped into a reference anywhere.
add_on_heap = true;

auto unit_decl = u->typeDeclaration();
const auto& decl = context()->dependentDeclarations(unit_decl);
if ( auto x = std::find(decl.begin(), decl.end(), unit_decl); x != decl.end() )
// Add &on-heap attribute to units types that are wrapped into a reference anywhere.
add_on_heap = true;

if ( add_on_heap ) {
recordChange(hilti::util::fmt("marking struct type %s as %%on-heap", n->canonicalID()));
n->attributes()->add(context(), builder()->attribute("&on-heap"));
}

recordChange(n, "replaced unit type with struct");
}

Expand Down
71 changes: 44 additions & 27 deletions spicy/toolchain/src/compiler/codegen/parser-builder.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ struct ProductionVisitor : public production::Visitor {
const Grammar& grammar;
hilti::util::Cache<std::string, ID> parse_functions;
std::vector<hilti::declaration::Field*> new_fields;
Expressions _destinations;
std::vector<std::pair<Expression*, Expression*>>
_destinations; // .first is the value, .second is target for assignment
std::vector<ID> _path; // paths of IDs followed to get to current unit/field

auto cg() { return pb->cg(); }
Expand All @@ -90,13 +91,25 @@ struct ProductionVisitor : public production::Visitor {
auto pushBuilder(std::shared_ptr<Builder> b) { return pb->pushBuilder(std::move(b)); }
auto popBuilder() { return pb->popBuilder(); }

auto destination() { return _destinations.back(); }
// Return the current destination from the top of the stack. If
// `assign_target` is true, returns a value that can be assigned to;
// otherwise a value that can be read or modified.
auto destination(bool assign_target = false) {
return assign_target ? _destinations.back().second : _destinations.back().first;
}

// Pushes a new destination onto the stack. If assign_target is set, a
// separate target is recorded for assigning values. If not set, the same
// expression is used for reading/modification and assignment.
auto pushDestination(Expression* e, Expression* assign_target = nullptr) {
if ( ! assign_target )
assign_target = e;

auto pushDestination(Expression* e) {
HILTI_DEBUG(spicy::logging::debug::ParserBuilder, fmt("- push destination: %s", *e));
_destinations.emplace_back(e);
HILTI_DEBUG(spicy::logging::debug::ParserBuilder, fmt("- push destination: %s / %s", *e, assign_target));
_destinations.emplace_back(e, assign_target);
}

// Pops the most recent destination from the stack.
auto popDestination() {
auto back = _destinations.back();
_destinations.pop_back();
Expand All @@ -107,7 +120,17 @@ struct ProductionVisitor : public production::Visitor {
else
HILTI_DEBUG(spicy::logging::debug::ParserBuilder, fmt("- pop destination, now: none"));

return back;
return back.first;
}

// Creates a new temporary variable of a give type and pushes that onto the
// destination stack. TODO: Update comment.
void pushTmpDestination(const std::string& prefix, QualifiedType* t) {
if ( t->type()->isA<type::Unit>() )
t = builder()->qualifiedType(builder()->typeValueReference(t), t->constness());

auto dst = builder()->addTmp(prefix, t);
pushDestination(dst);
}

// RAII helper to update the visitor's `_path` as we descend the parse tree.
Expand Down Expand Up @@ -516,7 +539,7 @@ struct ProductionVisitor : public production::Visitor {
state().lahead, state().lahead_end, state().error};

if ( ! unit && p.meta().field() )
args.push_back(destination());
args.push_back(destination(true));

auto call = builder()->memberCall(state().self, id, args);
builder()->addAssign(builder()->tuple({state().cur, state().lahead, state().lahead_end, state().error}), call);
Expand Down Expand Up @@ -545,11 +568,8 @@ struct ProductionVisitor : public production::Visitor {

// Push destination for parsed value onto stack.

if ( auto c = meta.container() ) {
auto etype = c->parseType()->type()->elementType();
auto container_element = builder()->addTmp("elem", etype);
pushDestination(container_element);
}
if ( auto c = meta.container() )
pushTmpDestination("elem", c->parseType()->type()->elementType());

else if ( ! meta.isFieldProduction() )
pushDestination(destination());
Expand All @@ -558,18 +578,15 @@ struct ProductionVisitor : public production::Visitor {
// No value to store.
pushDestination(builder()->void_());

else if ( field->isForwarding() ) {
else if ( field->isForwarding() )
// No need for a new destination, but we need to initialize the one
// we have.
builder()->addAssign(destination(), builder()->default_(field->itemType()->type()));
}
builder()->addAssign(destination(true), builder()->default_(field->itemType()->type()));

else if ( (field->isAnonymous() || field->isSkip()) &&
! field->itemType()->type()->isA<hilti::type::Bitfield>() ) {
! field->itemType()->type()->isA<hilti::type::Bitfield>() )
// We won't have a field to store the value in, create a temporary.
auto dst = builder()->addTmp(fmt("transient_%s", field->id()), field->itemType());
pushDestination(dst);
}
pushTmpDestination(fmt("transient_%s", field->id()), field->itemType());

else {
// Can store parsed value directly in struct field.
Expand Down Expand Up @@ -619,7 +636,7 @@ struct ProductionVisitor : public production::Visitor {
if ( meta.field() && ! meta.field()->isSkip() ) {
Expression* default_ =
builder()->default_(builder()->typeName(unit->unitType()->typeID()), type_args, location);
builder()->addAssign(destination(), default_);
builder()->addAssign(destination(true), default_);
}

auto call = builder()->memberCall(destination(), "__parse_stage1", args);
Expand Down Expand Up @@ -700,11 +717,9 @@ struct ProductionVisitor : public production::Visitor {
builder()->member(state().self, "__offset"),
builder()->integer(0)));

if ( field && field->convertExpression() ) {
if ( field && field->convertExpression() )
// Need an additional temporary for the parsed field.
auto dst = builder()->addTmp(fmt("parsed_%s", field->id()), field->parseType());
pushDestination(dst);
}
pushTmpDestination(fmt("parsed_%s", field->id()), field->parseType());

pb->enableDefaultNewValueForField(true);

Expand Down Expand Up @@ -851,7 +866,7 @@ struct ProductionVisitor : public production::Visitor {
// Value was stored in temporary. Apply expression and store result
// at destination.
popDestination();
pb->applyConvertExpression(*field, val, destination());
pb->applyConvertExpression(*field, val, destination(true));
}

popState(); // From &size (pushed even if absent).
Expand Down Expand Up @@ -1722,7 +1737,7 @@ struct ProductionVisitor : public production::Visitor {
}

void operator()(const production::Ctor* p) final {
pb->parseLiteral(*p, destination());
pb->parseLiteral(*p, destination(true));
pb->trimInput();
}

Expand Down Expand Up @@ -1880,7 +1895,9 @@ struct ProductionVisitor : public production::Visitor {
popBuilder();
}

void operator()(const production::Variable* p) final { pb->parseType(p->type()->type(), p->meta(), destination()); }
void operator()(const production::Variable* p) final {
pb->parseType(p->type()->type(), p->meta(), destination(true));
}

void operator()(const production::While* p) final {
if ( p->expression() )
Expand Down
2 changes: 1 addition & 1 deletion tests/Baseline/spicy.optimization.default-parser-functions/log
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@
[debug/optimizer] [default-parser-functions.spicy:10:1-21:2] declaration::Constant "const bool __feat%foo@@P2%uses_random_access = False;" -> disabled feature 'uses_random_access' of type 'foo::P2' since it is not used
[debug/optimizer] [default-parser-functions.spicy:10:1-21:2] declaration::Constant "const bool __feat%foo@@P2%uses_stream = False;" -> disabled feature 'uses_stream' of type 'foo::P2' since it is not used
[debug/optimizer] [default-parser-functions.spicy:10:1-21:2] declaration::Constant "const bool __feat%foo@@P2%uses_sync_advance = False;" -> disabled feature 'uses_sync_advance' of type 'foo::P2' since it is not used
[debug/optimizer] [default-parser-functions.spicy:12:1-12:18] declaration::Type "type P0 = struct { optional<hilti::RecoverableFailure> __error &always-emit &internal; method tuple<const view<stream>, int<64>, const iterator<stream>, optional<hilti::RecoverableFailure>> __parse_foo__P0_stage2(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); } &on-heap;" -> null (removing unused type)
[debug/optimizer] [default-parser-functions.spicy:12:1-12:18] declaration::Type "type P0 = struct { optional<hilti::RecoverableFailure> __error &always-emit &internal; method tuple<const view<stream>, int<64>, const iterator<stream>, optional<hilti::RecoverableFailure>> __parse_foo__P0_stage2(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); };" -> null (removing unused type)
[debug/optimizer] [default-parser-functions.spicy:12:11-12:17] declaration::Field "hook optional<string> __str__();" -> null
[debug/optimizer] [default-parser-functions.spicy:12:11-12:17] declaration::Field "hook void __on_0x25_confirmed() &needed-by-feature="synchronization";" -> null
[debug/optimizer] [default-parser-functions.spicy:12:11-12:17] declaration::Field "hook void __on_0x25_done();" -> null
Expand Down
6 changes: 3 additions & 3 deletions tests/Baseline/spicy.optimization.default-parser-functions/noopt.hlt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ type P0 = struct {
method view<stream> parse2(inout value_ref<P0> __unit, inout value_ref<stream> __data, optional<view<stream>> __cur = Null, optional<spicy_rt::UnitContext> __context) &needed-by-feature="is_filter" &needed-by-feature="supports_sinks" &static;
method view<stream> parse3(inout value_ref<spicy_rt::ParsedUnit> __gunit, inout value_ref<stream> __data, optional<view<stream>> __cur = Null, optional<spicy_rt::UnitContext> __context) &needed-by-feature="is_filter" &needed-by-feature="supports_sinks" &static;
method tuple<const view<stream>, int<64>, const iterator<stream>, optional<hilti::RecoverableFailure>> __parse_foo__P0_stage2(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);
} &on-heap;
};
public type P1 = struct {
weak_ref<const stream> __stream &internal &needed-by-feature="uses_stream";
iterator<stream> __begin &internal &needed-by-feature="uses_random_access";
Expand Down Expand Up @@ -59,7 +59,7 @@ public type P1 = struct {
method view<stream> parse2(inout value_ref<P1> __unit, inout value_ref<stream> __data, optional<view<stream>> __cur = Null, optional<spicy_rt::UnitContext> __context) &needed-by-feature="is_filter" &needed-by-feature="supports_sinks" &static;
method view<stream> parse3(inout value_ref<spicy_rt::ParsedUnit> __gunit, inout value_ref<stream> __data, optional<view<stream>> __cur = Null, optional<spicy_rt::UnitContext> __context) &needed-by-feature="is_filter" &needed-by-feature="supports_sinks" &static;
method tuple<const view<stream>, int<64>, const iterator<stream>, optional<hilti::RecoverableFailure>> __parse_foo__P1_stage2(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);
} &on-heap;
};
public type P2 = struct {
uint<8> x &optional;
uint<8> y &optional;
Expand Down Expand Up @@ -91,7 +91,7 @@ public type P2 = struct {
method view<stream> parse2(inout value_ref<P2> __unit, inout value_ref<stream> __data, optional<view<stream>> __cur = Null, optional<spicy_rt::UnitContext> __context) &needed-by-feature="is_filter" &needed-by-feature="supports_sinks" &static;
method view<stream> parse3(inout value_ref<spicy_rt::ParsedUnit> __gunit, inout value_ref<stream> __data, optional<view<stream>> __cur = Null, optional<spicy_rt::UnitContext> __context) &needed-by-feature="is_filter" &needed-by-feature="supports_sinks" &static;
method tuple<const view<stream>, int<64>, const iterator<stream>, optional<hilti::RecoverableFailure>> __parse_foo__P2_stage2(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);
} &on-heap;
};

const bool __feat%foo@@P0%uses_offset = True;
const bool __feat%foo@@P0%uses_random_access = True;
Expand Down
4 changes: 2 additions & 2 deletions tests/Baseline/spicy.optimization.default-parser-functions/opt.hlt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public type P1 = struct {
method view<stream> parse2(inout value_ref<P1> __unit, inout value_ref<stream> __data, optional<view<stream>> __cur = Null, optional<spicy_rt::UnitContext> __context) &needed-by-feature="is_filter" &needed-by-feature="supports_sinks" &static;
method view<stream> parse3(inout value_ref<spicy_rt::ParsedUnit> __gunit, inout value_ref<stream> __data, optional<view<stream>> __cur = Null, optional<spicy_rt::UnitContext> __context) &needed-by-feature="is_filter" &needed-by-feature="supports_sinks" &static;
method tuple<const view<stream>, int<64>, const iterator<stream>, optional<hilti::RecoverableFailure>> __parse_foo__P1_stage2(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);
} &on-heap;
};
public type P2 = struct {
uint<8> x &optional;
uint<8> y &optional;
Expand All @@ -25,7 +25,7 @@ public type P2 = struct {
method view<stream> parse2(inout value_ref<P2> __unit, inout value_ref<stream> __data, optional<view<stream>> __cur = Null, optional<spicy_rt::UnitContext> __context) &needed-by-feature="is_filter" &needed-by-feature="supports_sinks" &static;
method view<stream> parse3(inout value_ref<spicy_rt::ParsedUnit> __gunit, inout value_ref<stream> __data, optional<view<stream>> __cur = Null, optional<spicy_rt::UnitContext> __context) &needed-by-feature="is_filter" &needed-by-feature="supports_sinks" &static;
method tuple<const view<stream>, int<64>, const iterator<stream>, optional<hilti::RecoverableFailure>> __parse_foo__P2_stage2(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);
} &on-heap;
};

const bool __feat%foo@@P0%uses_offset = False;
const bool __feat%foo@@P0%uses_random_access = False;
Expand Down
Loading