Wasmtime GitHub notifications bot (Sep 23 2022 at 23:02): Wasmtime GitHub notifications bot (Sep 23 2022 at 23:02):github-actions[bot] commented on issue #4953:
Subscribe to Label Action
cc @cfallin, @fitzgen
<details>
This issue or pull request has been labeled: "cranelift", "cranelift:area:aarch64", "cranelift:area:x64", "cranelift:meta", "isle"Thus the following users have been cc'd because of the following labels:
- cfallin: isle
- fitzgen: isle
To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.
Learn more.
</details>
Wasmtime GitHub notifications bot (Oct 04 2022 at 05:03):cfallin commented on issue #4953:
@jameysharp I think this is more-or-less ready for review now!
(In fact, I just spent a few hours doing the latest rebase, and these are fairly painful because this PR does a lot of code-motion to factor out ISLE commonalities for mid-end and backends, so I'd really prefer to try to get this merged before much more changes to avoid another rebase hell :-) )
I've updated the implementations of the various rewriting and elaboration algorithms to avoid explicit recursion, and I've factored out a generic "analysis" mechanism for the egraph crate to own; these were the main cleanups I wanted/needd to do before the prototype would be considered production-ready, I think. Regarding the rule-rewriting reentrancy issue, I opted to simply cap the recursion with a static limit (max of 5 chained rewrites) for now, as (i) we'll need such a limit anyway to protect against accidentally infinitely-(co)recursive rules, and (ii) it is about an order of magnitude simpler than unwinding the nested-immediate-rewrites into some sort of "pending node queue" and splitting the id-space, and probably more efficient in the common case.
In any case, let me know what you think -- I'm eager for feedback!
Wasmtime GitHub notifications bot (Oct 04 2022 at 05:03):cfallin edited a comment on issue #4953:
@jameysharp I think this is more-or-less ready for review now!
(In fact, I just spent a few hours doing the latest rebase/merge, and these are fairly painful because this PR does a lot of code-motion to factor out ISLE commonalities for mid-end and backends, so I'd really prefer to try to get this merged before much more changes to avoid another rebase hell :-) )
I've updated the implementations of the various rewriting and elaboration algorithms to avoid explicit recursion, and I've factored out a generic "analysis" mechanism for the egraph crate to own; these were the main cleanups I wanted/needd to do before the prototype would be considered production-ready, I think. Regarding the rule-rewriting reentrancy issue, I opted to simply cap the recursion with a static limit (max of 5 chained rewrites) for now, as (i) we'll need such a limit anyway to protect against accidentally infinitely-(co)recursive rules, and (ii) it is about an order of magnitude simpler than unwinding the nested-immediate-rewrites into some sort of "pending node queue" and splitting the id-space, and probably more efficient in the common case.
In any case, let me know what you think -- I'm eager for feedback!
Wasmtime GitHub notifications bot (Oct 04 2022 at 06:13):cfallin commented on issue #4953:
@alexcrichton it looks like the verify-publish CI job is failing here because for some reason it's looking for the (not-yet-published) cranelift-egraph crate on crates.io rather than in the workspace. I copied the magic incantations for workspace-inheritance from the other intra-repo deps but I'm not sure if I'm missing something -- it's surprising that this fails if all the other builds work fine?
Last updated: Nov 22 2024 at 16:03 UTC