We have 6 globals, all of which except for __table_base are 64-bit under wasm64.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
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.
Yeah I think a 64-bit version of userstack.ll makes sense. (a fork or a second set of CHECKs, whatever you think would be cleaner). There's also stack-alignment.ll which covers dynamic stack adjustment.
lld/wasm/Driver.cpp | ||
---|---|---|
385 | any particular reason this shouldn't use the more conventional -m32/m64? | |
llvm/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp | ||
84–86 | should __table_base stay as i32? |
Great!
clang/lib/Driver/ToolChains/WebAssembly.cpp | ||
---|---|---|
68 | All the other drivers seem to do CmdArgs.push_back("-m"); .. CmdArgs.push_back("xxx"); I would expect these to be two distinct argument.. in fact I'm not sure how it works with once arg that contains a space like this. | |
lld/wasm/Driver.cpp | ||
385 | Yeah it looks like gnu ld and lld elf always accept -m <arch> from clang. | |
390 | It looks like lld ELF inherits the GNU ld terminology and calls this argument "emulation". -m emulation Emulate the emulation linker. You can list the available emulations with the --verbose or -V options. If the -m option is not used, the emulation is taken from the "LDEMULATION" environment variable, if that is defined. Otherwise, the default emulation depends upon how the linker was configured. ` Pretty odd choice of name I think and we probably are ok no to copy that here. But how about I think I prefer the term "architecture" to target. Seems a little more specific. How about: error("invalid target architecture: " + s); | |
lld/wasm/InputChunks.cpp | ||
338 | Just use normal variable names here? opcode_const and opcode_add? |
So the code LGTM, were you going to add to usertest.ll in this CL?
llvm/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp | ||
---|---|---|
84–86 | Oh, right; I'd misread this as setting it just for table_base but I had it backwards; it's exempting table_base. So yeah this is right. |
- 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.
Compiler changes LGTM
llvm/test/CodeGen/WebAssembly/stack-alignment.ll | ||
---|---|---|
1–2 | that -D flag is really nice. |
All the other drivers seem to do CmdArgs.push_back("-m"); .. CmdArgs.push_back("xxx");
I would expect these to be two distinct argument.. in fact I'm not sure how it works with once arg that contains a space like this.