Stream: git-wasmtime

Topic: wasmtime / PR #3769 Cranelift: fix debuginfo wrt cold blo...


view this post on Zulip Wasmtime GitHub notifications bot (Feb 05 2022 at 03:56):

cfallin requested fitzgen for a review on PR #3769.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 05 2022 at 03:56):

cfallin opened PR #3769 from fix-debuginfo-cold-blocks to main:

The debuginfo analyses are written with the assumption that the order of
instructions in the VCode is the order of instructions in the final
machine ocde. This was previously a strong invariant, until we
introduced support for cold blocks. Cold blocks are implemented by
reordering during emission, because the VCode ordering has other
requirements related to lowering (responding def-use dependencies in the
reverse pass), so it is much simpler to reorder instructions at the last
moment. Unfortunately, this causes the breakage we now see.

This commit fixes the issue by skipping all cold instructions when
emitting value-label ranges (which are translated into debuginfo). This
means that variables defined in cold blocks will not have DWARF
metadata. But cold blocks are usually compiler-inserted slowpaths, not
user code, so this is probably OK. Debuginfo is always best-effort, so
in any case this does not violate any correctness constraints.

Discovered while debugging a test failure in #3733.

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

Please ensure all communication adheres to the code of conduct.
-->

view this post on Zulip Wasmtime GitHub notifications bot (Feb 05 2022 at 06:33):

cfallin updated PR #3769 from fix-debuginfo-cold-blocks to main.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 05 2022 at 07:11):

cfallin updated PR #3769 from fix-debuginfo-cold-blocks to main.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 05 2022 at 07:13):

cfallin updated PR #3769 from fix-debuginfo-cold-blocks to main.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 05 2022 at 07:14):

cfallin edited PR #3769 from fix-debuginfo-cold-blocks to main:

The debuginfo analyses are written with the assumption that the order of
instructions in the VCode is the order of instructions in the final
machine ocde. This was previously a strong invariant, until we
introduced support for cold blocks. Cold blocks are implemented by
reordering during emission, because the VCode ordering has other
requirements related to lowering (respecting def-use dependencies in the
reverse pass), so it is much simpler to reorder instructions at the last
moment. Unfortunately, this causes the breakage we now see.

This commit fixes the issue by skipping all cold instructions when
emitting value-label ranges (which are translated into debuginfo). This
means that variables defined in cold blocks will not have DWARF
metadata. But cold blocks are usually compiler-inserted slowpaths, not
user code, so this is probably OK. Debuginfo is always best-effort, so
in any case this does not violate any correctness constraints.

Discovered while debugging a test failure in #3733.

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

Please ensure all communication adheres to the code of conduct.
-->

view this post on Zulip Wasmtime GitHub notifications bot (Feb 05 2022 at 07:15):

cfallin updated PR #3769 from fix-debuginfo-cold-blocks to main.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 07 2022 at 16:58):

fitzgen submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 07 2022 at 16:58):

fitzgen merged PR #3769.


Last updated: Dec 23 2024 at 12:05 UTC