Fix singlepass codegen regression #2787
Conversation
|
In some tests blockchains were having a lot of arguments in the functions. |
The vector only have a handfull of value: less than 10. I don't think it's changes much. But if there is a better structure, why not (but egain, it's a very small set) |
|
Perhaps we can use IndexSet for determinism? (other ideas/crates that you know @Amanieu ?) https://docs.rs/indexmap/1.6.1/indexmap/set/struct.IndexSet.html |
|
Since there are never going to be more than 32 elements in the set, just use a bit set. OR to set, AND to clear, |
…e vec for the push/pop
| fn push_used_gpr(&mut self) -> usize { | ||
| let mut used_gprs = self.get_used_gprs(); | ||
| used_gprs.sort(); | ||
| fn push_used_gpr(&mut self, used_gprs: &Vec<GPR>) -> usize { | ||
| for r in used_gprs.iter() { |
There was a problem hiding this comment.
HashSet still has a randomized iteration order so this makes the codegen of singlepass non-deterministic (but still correct).
There was a problem hiding this comment.
True, but do we need to generate the same code on each run?
There was a problem hiding this comment.
We do want to have deterministic reproducible builds for a given input. See #2173.
There was a problem hiding this comment.
Fair enough. I'll remove the hashmaps
|
bors |
|
bors try |
tryBuild failed: |
…map now, to avoid risk of non deterministic runs
|
bors try |
|
bors r+ |
Description
Issue was with the conversion Hash -> vec of the used GPR and SIMD registers before mass Push/Pop. Because a register might be allocated/deallocated in between, the order of the resulting vector might change.
The fix use a simple
sorton the vector before push/pop elements, to be sure the order is deterministic.The PR also contains a few ARM64 machine change, so the test can be run also on Aarch64 Linux & macOS (tested to be working too).
Fixes #2782