Add a pass for minimizing recursion groups #6832

tlively · 2024-08-10T19:01:19Z

Most of our type optimization passes emit all non-public types as a
single large rec group, which trivially ensures that different types
remain different, even if they are optimized to have the same structure.
Usually emitting a single large rec group is fine, but it also means
that if the module is split, all of the types will need to be repeated
in all of the split modules. To better support this use case, add a pass
that can split the large rec group back into minimal rec groups, taking
care to preserve separate type identities by emitting different
permutations of the same group where possible or by inserting unused
brand types to differentiate them.

tlively · 2024-08-11T07:02:30Z

Testing this out on some real-world modules, this pass ended up making the modules smaller by 5-10% uncompressed and ~0.5% compressed. I wouldn't have thought that was possible since this pass can only increase the number of types and add extra bytes introducing the new recursion groups, but breaking up the recursion groups must have let the binary writer reorder types to make the cumulative size of the type index encodings smaller. This points to an opportunity to improve the code for ordering types within the large recursion groups we emit.

tlively · 2024-08-12T21:58:04Z

The fuzzer is very happy with this: it's at 260k iterations and counting.

That being said, I realized there is an edge case we don't handle where one of the minimized recursion groups might incorrectly have the same shape as a public recursion group. I will upload a fix and test here, but I don't expect it to be invasive, so I don't think it should block initial code review.

kripken

Test comments so far.

Is "brand" a standard term in this context? Mentally, it makes me imagine small Nike or Pepsi logos on the types...

kripken · 2024-08-12T19:44:33Z

test/lit/passes/minimize-rec-groups.wast

+    ;; CHECK:       (type $a (struct (field (ref $b))))
+    (type $a (struct (field (ref $b))))
+    (type $b (struct (field (ref $a))))
+    ;; CHECK:      (type $c (struct (field (ref $a))))


Slightly unfortunate that the one space of indentation is the only way to see that this is not in the same rec group. Is it a bug in the auto-updater script that it doesn't add a CHECK line for the ) that closes the rec(?

More of a missing feature than a bug, I'd say, but yes. Getting the updater to print (rec at all requires some special casing since there is no $name there. I can take a look at how difficult it would be to get it to emit the closing paren, too.

Ok I took a look at this but have given up because we are already several iterations into https://xkcd.com/1171/ with the auto update script for lit tests.

kripken · 2024-08-12T19:47:53Z

test/lit/passes/minimize-rec-groups.wast

+
+    ;; CHECK:       (type $b (func))
+    (type $b (func))
+  )


Maybe it's better to leave the types in the same rec group, as an alternative to using a brand? Given that such conflicts may be rare, that might not cause too much extra size even if we lose some opportunities in separate compilation to remove rec groups?

Yes, I considered that solution as well. It has some nice properties but also some less nice properties. The obvious nice property is that it avoids adding new unused types. The big downside is that keeping identically structured types together creates a lot of symmetry in the rec groups that reduce the number of distinct permutations. If the original SCCs only contained one type (which is by far the common case), then there are no distinct permutations of the group at all, no matter how many of the SCCs you group together. So the size of the recursion groups grows a lot faster with this solution compared to the brand solution. Which one will actually have the lower cumulative code size depends on how the module will be split up.

kripken · 2024-08-12T20:01:20Z

test/lit/passes/minimize-rec-groups.wast

+    (type $c (struct))
+  )
+
+  ;; CHECK:       (type $4 (array (mut i8)))


Without the closing ) of the rec group it seems almost impossible to tell if this is inside the last rec group...

Yeah, you have to count spaces in the indentation to determine that it's at the same level as (type $c (struct)) rather than being in the same level as (rec :(

kripken · 2024-08-12T20:47:24Z

test/lit/passes/minimize-rec-groups.wast

+    (type $a3 (sub (struct (field (ref $b3)))))
+    ;; CHECK:       (type $c3 (sub $a3 (struct (field (ref $b3)))))
+
+    ;; CHECK:       (type $b3 (sub $a3 (struct (field (ref $b3)) (field (ref $c3)))))


