Wasmtime GitHub notifications bot (Nov 24 2022 at 01:59): Wasmtime GitHub notifications bot (Nov 24 2022 at 01:59):jameysharp opened PR #5322 from isle-shadowed to main:
Some of our ISLE rules can never fire because there's a higher-priority rule that will always fire instead.
Sometimes the worst that can happen is we generate sub-optimal output. That's not so bad but we'd still like to know about it so we can fix it. On x64, a current example is that lowering the combination of
imulwithswiden_highwill always emit separate machine instructions for each, rather than a single instruction for the combination.In other cases there might be instructions which can't be lowered in isolation. If a general rule for lowering one of the instructions is higher-priority than the rule for lowering the combined sequence, then lowering the combined sequence will always fail. I suspect there aren't any of these because somebody probably would have noticed already, but I don't know.
Either way, this is always a bug, so make it a fatal error if we can detect it.
I'm opening this as a draft because I've made this condition a fatal error without fixing any of the existing instances of that error. I'd like help from folks who are more familiar with the backends to figure out how to fix each case, and maybe write new filetests to exercise the relevant rules.
There are currently:
- 5 too-general rules in x64
- 5 too-general rules in s390x (cc: @uweigand)
- 24 too-general rules in aarch64
Details of all these cases should be visible in the failure output from CI for this PR, once that runs. I think the
cargo checkjob should catch this, among others.
Wasmtime GitHub notifications bot (Nov 24 2022 at 01:59):jameysharp requested elliottt for a review on PR #5322.
Wasmtime GitHub notifications bot (Nov 24 2022 at 02:03):jameysharp edited PR #5322 from isle-shadowed to main:
Some of our ISLE rules can never fire because there's a higher-priority rule that will always fire instead.
Sometimes the worst that can happen is we generate sub-optimal output. That's not so bad but we'd still like to know about it so we can fix it. On x64, a current example is that lowering the combination of
imulwithswiden_highwill always emit separate machine instructions for each, rather than a single instruction for the combination.In other cases there might be instructions which can't be lowered in isolation. If a general rule for lowering one of the instructions is higher-priority than the rule for lowering the combined sequence, then lowering the combined sequence will always fail. I suspect there aren't any of these because somebody probably would have noticed already, but I don't know.
Either way, this is always a bug, so make it a fatal error if we can detect it.
I'm opening this as a draft because I've made this condition a fatal error without fixing any of the existing instances of that error. I'd like help from folks who are more familiar with the backends to figure out how to fix each case, and maybe write new filetests to exercise the relevant rules.
There are currently:
- 5 too-general rules in x64
- 5 too-general rules in s390x (cc: @uweigand)
- 24 too-general rules in aarch64
Details of all these cases are visible in the failure output from CI for this PR.
Wasmtime GitHub notifications bot (Nov 28 2022 at 17:59):jameysharp updated PR #5322 from isle-shadowed to main.
Wasmtime GitHub notifications bot (Nov 28 2022 at 19:14):jameysharp updated PR #5322 from isle-shadowed to main.
Wasmtime GitHub notifications bot (Nov 29 2022 at 01:42):jameysharp updated PR #5322 from isle-shadowed to main.
Wasmtime GitHub notifications bot (Nov 29 2022 at 01:44):jameysharp has marked PR #5322 as ready for review.
Wasmtime GitHub notifications bot (Nov 29 2022 at 19:10):jameysharp updated PR #5322 from isle-shadowed to main.
Wasmtime GitHub notifications bot (Nov 30 2022 at 18:24):elliottt submitted PR review.
Wasmtime GitHub notifications bot (Nov 30 2022 at 23:06):jameysharp merged PR #5322.
Last updated: Nov 22 2024 at 16:03 UTC