cfallin opened PR #1575 from test-fixes
to master
:
This change adds SourceLoc information per instruction in a
VCode<Inst>
container, and keeps this information up-to-date across register allocation
and branch reordering. The information is initially collected during
instruction lowering, eventually collected on the MachSection, and finally
provided to the environment that wraps the codegen crate for wasmtime.This PR is based on top of #1570 and #1571 (part of a series fixing tests).
This PR depends on wasmtime/regalloc.rs#50, a change to the register
allocator to provide instruction-granularity info on the rewritten
instruction stream (rather than block-granularity).With the prior PRs applied as well, quite a few more unit tests pass;
the exclusion list in #1526 should be updated if this PR lands first.
cfallin requested bnjbvr and julian-seward1 for a review on PR #1575.
cfallin requested bnjbvr and julian-seward1 for a review on PR #1575.
cfallin edited PR #1575 from test-fixes
to master
:
This change adds SourceLoc information per instruction in a
VCode<Inst>
container, and keeps this information up-to-date across register allocation
and branch reordering. The information is initially collected during
instruction lowering, eventually collected on the MachSection, and finally
provided to the environment that wraps the codegen crate for wasmtime.This PR is based on top of #1570 and #1571 (part of a series fixing tests).
This PR depends on bytecodealliance/regalloc.rs#50, a change to the register
allocator to provide instruction-granularity info on the rewritten
instruction stream (rather than block-granularity).With the prior PRs applied as well, quite a few more unit tests pass;
the exclusion list in #1526 should be updated if this PR lands first.
cfallin edited PR #1575 from test-fixes
to master
:
This change adds SourceLoc information per instruction in a
VCode<Inst>
container, and keeps this information up-to-date across register allocation
and branch reordering. The information is initially collected during
instruction lowering, eventually collected on the MachSection, and finally
provided to the environment that wraps the codegen crate for wasmtime.This PR is based on top of #1570 and #1571 (part of a series fixing tests).
This PR depends on bytecodealliance/regalloc.rs#50, a change to the register
allocator to provide instruction-granularity info on the rewritten
instruction stream (rather than block-granularity). I have not yet
updatedcranelift/codegen/Cargo.toml
; we will need to land that PR
and do a release first.With the prior PRs applied as well, quite a few more unit tests pass;
the exclusion list in #1526 should be updated if this PR lands first.
cfallin edited PR #1575 from test-fixes
to master
:
This change adds SourceLoc information per instruction in a
VCode<Inst>
container, and keeps this information up-to-date across register allocation
and branch reordering. The information is initially collected during
instruction lowering, eventually collected on the MachSection, and finally
provided to the environment that wraps the codegen crate for wasmtime.This PR is based on top of #1570 and #1571 (part of a series fixing tests).
This PR depends on bytecodealliance/regalloc.rs#50, a change to the register
allocator to provide instruction-granularity info on the rewritten
instruction stream (rather than block-granularity). I have not yet
updatedcranelift/codegen/Cargo.toml
; we will need to land that PR
and do a release first.With the prior PRs applied as well, quite a few more unit tests pass;
the exclusion list in #1526 should be updated if this PR lands first.Addresses part of #1523 (debug info), though debug info consists of
more than just source-location mapping.
bjorn3 submitted PR Review.
bjorn3 submitted PR Review.
bjorn3 created PR Review Comment:
Every function is compiled separately without knowledge of the final location in memory, so this can never be absolute. Did you mean relative tot the start of the function?
bjorn3 created PR Review Comment:
There is already a SourceLoc reserved for when the source loc is unknown: SourceLoc::default().
bnjbvr submitted PR Review.
bnjbvr submitted PR Review.
bnjbvr created PR Review Comment:
nit: current
bnjbvr created PR Review Comment:
nit: to remove one extra level of indent, use
expect
on the result of take.
bnjbvr created PR Review Comment:
Could the copy be avoided here? (Either by letting the one client iterate on the sections, or return a Map iterator with
self.sections.iter().map(|section| §ion.srclocs)
or something like this)I think this requires sorting the srclocs before we actually sort them, but this is doable.
bnjbvr created PR Review Comment:
Agreed, it'd be nice to reuse it in lieu of all the
Option<SourceLoc>
in this patch.
bnjbvr created PR Review Comment:
Could
final_srclocs
(and thussrclocs
) store range of instructions mapping to a single srcloc, instead? (Just the start instruction should be enough, since the next entry's start instruction would be next to the previous' entry end)
bnjbvr created PR Review Comment:
nit: double space between
the
andregion
bnjbvr created PR Review Comment:
could you debug_assert that
end >= start
, for sanity?
cfallin submitted PR Review.
cfallin created PR Review Comment:
I thought about this briefly, but it makes the rearranging after regalloc somewhat more complicated: we have to reconstitute the original per-instruction
srcloc
s because the block reordering may have split one of the same-location extents in two. I'd rather avoid that complexity (and branchiness), at the cost of slightly more memory... (but perhaps there's another way I've missed).
cfallin updated PR #1575 from test-fixes
to master
:
This change adds SourceLoc information per instruction in a
VCode<Inst>
container, and keeps this information up-to-date across register allocation
and branch reordering. The information is initially collected during
instruction lowering, eventually collected on the MachSection, and finally
provided to the environment that wraps the codegen crate for wasmtime.This PR is based on top of #1570 and #1571 (part of a series fixing tests).
This PR depends on bytecodealliance/regalloc.rs#50, a change to the register
allocator to provide instruction-granularity info on the rewritten
instruction stream (rather than block-granularity). I have not yet
updatedcranelift/codegen/Cargo.toml
; we will need to land that PR
and do a release first.With the prior PRs applied as well, quite a few more unit tests pass;
the exclusion list in #1526 should be updated if this PR lands first.Addresses part of #1523 (debug info), though debug info consists of
more than just source-location mapping.
cfallin submitted PR Review.
cfallin created PR Review Comment:
Yup, fixed, thanks!
cfallin submitted PR Review.
cfallin created PR Review Comment:
Ah, yes, this is much nicer (as well as the corresponding change to not-present indices on the regalloc side) -- done.
cfallin submitted PR Review.
cfallin created PR Review Comment:
Yup, done; wrote a little custom iterator. (I might have been able to get it to work returning an
impl Iterator<Item = &'a MachSrcLoc>
but getting the types to align for the zero-sections case and otherwise is tricky, since impl-returns must match on all returning paths...)
bnjbvr requested bnjbvr and julian-seward1 for a review on PR #1575.
bnjbvr submitted PR Review.
cfallin updated PR #1575 from test-fixes
to master
:
This change adds SourceLoc information per instruction in a
VCode<Inst>
container, and keeps this information up-to-date across register allocation
and branch reordering. The information is initially collected during
instruction lowering, eventually collected on the MachSection, and finally
provided to the environment that wraps the codegen crate for wasmtime.This PR is based on top of #1570 and #1571 (part of a series fixing tests).
This PR depends on bytecodealliance/regalloc.rs#50, a change to the register
allocator to provide instruction-granularity info on the rewritten
instruction stream (rather than block-granularity). I have not yet
updatedcranelift/codegen/Cargo.toml
; we will need to land that PR
and do a release first.With the prior PRs applied as well, quite a few more unit tests pass;
the exclusion list in #1526 should be updated if this PR lands first.Addresses part of #1523 (debug info), though debug info consists of
more than just source-location mapping.
cfallin updated PR #1575 from test-fixes
to master
:
This change adds SourceLoc information per instruction in a
VCode<Inst>
container, and keeps this information up-to-date across register allocation
and branch reordering. The information is initially collected during
instruction lowering, eventually collected on the MachSection, and finally
provided to the environment that wraps the codegen crate for wasmtime.This PR is based on top of #1570 and #1571 (part of a series fixing tests).
This PR depends on bytecodealliance/regalloc.rs#50, a change to the register
allocator to provide instruction-granularity info on the rewritten
instruction stream (rather than block-granularity). I have not yet
updatedcranelift/codegen/Cargo.toml
; we will need to land that PR
and do a release first.With the prior PRs applied as well, quite a few more unit tests pass;
the exclusion list in #1526 should be updated if this PR lands first.Addresses part of #1523 (debug info), though debug info consists of
more than just source-location mapping.
cfallin merged PR #1575.
bjorn3 submitted PR Review.
bjorn3 created PR Review Comment:
I actually want to know about all instructions, not just those with a non-default srcloc.
cfallin submitted PR Review.
cfallin created PR Review Comment:
Thanks for pointing this out -- you're right! New PR here: #1632
Last updated: Dec 23 2024 at 12:05 UTC