2af8d1e93c
* ISLE compiler: fix priority-trie interval bug. (#4093) This PR fixes a bug in the ISLE compiler related to rule priorities. An important note first: the bug did not affect the correctness of the Cranelift backends, either in theory (because the rules should be correct applied in any order, even contrary to the stated priorities) or in practice (because the generated code actually does not change at all with the DSL compiler fix, only with a separate minimized bug example). The issue was a simple swap of `min` for `max` (see first commit). This is the minimal fix, I think, to get a correct priority-trie with the minimized bug example in this commit. However, while debugging this, I started to convince myself that the complexity of merging multiple priority ranges using the sort of hybrid interval tree / string-matching trie data structure was unneeded. The original design was built with the assumption we might have a bunch of different priority levels, and would need the efficiency of merging where possible. But in practice we haven't used priorities this way: the vast majority of lowering rules exist at the default (priority 0), and just a few overrides are explicitly at prio 1, 2 or (rarely) 3. So, it turns out to be a lot simpler to label trie edges with (prio, symbol) rather than (prio-range, symbol), and delete the whole mess of interval-splitting logic on insertion. It's easier (IMHO) to convince oneself that the resulting insertion algorithm is correct. I was worried that this might impact the size of the generated Rust code or its runtime, but In fact, to my initial surprise (but it makes sense given the above "rarely used" factor), the generated code with this compiler fix is *exactly the same*. I rebuilt with `--features rebuild-isle,all-arch` but... there were no diffs to commit! This is to me the simplest evidence that we didn't really need that complexity. * Fix earlier commit from #4093: properly sort trie. This commit fixes an in-hindsight-obvious bug in #4093: the trie's edges must be sorted recursively, not just at the top level. With this fix, the generated code differs only in one cosmetic way (a let-binding moves) but otherwise is the same. This includes @fitzgen's fix to the CI (from the revert in #4102) that deletes manifests to actually check that the checked-in source is consistent with the checked-in compiler. The force-rebuild step is now in a shell script for convenience: anyone hacking on the ISLE compiler itself can use this script to more easily rebuild everything. * Add note to build.rs to remind to update force-rebuild-isle.sh
20 lines
783 B
Bash
Executable File
20 lines
783 B
Bash
Executable File
#!/bin/sh
|
|
#
|
|
# This script rebuilds all ISLE generated source that is checked in, even if
|
|
# the source has not changed relative to the manifests.
|
|
#
|
|
# This is useful when one is developing the ISLE compiler itself; otherwise,
|
|
# changing the compiler does not automatically change the generated code, even
|
|
# if the `rebuild-isle` feature is specified.
|
|
|
|
set -e
|
|
|
|
# Remove the manifests (which contain hashes of ISLE source) to force the build
|
|
# script to regenerate all backends.
|
|
rm -f cranelift/codegen/src/isa/*/lower/isle/generated_code.manifest
|
|
|
|
# `cargo check` will both invoke the build script to rebuild the backends, and
|
|
# check that the output is valid Rust. We specify `all-arch` here to include
|
|
# all backends.
|
|
cargo check -p cranelift-codegen --features rebuild-isle,all-arch
|