Stream: git-wasmtime

Topic: wasmtime / PR #5383 cranelift-isle: Factor constraint/bin...

Wasmtime GitHub notifications bot (Dec 06 2022 at 16:58):

jameysharp opened PR #5383 from isle-binding to main:

It turns out that during codegen I'll want to know which bindings were added for a particular constraint. Factoring that out and making sure to use it everywhere that constraints and bindings are created ensures that these will always stay in sync. It also simplifies the implementation of normalize_equivalence_classes, which needs to create bindings for constraints but doesn't care what they are.

Also make add_pattern_constraints non-recursive and reuse allocations.

Wasmtime GitHub notifications bot (Dec 06 2022 at 16:58):

jameysharp requested cfallin for a review on PR #5383.

Wasmtime GitHub notifications bot (Dec 08 2022 at 01:33):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Dec 08 2022 at 01:33):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Dec 08 2022 at 01:33):

cfallin created PR review comment:

This is somewhat unintuitive -- iterating over an index gives me ... all indices less than that index?

If we have to have such a method, I'd prefer for it not to be a trait impl of IntoIterator (this also avoids the need to specify the explicit IntoIter type and expose the iterator-chain innards) but rather have something like fn up_to(self) -> impl Iterator<Item = Self> { (0..self.0).map(TupleIndex) }.

Wasmtime GitHub notifications bot (Dec 08 2022 at 01:33):

cfallin created PR review comment:

I think I understand what this method is doing, but it's extremely convoluted and difficult to understand -- the explicit state machine isn't doing it any favors. Since we're inside islec and performance is not extremely critical, could we return a Vec<Binding> here? Then one could do a match self { Constraint::ConstInt { .. } => vec![], Constraint::Some => vec![source], ... } instead. Likewise this would avoid the need to iterate over the tuple index (above).

Wasmtime GitHub notifications bot (Dec 08 2022 at 01:53):

jameysharp updated PR #5383 from isle-binding to main.

Wasmtime GitHub notifications bot (Dec 08 2022 at 01:55):

jameysharp has enabled auto merge for PR #5383.

Wasmtime GitHub notifications bot (Dec 08 2022 at 02:26):

jameysharp merged PR #5383.

Last updated: Nov 22 2024 at 16:03 UTC