Just out of curiosity, why do $b3,$c3 end up reordered? (no harm from it, but it isn't helping either as there is a brand)

When we detect that we have two SCCs with the same shape, we form a nontrivial equivalence class for them to be members of. To do that, we find the canonical shape for the equivalence class and start generating permutations based on that. Calculating that canonical shape is what changes the order. In particular, the $c types are ordered before the $b types because they have fewer fields.

tlively · 2024-08-12T23:00:48Z

Is "brand" a standard term in this context? Mentally, it makes me imagine small Nike or Pepsi logos on the types...

Yes, although this usage comes from the other definition of brand as a noun: "an identifying mark burned on livestock..." Trying to search for examples is proving very difficult, since the results are overwhelmingly for the kind of brand you were imagining. The example closest to home is that TC39 discusses brands a lot: https://github.com/tc39/how-we-work/blob/main/terminology.md#brand-check.

kripken · 2024-08-12T23:24:26Z

Yes, although this usage comes from the other definition of brand as a noun

Ah, I see, thanks. Though that meaning isn't necessarily to differentiate (it can be to symbolize ownership). How about "brand" => "diacritic", which is ancient Greek for "to distinguish", and is literally a mark that is applied in order to distinguish two otherwise identical things?

Or, perhaps "accent", as in "They are the same rec group, like British and American English are the same language, but the accents are different."

kripken

Some comments in what I've read so far.

kripken · 2024-08-12T23:28:21Z

src/passes/MinimizeRecGroups.cpp

+// new brand type. This iterator provides an infinite sequence of possible brand
+// types, prioritizing those with the most compact encoding.
+struct BrandTypeIterator {
+  static constexpr size_t optionCount = 18;


18 looks like a magic number here..? The explanation seems to be in the body of initFieldOptions which sets 18 values. Can this 18 be set there?

Not if we want this to continue to be a constexpr, but maybe that's not so important. I'll refactor this.

Oh no, we need either a constexpr or a literal 18 here because it's part of the fieldOptions type, unless we want to make it a vector. But it seems nice to keep it as an array.

Sounds good. Maybe just add a comment pointing people to the code that explains why this is 18 and not any other number.

kripken · 2024-08-12T23:31:22Z

src/passes/MinimizeRecGroups.cpp

+    }
+  };
+
+  bool useArray;


I can't seem to find where this is initialized?

But, separately, is it worth the complexity? Maybe just using structs is simpler.

I would say it's worth the complexity since an array encoding is 25% smaller (if I did the math right) than a singleton struct encoding and I don't think it adds a lot of complexity, but I can easily see the argument the other way as well.

kripken · 2024-08-12T23:35:20Z

src/passes/MinimizeRecGroups.cpp

+// components that have the same shape. When we find such a collision, we merge
+// the components into a single equivalence class where we track how we have
+// disambiguated all such isomorphic components.
+struct GroupClassInfo {


Is "group" meant to be a verb or a noun here? I'm having trouble figuring out how to read the name of this class.

If this class groups ClassInfos, perhaps ClassInfoGrouper?

No, it's the info for a group class.

kripken · 2024-08-12T23:36:57Z

src/passes/MinimizeRecGroups.cpp

+        RecGroupShape({**brand}) == RecGroupShape({*singletonType})) {
+      ++*brand;
+    }
+    // The brand type must be distinct from


comment fragment?

Yep, looks like an unfinished predecessor to "// Make sure the brand is not the same as the real type" above. Will delete.

tlively · 2024-08-13T00:40:01Z

Yes, although this usage comes from the other definition of brand as a noun

Ah, I see, thanks. Though that meaning isn't necessarily to differentiate (it can be to symbolize ownership). How about "brand" => "diacritic", which is ancient Greek for "to distinguish", and is literally a mark that is applied in order to distinguish two otherwise identical things?

Or, perhaps "accent", as in "They are the same rec group, like British and American English are the same language, but the accents are different."

I would prefer to keep the standard term. The brand indicates that differently branded types belong to different groups, just as differently branded cows belong to different farms, and just as differently branded JS values belong to different types.

kripken · 2024-08-13T17:21:36Z

Fair enough about the term, sgtm.

kripken · 2024-08-13T17:24:16Z

src/passes/MinimizeRecGroups.cpp

