Wasmtime GitHub notifications bot (Sep 11 2021 at 13:15): Wasmtime GitHub notifications bot (Sep 11 2021 at 13:15):uweigand opened PR #3330 from s390x-regressions to main:
- Add relocation handling needed after PR #3275
- Fix incorrect handling of signed constants detected by PR #3056 test
- Disable fuzzing tests that require pre-built v8 binaries
- Disable cranelift test that depends on i128
- Temporarily disable memory64 tests
<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
Wasmtime GitHub notifications bot (Sep 13 2021 at 13:59):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Sep 13 2021 at 13:59):alexcrichton created PR review comment:
How come memory64 isn't implemented on s390x? Does it lower to unimplemented clif things at the moment?
Wasmtime GitHub notifications bot (Sep 13 2021 at 13:59):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Sep 13 2021 at 14:00):alexcrichton created PR review comment:
Given the shift, is the range actually one bit larger than what's specified here?
Wasmtime GitHub notifications bot (Sep 13 2021 at 14:11):uweigand submitted PR review.
Wasmtime GitHub notifications bot (Sep 13 2021 at 14:11):uweigand created PR review comment:
Yes. This is all fixable, but I'd prefer to do that separately.
Wasmtime GitHub notifications bot (Sep 13 2021 at 14:12):uweigand created PR review comment:
The range is actually +/- 4GB, not +/-2 GB. But that apparently cannot be represented as CodeOffset anyway, so I simply accept all valid CodeOffsets here.
Wasmtime GitHub notifications bot (Sep 13 2021 at 14:12):uweigand submitted PR review.
Wasmtime GitHub notifications bot (Sep 13 2021 at 14:13):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Sep 13 2021 at 14:13):alexcrichton created PR review comment:
Could this return
0xffff_ffffas a placeholder? One day I think it'd be good to represent some code offsets in this area as a 64-bit integer, but that I think is a much bigger change.
Wasmtime GitHub notifications bot (Sep 13 2021 at 14:21):uweigand submitted PR review.
Wasmtime GitHub notifications bot (Sep 13 2021 at 14:21):uweigand created PR review comment:
I tried that when adding BranchRIL (which has the same range) initially, and it caused failures (some kind of overflows in common code), so that's what I ended up with. I guess this could be revisited at some point, but then we should do it for all affected LabelUse types, not just PCRel32Dbl.
Wasmtime GitHub notifications bot (Sep 13 2021 at 14:23):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Sep 13 2021 at 14:23):alexcrichton created PR review comment:
Can the overflow be fixed with a
saturating_addor something like that somewhere? If not can the comment be updated that this should be 4GB but it's specified as 2GB for now?
Wasmtime GitHub notifications bot (Sep 13 2021 at 14:43):uweigand submitted PR review.
Wasmtime GitHub notifications bot (Sep 13 2021 at 14:43):uweigand created PR review comment:
That would probably help, I'll give it a try. It's still better to do that in a separate PR I think.
Wasmtime GitHub notifications bot (Sep 13 2021 at 15:39):uweigand updated PR #3330 from s390x-regressions to main.
Wasmtime GitHub notifications bot (Sep 15 2021 at 17:35):cfallin submitted PR review.
Wasmtime GitHub notifications bot (Sep 20 2021 at 14:12):alexcrichton merged PR #3330.
Last updated: Dec 23 2024 at 12:05 UTC