afonso360 commented on issue #5612:
When I investigated that issue I think it was due to a veneer being inserted right in the middle of a load_constant sequence, that uses fixed offset jumps. Its likely that the other fixes changed the generated code enough that that particular test case no longer triggers that same issue.
I'll run the fuzzer again to find another counterexample.
yuyang-ok commented on issue #5612:
Yeah, sounds reasonable. @afonso360
yuyang-ok commented on issue #5612:
I think I have locate the problem.
Here I put a
Minstrel::RawData
.
https://github.com/bytecodealliance/wasmtime/blob/main/cranelift/codegen/src/isa/riscv64/inst/emit.rs#L80And when emit I try to
emit_island
.
https://github.com/bytecodealliance/wasmtime/blob/main/cranelift/codegen/src/isa/riscv64/inst/emit.rs#L473
Those load constant instruction must execute together.I also think this struct
LoadConstant
should only used in emit stage not lower stage.
I test at299be327d
.<img width="1182" alt="image" src="https://user-images.githubusercontent.com/96557710/213897473-6e6af2b9-1f17-4a0f-90b9-389cfd2b4478.png">
<img width="1182" alt="image" src="https://user-images.githubusercontent.com/96557710/213897490-e992da26-1899-4bbf-9a05-2e43ad11b3e1.png">
yuyang-ok commented on issue #5612:
I checked the code
LoadConstant
is only used at emit stage.Maybe I use this in lower stage too early.
I don't know who modify the code.
yuyang-ok edited a comment on issue #5612:
I think I have locate the problem.
Here I put a
Minstrel::RawData
.
https://github.com/bytecodealliance/wasmtime/blob/main/cranelift/codegen/src/isa/riscv64/inst/emit.rs#L80And when emit I try to
emit_island
.
https://github.com/bytecodealliance/wasmtime/blob/main/cranelift/codegen/src/isa/riscv64/inst/emit.rs#L470
Those load constant instruction must execute together.I also think this struct
LoadConstant
should only used in emit stage not lower stage.
I test at299be327d
.<img width="1182" alt="image" src="https://user-images.githubusercontent.com/96557710/213897473-6e6af2b9-1f17-4a0f-90b9-389cfd2b4478.png">
<img width="1182" alt="image" src="https://user-images.githubusercontent.com/96557710/213897490-e992da26-1899-4bbf-9a05-2e43ad11b3e1.png">
yuyang-ok deleted a comment on issue #5612:
I checked the code
LoadConstant
is only used at emit stage.Maybe I use this in lower stage too early.
I don't know who modify the code.
yuyang-ok edited a comment on issue #5612:
I think I have locate the problem.
Here I put a
Minstrel::RawData
.
https://github.com/bytecodealliance/wasmtime/blob/main/cranelift/codegen/src/isa/riscv64/inst/emit.rs#L80And when emit I try to
emit_island
.
https://github.com/bytecodealliance/wasmtime/blob/main/cranelift/codegen/src/isa/riscv64/inst/emit.rs#L470
Those load constant instruction must execute together.I also think this struct
LoadConstant
should only used in emit stage not lower stage.
I test at commit299be327d
.<img width="1182" alt="image" src="https://user-images.githubusercontent.com/96557710/213897473-6e6af2b9-1f17-4a0f-90b9-389cfd2b4478.png">
<img width="1182" alt="image" src="https://user-images.githubusercontent.com/96557710/213897490-e992da26-1899-4bbf-9a05-2e43ad11b3e1.png">
afonso360 commented on issue #5612:
That looks like it could fix the issue! I'm going to run the fuzzer for a while to check if it finds another counter example on this branch!
yuyang-ok edited a comment on issue #5612:
I think I have locate the problem.
Here I put a
Minst::RawData
.
https://github.com/bytecodealliance/wasmtime/blob/main/cranelift/codegen/src/isa/riscv64/inst/emit.rs#L80And when emit I try to
emit_island
.
https://github.com/bytecodealliance/wasmtime/blob/main/cranelift/codegen/src/isa/riscv64/inst/emit.rs#L470
Those load constant instruction must execute together.I also think this struct
LoadConstant
should only used in emit stage not lower stage.
I test at commit299be327d
.<img width="1182" alt="image" src="https://user-images.githubusercontent.com/96557710/213897473-6e6af2b9-1f17-4a0f-90b9-389cfd2b4478.png">
<img width="1182" alt="image" src="https://user-images.githubusercontent.com/96557710/213897490-e992da26-1899-4bbf-9a05-2e43ad11b3e1.png">
yuyang-ok edited a comment on issue #5612:
I think I have locate the problem.
Here I put a
Minst::RawData
.
https://github.com/bytecodealliance/wasmtime/blob/main/cranelift/codegen/src/isa/riscv64/inst/emit.rs#L80And when emit I try to
emit_island
.
https://github.com/bytecodealliance/wasmtime/blob/main/cranelift/codegen/src/isa/riscv64/inst/emit.rs#L470
Those load constant instructions must execute together.I also think this struct
LoadConstant
should only used in emit stage not lower stage.
I test at commit299be327d
.<img width="1182" alt="image" src="https://user-images.githubusercontent.com/96557710/213897473-6e6af2b9-1f17-4a0f-90b9-389cfd2b4478.png">
<img width="1182" alt="image" src="https://user-images.githubusercontent.com/96557710/213897490-e992da26-1899-4bbf-9a05-2e43ad11b3e1.png">
afonso360 commented on issue #5612:
It ran the fuzzer overnight without crashing, It looks like it's fixed!
yuyang-ok commented on issue #5612:
@afonso360 I think this issue is fixed.:-)
yuyang-ok edited a comment on issue #5612:
@afonso360 I think this issue is fixed too.:-)
yuyang-ok commented on issue #5612:
@afonso360 What are we waiting for?
afonso360 commented on issue #5612:
Hey, someone else has to review this and merge it.
yuyang-ok commented on issue #5612:
@afonso360 ok, thanks
Last updated: Dec 23 2024 at 13:07 UTC