Wasmtime GitHub notifications bot (Jan 09 2021 at 21:07): Wasmtime GitHub notifications bot (Jan 09 2021 at 21:07):cfallin commented on Issue #2565:
@yurydelendik with the vmctx-always-alive hack, this seems to pass the gdb DWARF test locally on the new backend now (and I'm watching CI); thanks!
Question above about ranges still stands; I want to make sure I got the conversions right.
Otherwise, though, I think this is ready for review.
Wasmtime GitHub notifications bot (Jan 09 2021 at 21:07):bjorn3 commented on Issue #2565:
Perhaps the simplest would be to just add a hack that emits a value_label psuedo-instruction for vmctx at return -- this would keep it alive and ensure it can be accessed at least from a spillslot if not a register.
It would be nice to have a more general way of keeping a value alive somewhere for debuginfo purposes.
Wasmtime GitHub notifications bot (Jan 09 2021 at 21:12):cfallin commented on Issue #2565:
Perhaps the simplest would be to just add a hack that emits a value_label psuedo-instruction for vmctx at return -- this would keep it alive and ensure it can be accessed at least from a spillslot if not a register.
It would be nice to have a more general way of keeping a value alive somewhere for debuginfo purposes.
Yes, I agree. I actually thought of a really simple/ugly (but maybe more robust) way of doing this -- allocate slots on the stack for all
ValueLabels, and emit actual stores to these slots every time a labeled value is defined (or the label is associated with a new value). Then the DWARF generation can always use SP-offset expressions.This would horribly pessimize code performance, though maybe not too much more than non-optimized debug code already is; but it would be much simpler and more likely to be correct in all sorts of edge cases. I see this as sort of in the same spirit as the idea proposed in #2459 for reftypes tracing: in both cases we instrument the CLIF to provide needed functionality so that the core compiler remains simpler and easier to verify. It is a little unconventional, though, so perhaps for another time.
Wasmtime GitHub notifications bot (Jan 21 2021 at 22:07):cfallin commented on Issue #2565:
@fitzgen or @bnjbvr, would you be able to spare some time to review this? I don't want it to sit too long as it's starting to become stale (I'll rebase across the conflicts when handling any review comments). Thanks!
Wasmtime GitHub notifications bot (Jan 21 2021 at 22:41):fitzgen commented on Issue #2565:
Sure, I can review.
Wasmtime GitHub notifications bot (Jan 22 2021 at 00:03):cfallin commented on Issue #2565:
Thanks! I went over this again quickly and noticed a few typos, and decided to rebase while I was here. Also note there's a ridealong change to remove an outdated doc-comment about the backend pipeline (I can split that out if you'd like).
Wasmtime GitHub notifications bot (Jan 23 2021 at 00:47):cfallin commented on Issue #2565:
@fitzgen I addressed your comments, but while double-checking the CI results one last time, realized the debug tests weren't actually running on CI (GitHub Actions config issue). On fixing that, discovered that the tests I was running locally were a subset of all debug tests, and two of the others were failing. These failures were due to (IMHO) not-super-important differences in codegen, so I added directives to ignore; but let me know if this is still r+ or if you'd like me to dig deeper. Thanks!
Last updated: Oct 23 2024 at 20:03 UTC