Conversation
|
How is tracking the input location/offset of each IR node any different than tracking the input source position? why does it need to be done differently? |
|
By If there is also a tracking of Long-term, I think we should remove the source maps tracking logic, once DWARF is stable. We can emit a source map using DWARF at the end of the pipeline if that is still useful. |
|
Right, but i mean to track |
|
Wait nevermind, I just looked at the code and I'm completely wrong about that. I could have sworn we had done it that way. So I guess the question is, why not just have the info on each node to avoid the map insertion cost? For source maps, the information is stored separately so we want the O(1) lookup but since this information is available at node creation time I think we can store it on the node instead. |
| currFunction->debugLocations[curr] = *currDebugLocation.begin(); | ||
| } | ||
| if (DWARF && currFunction) { | ||
| currFunction->binaryLocations[curr] = startPos - codeSectionLocation; |
There was a problem hiding this comment.
so concretely, could this be curr->binaryLocation = startPos - codeSectionLocation?
| } | ||
| // show a binary position, if there is one | ||
| if (debugInfo) { | ||
| auto iter = currFunction->binaryLocations.find(curr); |
There was a problem hiding this comment.
and this could be something like o << "code offset " << curr->binaryLocation
Efficiency in the non-debug case, so the nodes are as small as possible. |
|
Also for the debug case, many nodes may not have any debug info, so we save extra size on them too. |
|
That's a good point; although since the offset is just a 32-bit number, and the cost of a map entry is at least the pointer-sized key plus the value (plus any overhead; bucket chain?), there would have to be info for fewer than 1/3 of the nodes for that to be an improvement. |
|
(by the way I don't have a strong opinion about the map vs extra node space, we could go either way on that unless the cost starts getting noticeable). |
|
Yeah, size overhead might be a tradeoff. I lean towards a separate map, because it keeps the non-debug build faster and leaner, and that's important for iteration speed, and there's a chance it'll even be faster for debug builds (as you said, 1/3 or so may be the cutoff point, and at least in an opt build with debug info I'd bet its even fewer). |
|
I think in debug builds it will always be a little slower to use a map. In the code here we are doing a map lookup in several places where we could just be loading from a structure (one that's likely already hot in the cache). But you are right about the non-debug builds. Anyway this is fine for now, it wouldn't be a big deal to change yet if we wanted to later. |
This imports LLVM code for DWARF handling. That code has the Apache 2 license like us. It's also the same code used to emit DWARF in the common toolchain, so it seems like a safe choice. This adds two passes: --dwarfdump which runs the same code LLVM runs for llvm-dwarfdump. This shows we can parse it ok, and will be useful for debugging. And --dwarfupdate writes out the DWARF sections (unchanged from what we read, so it just roundtrips - for updating we need #2515). This puts LLVM in thirdparty which is added here. All the LLVM code is behind USE_LLVM_DWARF, which is on by default, but off in JS for now, as it increases code size by 20%. This current approach imports the LLVM files directly. This is not how they are intended to be used, so it required a bunch of local changes - more than I expected actually, for the platform-specific stuff. For now this seems to work, so it may be good enough, but in the long term we may want to switch to linking against libllvm. A downside to doing that is that binaryen users would need to have an LLVM build, and even in the waterfall builds we'd have a problem - while we ship LLVM there anyhow, we constantly update it, which means that binaryen would need to be on latest llvm all the time too (which otherwise, given DWARF is quite stable, we might not need to constantly update). An even larger issue is that as I did this work I learned about how DWARF works in LLVM, and while the reading code is easy to reuse, the writing code is trickier. The main code path is heavily integrated with the MC layer, which we don't have - we might want to create a "fake MC layer" for that, but it sounds hard. Instead, there is the YAML path which is used mostly for testing, and which can convert DWARF to and from YAML and from binary. Using the non-YAML parts there, we can convert binary DWARF to the YAML layer's nice Info data, then convert that to binary. This works, however, this is not the path LLVM uses normally, and it supports only some basic DWARF sections - I had to add ranges support, in fact. So if we need more complex things, we may end up needing to use the MC layer approach, or consider some other DWARF library. However, hopefully that should not affect the core binaryen code which just calls a library for DWARF stuff. Helps #2400
| getInt32(); // magic | ||
| getInt32(); // version | ||
| bool has = false; | ||
| while (more()) { |
There was a problem hiding this comment.
It seems kind of crazy that we don't already have some kind of general section-iteration mechanism. But I guess that's not the kind of thing Binaryen is usually used for...
This is ok for now (assuming it doesn't duplicate something we do have elsewehere) but let's try not to write this code again in the future.
|
|
||
| std::vector<UserSection> userSections; | ||
|
|
||
| // Source maps debug info. |
There was a problem hiding this comment.
Maybe a separate change but we should probably rename these fields to make it more clear.
There was a problem hiding this comment.
I hope to remove the source maps code in the not far future, which would remove any confusion between these fields. (Once dwarf line numbers works, we can use that for source maps.)
2 participants
Optionally track the binary format code section offsets,
that is, when loading a binary, remember where each IR
node was read from. This is necessary for DWARF
debug info, as these are the offsets DWARF refers to.
(Note that eventually we may want to do something
else, like first read the DWARF and only then add
debug info annotations into the IR in a more LLVM-like
manner, but this is more straightforward and should be
enough to update debug lines and ranges).
This tracking adds noticeable overhead - every single
IR node adds an entry in a map - so avoid it unless
actually necessary. Specifically, if the user passes in
-gand there are actually DWARF sections in thebinary, and we are not about to remove those sections,
then we need it.
Print binary format code section offsets in text, when
printing with
-g. This will help debug and test dwarfsupport. It looks like
;; code offset: 0x7as an annotation right before each node.
Also add support for
-ginwasm-opttests (unlikea pass, it has just one
-as a prefix).Helps #2400