+
+// As we iterate through the strongly connected components, we may find
+// components that have the same shape. When we find such a collision, we merge
+// the components into a single equivalence class where we track how we have


Suggested change

// the components into a single equivalence class where we track how we have

// the components into a single group class where we track how we have

(or maybe both? either way getting "group" into the comment would help I think)

kripken · 2024-08-13T17:26:10Z

src/passes/MinimizeRecGroups.cpp

+    new (&orders) TopologicalOrders(subtypeGraph);
+  }
+
+  void permute(RecGroupInfo&);


Please add a comment here.

Is "Update permute" a typo perhaps?

Heh, I guess I couldn't decide which word to use, so I accidentally used both.

tlively · 2024-08-13T22:20:32Z

Comments updated as requested.

kripken · 2024-08-13T22:36:11Z

src/passes/MinimizeRecGroups.cpp

+    types = ModuleUtils::getPrivateHeapTypes(*module);
+    for (auto type : ModuleUtils::collectHeapTypes(*module)) {
+      typeIndices.insert({type, typeIndices.size()});
+    }


It looks like types is only the private types, while typeIndices is all the types. That could be confusing - perhaps rename the former privateTypes?

Will keep that in mind when I fix this problem with public types.

src/passes/MinimizeRecGroups.cpp

kripken · 2024-08-13T22:40:58Z

src/passes/MinimizeRecGroups.cpp

+      classInfo.permute(groupInfo);
+
+      updateShape(group);
+      return;


Is there no risk of getting a very deep stack due to this recursion? It seems like in principle there could be a conflict after adding the brand/permuting, and so forth. I'm not sure if it's worth the complexity, but maybe a comment?

Yes, you could set up a pathological input with an arbitrarily deep stack by visiting each permution of a group separately and only then seeing a repeat group that causes the whole thing to collapse into a single equivalence class. A more naive approach would also risk long stretches of useless work as it iterates through permutations that are automorphic to previous permutations, but we eliminate that source of repetition by construction.

I don't think it will be too bad to switch to a worklist here, so I'll do that.

kripken · 2024-08-13T22:41:28Z

src/passes/MinimizeRecGroups.cpp

+    // create a nontrivial equivalence class, this is usually not possible
+    // because two equivalence classes with isomorphic types should actually be
+    // the same equivalence class. The exception is when the base shapes of two
+    // different equivalence classses are the same, but there is still a


Suggested change

// different equivalence classses are the same, but there is still a

// different equivalence classes are the same, but there is still a

src/passes/MinimizeRecGroups.cpp

kripken · 2024-08-13T23:36:38Z

src/passes/MinimizeRecGroups.cpp

+
+    // Case 1C: We have permuted ourselves into equivalence with some other
+    // group unaffiliated with a nontrivial equivalence class. Bring that other
+    // group into our equivalence class and try the next permutation instead.


Why bring that other unaffiliated group into this class, at this time? Is that an optimization, or necessary for correctness?

Necessary for correctness, insofar as we consider deviations from the intended lazy/eager hybrid design to be correctness bugs. We've discovered that the groups in the current equivalence class are isomorphic to some other group, so logically that group is part of the current equivalence class. Not reflecting that in the data structures means that in the future we might have two "different" equivalence classes that are generating the same shapes as each other and wasting a lot of work.

But won't updateShape be called on that group later (from the main loop, line 352)? That is, when we get to that group, wouldn't it see the conflict? I may be missing something here.

No, this case arises when the unaffiliated group has been processed before the current group.

Ah, then what I am confused about is the text

Bring that other group into our equivalence class and try the next permutation instead

I would expect the reverse. That is, I'd expect that we leave that existing group as it is - it is fine, it's already handled - and we keep modifying ourselves until we are no longer in conflict with anything?

No, that would only be the case if we eagerly canonicalized every rec group.

Say we have rec groups 0, 1, and 2 with shapes A and A' that are isomorphic to each other. We might process them in this order:

0: A 1: A' 2: A

After processing 0 and 1, we're going to have two different equivalence classes in the data structures because we don't yet know that A and A' are isomorphic. Then we process 2, which has the same shape as 0. We join them into a nontrivial equivalence class, canonicalize their shapes, and reassign the new shapes to 0 and 2. But what if the new shape we try to give 2 turns out to be A'? Only now do we know that the class containing 0 and 2 should be joined with 1's class. We update the classes, then try again assigning 2 a new shape A''.

