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.

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

Feature/strpool #184

Merged
merged 76 commits into from
Mar 9, 2023
Merged

Feature/strpool #184

merged 76 commits into from
Mar 9, 2023

Conversation

leissa
Copy link
Member

@leissa leissa commented Feb 25, 2023

This PR significantly speeds up the whole compiler. Impala test suite runs in 23s instead of 89s on my notebook (Debug build). And this time includes firing up clang and running the actual compiled program. Apparently, we spend much time on master packing and unpacking debug infos.

Def::meta is gone

Was only used at two spots:

  1. Memorize field names for extract expressions a#x in the frontend. We are now simply using a map instead.
  2. For keeping track of implicits. This is now a flags of a Pi instead which is much cleaner and simpler anyway.

Sym

All strings are tossed into a global hash set - the SymPool. This one lives in a new class Driver where a few global variables live. Well, there are not really global - that's the point of the Driver class. Also, it decouples some stuff from World and fights its god class status.

Sym simply wraps const std::string* (so pass it around as value) and does ==/!= comparisons in O(1). The empty string is internally handled as nullptr. Thus, you can create a Sym representing an empty string without having access to the SymPool. The empty string, nullptr, and "\0" are all identified as Sym().

Since creating a Sym has a slight runtime penalty (although much faster than the old solution), it's a good idea to create needed Symbols beforehand. You can see that in lower_for.h and a few other spots where I already did that but was too lazy to do it in all passes. It's only a slight penalty and hence probably hardly worth the effort. But if you feel like using this trick in your passes, the anonymous sruct and struct initialization - as I did in lower_for.h - are arguably the solution requiring the least amount of boilerplate.

Def::dbg

This changed from const Def* to struct Dbg { Loc loc; Sym sym }.

Instead of passing const Def* dbg all over the place, all classes now have setters:

def1->set(loc);
def2->set(loc, sym);
def3->set(sym);
def4->set(dbg);

Or directly set on creation:

auto lam = world.nom_lam(/*...*/)->set(loc); // lam has type Lam*

Misc

  • Since I touched a lot of signatures I also changed those from const Def* to Ref.
  • The introduction of the setters requires some macro magic which is a bit unfortunate but AFAIK there is no better way.
  • see also optimization exhibits non-deterministic behavior #143
  • Resolved some subtle Doxygen issues

leissa added 9 commits March 7, 2023 15:57
Also added missing std::move in Def::setters
These variants don't create new nodes as opposed to
isa_lit(def->arity())
and
as_lit(def->arity()).
This is slightly faster but is important in assertions to not have a
discprepancy between Debug and Release build in code like this:
assert(A == as_lit(arity()));
Solely relying on the name introduces subtle bugs when some Def's
name coincidentally has the same name as an external. This flag
distinguishes externals from non-externals with the same name and also
makes Def::is_external checks faster.
@leissa leissa marked this pull request as ready for review March 8, 2023 15:14
@leissa leissa requested review from fodinabor and NeuralCoder3 March 8, 2023 15:20
dialects/affine/passes/lower_for.h Outdated Show resolved Hide resolved
dialects/autodiff/auxiliary/autodiff_rewrite_toplevel.cpp Outdated Show resolved Hide resolved
dialects/clos/phase/clos_conv.cpp Show resolved Hide resolved
dialects/direct/passes/cps2ds.cpp Show resolved Hide resolved
@leissa leissa merged commit 717fc70 into master Mar 9, 2023
@leissa leissa deleted the feature/strpool branch March 9, 2023 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants