- User Since
- Mar 9 2018, 11:57 AM (142 w, 1 d)
Fri, Nov 13
Thu, Nov 12
After discussion with more Wasm+DWARF interested parties, it became clear that asking producers and consumers of DWARF to treat DW_FORM_addr as anything other than architecture dependent is going to be problematic, so the way forward for now is to make this field 64-bit on wasm64 as intended, and leave it 32-bit on wasm32.
@yurydelendik note that wasm64 is an LLVM-level construct, in general Wasm we may one day have multiple memories, which means that "being 64-bit" is a property of each load/store instruction individually. So assuming we have a DWARF section associated with a possibly mixed mode Wasm module, there's no way to determine the size of DW_AT_low_pc or DW_FORM_addr from the module.
Tue, Nov 10
As for how to make a test for this.. The code before this patch would (I assume) write 4 extra bytes, which would cause dwarfdump to show incorrect information. Our existing test however wasn't affected. So to cover this patch against regression would require some larger dwarfdump example I guess..
@dblaikie Confusingly, these are all different: function pointers at runtime (in a Wasm VM) are 32-bit indices. LLVM function pointers are 64-bit in wasm64 for consistency, but get truncated when lowered in Isel. Then here we have a 3rd kind of code pointer, just for DWARF, since Wasm doesn't have the concept of a pointer to an instruction inside a function (which DWARF needs for DW_AT_low_pc, and we need to relocate).
Note this has not been tested on non-Wasm platforms yet. Wanted to first get feedback if this is a legit way to solve the problem, or if there are better ways.
MC parts generally look good to me.
Fri, Oct 30
The paths in this test only worked on Windows. A fix has been pushed in b093eba08478db387fc2765dee1e1bf2d64fcf68
updated test for --show-form and added TODO
Oct 29 2020
Review fixes and tests.
Oct 1 2020
Sep 17 2020
Sep 11 2020
Aug 12 2020
Aug 10 2020
Nice, fairly unintrusive actually. If you desperately wanted to fix the need for changing W dynamically, you could instead make it allocate a second WasmObjectWriter to write the dwo version? :)
Aug 7 2020
Jul 30 2020
Jul 27 2020
Replace fragile way of finding the extend instruction by just getting the def of the br_table operand.
Jul 21 2020
Jul 20 2020
Ok, so we had incorrectly specified DW_FORM_udata for the label, but AddLabel had previously caused it to be emitted as an int32 anyway? So this doesn't change anything about the DWARF output?
Jul 16 2020
Code review fixes
Jul 13 2020
Made LLD is64 optional, so we can know if is set consistently.
Jul 7 2020
Jul 6 2020
Test uses 32/64-bit specific .o files
Reinstated init/drop defs + test
Added part of LLD data-layout.ll test
Jul 3 2020
@sbc100 any suggestion for which test would be best to add a 64-bit version of? Most important thing is to ensure LLD is outputting 64-bit memory limits
Jul 2 2020
Not much happening here, other than updating the memory limits writing. Probably needs an LLD specific test, @sbc100 ?
Removed unused init/drop intrinsics instead of trying to make them work for wasm64.
Also testing bulk-memory-encoding.s
Jun 29 2020
@tlively I have no idea if my changes in IntrinsicsWebAssembly.td make sense, they merely pass the tests ;) please advise.
Jun 25 2020
types & variables in InputChunks.cpp
- Fixed ISEL for FrameIndex
- Fixed 64-bit conditions in branches (thanks @aheejin!)
- Made the FrameIndex generation code in WebAssemblyRegisterInfo work.
- Made userstack.ll and stack-alignment.ll pass in wasm64.
- Code review fixes.
Jun 22 2020
@dschuff still working on it, it uncovered some issues (as it should :)
I'll likely fork userstack.ll since the majority of lines need changes.
Jun 18 2020
This is a first iteration, probably needs more tests :) I was thinking of forking userstack.ll since it has most __stack_pointer tests, but I am not sure if it's that useful. Needs a test that uses the new wasm-ld flag. Opinions welcome.
Jun 15 2020
Made dedicated supportsWasm64/resolveWasm64 rather than sharing the Wasm32 versions.
Rebased against previous commit
Thanks for the tips. Though frankly in this case I should have tried to make this commit independent, since significant changes in the other one made rebasing a mess.
Jun 12 2020
Added HasAddr32|64 to all the patterns.
@sbc100 Like I said, I think these silent truncations by using explicit types are a maintenance problem which the guidelines mention.
Refactored atomic patterns
as for auto, indeed its says "Use auto if and only if it makes the code more readable or easier to maintain". That was exactly my point when I mentioned "The reason I replaced these with auto is we had a bunch of spots where we had unnecessary truncation because of a uint32_t var = exp that results in a 64-bit value. Using auto avoids these problems, now, and with future changes."
Changed init expr to use all union types, and added code to check for opcode.
Other misc code review.
Fixed linker error due to tablegen adding both generated functions to same #ifdef :(
Jun 11 2020
Jun 8 2020
Added HasAddr64 to patterns, and made it default for instructions.