Given those rec groups and shapes, wouldn't the simpler thing be to notice that A and A' are isomorphic when we get to 1? And then fix that up, so that at every point in time, every processed group is unique modulo isomorphism and every group we see later just needs to be modified so it is not isomorphic to them all.

(Sorry, I'm probably missing something basic here!)

It would be simpler, but it would also be expensive. The canonicalization routine is something like O(n^2 log n), so we save a lot of work by canonicalizing lazily once we discover a non-trivial equivalence class.

I'll push my commit that describes the high-level algorithmic design better. Maybe that will help.

Oh, thanks... now I think I see.

This does seem more complicated than I'd expect. Did you measure canonicalization as being so slow in practice as to require this?

But I guess I'm ok either way. The nice thing in compilers is that passes are modular, so if we end up hitting hard to fix bugs in this pass, we can rewrite it.

I hacked up a version that does eager canonicalization and it runs in about the same time, which I guess makes sense because most of the recursion groups are tiny.

src/passes/MinimizeRecGroups.cpp

kripken · 2024-08-14T22:59:42Z

src/passes/MinimizeRecGroups.cpp

+    }
+
+    // Case 1B: There is a conflict with a group from a different nontrivial
+    // equivalnece class. Because we canonicalize the shapes whenever we first


Suggested change

// equivalnece class. Because we canonicalize the shapes whenever we first

// equivalence class. Because we canonicalize the shapes whenever we first

kripken · 2024-08-14T23:00:49Z

src/passes/MinimizeRecGroups.cpp

+  // Does this group include a brand type that does not correspond to a type in
+  // the original module?
+  bool hasBrand = false;
+  // This group may be the representative group for its nontrival equivalence


Suggested change

// This group may be the representative group for its nontrival equivalence

// This group may be the representative group for its nontrivial equivalence

kripken · 2024-08-14T23:01:26Z

src/passes/MinimizeRecGroups.cpp

+  // This group may be the representative group for its nontrival equivalence
+  // class, in which case it holds the necessary extra information used to add
+  // new groups to the class.
+  std::optional<GroupClassInfo> classInfo;


Why store this information on the representative rather than on the class itself?

The point of forming equivalence classes in the first place is so that all the different isomorphic group shapes can point to the same generator of fresh permutations. That shared generator and other shared group information lives on the representative group, which is the one that can be found via the disjoint set forest starting at any of the other groups in the equivalence class.

In other words, putting the information on the representative is putting the information on the class itself, since the representative group is the representation of the equivalence class.

kripken · 2024-08-14T23:02:02Z

src/passes/MinimizeRecGroups.cpp

+// group, so canonicalization itself would be wasted work.
+//
+// The simplest possible design in the other direction is to never canonicalize
+// and instead to lazily build up equivalences classes of isormorphic groups


Suggested change

// and instead to lazily build up equivalences classes of isormorphic groups

// and instead to lazily build up equivalences classes of isomorphic groups

kripken · 2024-08-14T23:43:28Z

src/passes/MinimizeRecGroups.cpp

+// and instead to lazily build up equivalences classes of isormorphic groups
+// when conflicts are detected in practice. Conflicts are resolved by choosing
+// the next permutation generated by the representative element of the
+// equivalence class. This solution is not ideal because groups with high


I don't understand the role of the "representative element" here. I would assume we just want the next permutation of the equivalence class, period?

Right, and the information for the equivalence class is stored on the representative element of that class. That way we don't need to update any additional data structures when we merge equivalence classes.

I see, thanks. In that case, is it worth mentioning it here? That we store that on the representative element is an implementation detail, not fundamental to the algorithm. I feel that keeping this comment high level would be clearer, in other words.

I think this detail is useful because it clarifies what happens when equivalence classes are merged. After a merge, the subsequent permutations are generated by whatever the new representative element is. If we take these comments as roadmaps for how we might rewrite and simplify the pass in the future, that level of detail could be useful.

kripken · 2024-08-15T15:48:01Z

src/passes/MinimizeRecGroups.cpp

+    // due to brands in each class matching the base type of the other class. In
+    // this case we don't actually want to join the classes; we just want to
+    // advance to the next configuration to resolve the conflict.
+    if (groups[groupRep].classInfo && groups[otherRep].classInfo) {


Maybe add an assert groups[groupRep].classInfo != groups[otherRep].classInfo - ?

Equality is not defined on GroupClassInfo. They are physically in different locations because groupRep != otherRep, but that's evident from the control flow.

kripken · 2024-08-16T20:11:35Z

src/passes/MinimizeRecGroups.cpp

+    //
+    // Corollary 1.2: No two isomorphic SCCs with the same first element are the
+    // same since no nontrivial automorphism can keep the first element
+    // stationary.


I get what you're saying here, but how about phrasing it like this?

If two isomorphic SCCs have the same first element then they must be identical, i.e., isomorphic via the trivial automorphism, as no non-trivial automorphism can keep the first element stationary.

Or, you can add the word "distinct" ("No two distinct ..").

wdyt of the latest wording?

Wording looks good!

kripken · 2024-08-16T20:24:49Z

src/passes/MinimizeRecGroups.cpp

+    //
+    //     Proof: By contradiction. Assume two such SCCs are automorphic to each
+    //     other. Then their initial elements must be in the same automorphism
+    //     cycle because they occupy the same index in two automorphic graphs.


I'm not sure what you mean by "initial elements" here. This is the first time the term appears in the document that I can see.

I am guessing it means some prefix of the rec group? But that doesn't seem right, e.g.

(rec (type $a1 (struct (field (ref $a2) (ref $b1)))) (type $b1 (struct (field (ref $b2) (ref $a1)))) (type $a2 (struct (field (ref $a1) (ref $b2)))) (type $b2 (struct (field (ref $b1) (ref $a2)))) )

We have the automorphism ($a1 $a2) ($b1 $b2), which does not keep the first two elements$a1, $b1 in a cycle.

By "initial element" I mean the element at index 0. In your example, applying the automorphism gives a graph with a different initial element, $a2 instead of $a1. $a1 and $a2 are in the same cycle in some automorphism of the SCC.

Ah, it's the plural that threw me:

Then their initial elements

I get now that you mean the initial element in the two, so one element in each, but that was quite confusing...

Avoiding the plural here, or using the term "initial element" before, could be helpful here I think.

A more precise way to put this would say that two SCCs are not automorphic to each other if their elements at index 0 are not automorphic to each other, but we haven't so far defined automorphism on individual vertices, and the definition would basically restate this theorem as a definition. Maybe we just delete this one.

See the last commit, which deletes theorem two and "inlines" the important takeaway into a later comment.

kripken · 2024-08-16T21:22:50Z

src/passes/MinimizeRecGroups.cpp

+    // stationary.
+    //
+    // Theorem 2: SCCs with initial elements that are not in an automorphism
+    // cycle with each other are not automorphic to each other.


Say I have two SCCs, (A,B,C,D) and (X,Y,Z,W). How can I talk about an automorphism cycle between them? Don't I need to have an automorphism for that? Are you talking about all possible automorphisms between them perhaps?

I am also a bit confused by the term "automorphic". "Isomorphic" makes sense, in that it says that two things can be mapped to each other,

Two mathematical structures are isomorphic if an isomorphism exists between them. (wikipedia)

But everything is "automorphic" in a boring way (everything can be mapped to itself, using the identity, trivially). Indeed, the term "automorphic" doesn't appear on Wikipedia's page for "Automorphism", and searching elsewhere I can't find general use of the term in math (only specific ones). Maybe I'm missing it?

Say I have two SCCs, (A,B,C,D) and (X,Y,Z,W). How can I talk about an automorphism cycle between them?

Yeah, this is pretty much a category error. That's why I think deleting this makes the most sense. PTAL at the commit "delete theorem 2" for the fix.

kripken · 2024-08-16T21:34:14Z

src/passes/MinimizeRecGroups.cpp

+    //
+    // Corollary 1.2: No two distinct isomorphisms of an SCC with the same first
+    // element are automorphic to each other since no nontrivial automorphism
+    // can keep the first element stationary.


Probably related to my confusion about the term "automorphic": I don't follow this. Before, we defined it like this:

an automorphism of a graph is a permutation of the vertices that does not change the graph

So if two SCCs are distinct, then they are not the same graph. So no automorphism is even relevant to speak of here: any mapping between them changes the graph, so it cannot be an automorphism, so no automorphism can exist. That is, we don't need any facts about the first element. So I must be missing the point yet again 😄

Returning to our example:

(rec (type $a1 (struct (field (ref $a2) (ref $b1)))) (type $b1 (struct (field (ref $b2) (ref $a1)))) (type $a2 (struct (field (ref $a1) (ref $b2)))) (type $b2 (struct (field (ref $b1) (ref $a2)))) )

A: $a1, $b1, $a2, $b2 and B: $a1, $a2, $b1, $b2 are distinct permutations (i.e. distinct isomorphisms) of this SCC. They have the same first element, $a1. The corollary says that there is no automorphism of A onto B or vice versa (i.e. A and B are not "automorphic" by analogy to the relationship between "isomorphism" and "isomorphic").

Hmm, I really think we should think about another term here:

The corollary says that there is no automorphism of A onto B

Automorphisms are of a thing on itself. That is, it is a function from X to X, not X to Y. That's wikipedia's definition and the one I've always heard. And we were consistent with it before, when we said "here is a rec group Foo and here is an automorphism on it in cycle notation".

Also, any automorphism (in that old sense) on A will turn A into itself, so no such transform can get to B (since A != B). Likewise any automorphism on B cannot get to A.

OTOH there is a transform that gets from A to B: 0,2,1,3. So my best guess is that you want to disallow this particular transform but allow others, but I can't see how/why yet.

$a1, $b1, $a2, $b2 has an automorphism ($a1 $a2) ($b1 $b2), so it is automorphic to $a2, $b2, $a1, $b1 even though those are distinct permutations that have different initial elements. If you write down the vertices of these graphs (i.e. colors and ordered edges), you will write down the same graph twice. But $a1 is a different type from $a2 even though they are automorphic to each other.

After offline discussion, it looks like the best path forward is to clarify that this corollary is using a notion of type identity that is not related to the structures of emitted rec groups. In general, it would be good to define all the different equivalence relations we are using up front.

Most of our type optimization passes emit all non-public types as a single large rec group, which trivially ensures that different types remain different, even if they are optimized to have the same structure. Usually emitting a single large rec group is fine, but it also means that if the module is split, all of the types will need to be repeated in all of the split modules. To better support this use case, add a pass that can split the large rec group back into minimal rec groups, taking care to preserve separate type identities by emitting different permutations of the same group where possible or by inserting unused brand types to differentiate them.

kripken · 2024-08-16T23:54:07Z

src/passes/MinimizeRecGroups.cpp

+    const auto& leastOrder = typeClasses.begin()->first.types;
+
+    // We want our canonical ordering to have the additional property that it
+    // contains with one type from each equivalence class before a second type


Suggested change

// contains with one type from each equivalence class before a second type

// contains one type from each equivalence class before a second type

kripken

lgtm with some comment improvements

tlively · 2024-08-17T02:01:56Z

I've now comprehensively edited the comments to be more careful about distinguishing between rec groups being the same (or not) and their shapes being the same (or not). This is an important distinction because the shapes are label-independent, but sometimes the labels (i.e. the separate type identities, independent of how they are emitted in the binary) actually matter. I've also added an example of an automorphism and explained how applying the automorphism does not change the shape of the rec group, but it still moves the types (i.e. the labels) around.

I'm going to go ahead and land this now, but @kripken, PTAL at the edits next week and feel free to provide additional feedback on how the comments can be clarified.

tlively requested a review from kripken August 10, 2024 19:01

kripken reviewed Aug 12, 2024

View reviewed changes

tlively force-pushed the type-updating-typenames branch from a46dbab to 00666ca Compare August 13, 2024 01:36

tlively force-pushed the minimize-recgroup-pass branch from 91ad5ed to 63623ed Compare August 13, 2024 01:36

tlively force-pushed the type-updating-typenames branch from 00666ca to 6b7035a Compare August 13, 2024 02:33

tlively force-pushed the minimize-recgroup-pass branch from 63623ed to 665ceac Compare August 13, 2024 02:33

tlively force-pushed the type-updating-typenames branch from 6b7035a to c15e816 Compare August 13, 2024 03:24

tlively force-pushed the minimize-recgroup-pass branch from 665ceac to c6ad8a9 Compare August 13, 2024 03:24

Base automatically changed from type-updating-typenames to main August 13, 2024 04:34

kripken reviewed Aug 13, 2024

View reviewed changes

tlively force-pushed the minimize-recgroup-pass branch from c6ad8a9 to 3581d70 Compare August 13, 2024 22:20

kripken reviewed Aug 13, 2024

View reviewed changes

src/passes/MinimizeRecGroups.cpp Outdated Show resolved Hide resolved

kripken reviewed Aug 13, 2024

View reviewed changes

src/passes/MinimizeRecGroups.cpp Show resolved Hide resolved

kripken reviewed Aug 13, 2024

View reviewed changes

tlively force-pushed the minimize-recgroup-pass branch from c4c7074 to 363db9e Compare August 14, 2024 19:06

kripken reviewed Aug 14, 2024

View reviewed changes

src/passes/MinimizeRecGroups.cpp Outdated Show resolved Hide resolved

kripken reviewed Aug 14, 2024

View reviewed changes

kripken reviewed Aug 15, 2024

View reviewed changes

tlively force-pushed the minimize-recgroup-pass branch from b630e29 to 6d2bc68 Compare August 16, 2024 19:24

kripken reviewed Aug 16, 2024

View reviewed changes

tlively added 15 commits August 16, 2024 16:20

fix conflicts caused by brands

6b32c59

formatting

b9c7bd1

delete comment fragment

a341e0e

initialize BrandTypeIterator::useArray explicitly

997bde1

improve comments

69cde4b

typo

eee885a

classses -> classes

1b267cd

eliminate recursion

022b7bb

design essay

1c32222

typos

0938bfc

in to => into

20c1094

make language more uniform and precise

faf6998

improve corollary 1.2

8f2eed9

delete theorem 2

f4ddd4a

kripken reviewed Aug 16, 2024

View reviewed changes

kripken approved these changes Aug 16, 2024

View reviewed changes

tlively added 2 commits August 16, 2024 17:48

remove stray 'with'

567067a

comprehensive editing pass

4a5cece

tlively force-pushed the minimize-recgroup-pass branch from 63e3103 to 4a5cece Compare August 17, 2024 01:54

tlively enabled auto-merge (squash) August 17, 2024 02:02

tlively merged commit e058bfb into main Aug 17, 2024

tlively deleted the minimize-recgroup-pass branch August 17, 2024 02:14

gkdn mentioned this pull request Aug 31, 2024

stringconsts gkdn/binaryen#1

Closed

	// the components into a single equivalence class where we track how we have
	// the components into a single group class where we track how we have

	// different equivalence classses are the same, but there is still a
	// different equivalence classes are the same, but there is still a

	// equivalnece class. Because we canonicalize the shapes whenever we first
	// equivalence class. Because we canonicalize the shapes whenever we first

	// This group may be the representative group for its nontrival equivalence
	// This group may be the representative group for its nontrivial equivalence

	// and instead to lazily build up equivalences classes of isormorphic groups
	// and instead to lazily build up equivalences classes of isomorphic groups

	// contains with one type from each equivalence class before a second type
	// contains one type from each equivalence class before a second type

Add a pass for minimizing recursion groups #6832

Add a pass for minimizing recursion groups #6832

Uh oh!

Conversation

tlively commented Aug 10, 2024

Uh oh!

tlively commented Aug 11, 2024

Uh oh!

tlively commented Aug 12, 2024

Uh oh!

kripken left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tlively commented Aug 12, 2024

Uh oh!

kripken commented Aug 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kripken left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tlively commented Aug 13, 2024

Uh oh!

kripken commented Aug 13, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tlively commented Aug 13, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kripken commented Aug 12, 2024 •

edited

Loading

tlively Aug 13, 2024 •

edited

Loading