sbc100 accepted this revision.
sbc100 added a comment.
This revision is now accepted and ready to land.
Great!
================
Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:68
+ else
+ CmdArgs.push_back("-m wasm32");
+
----------------
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.
================
Comment at: lld/wasm/Driver.cpp:385
+ StringRef s = arg->getValue();
+ if (s == "wasm32")
+ config->is64 = false;
----------------
aardappel wrote:
> dschuff wrote:
> > any particular reason this shouldn't use the more conventional `-m32`/`m64`?
> > edit: nevermind, this is lld not clang. I'll defer to Sam's opinion on lld
> > flags.
> Yup, @sbc100 suggested this.
Yeah it looks like gnu ld and lld elf always accept `-m <arch>` from clang.
================
Comment at: lld/wasm/Driver.cpp:390
+ else
+ error("'" + s + "' not a valid target");
+ }
----------------
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);`
================
Comment at: lld/wasm/InputChunks.cpp:338
+ auto WASM_OPCODE_PTR_CONST =
+ config->is64 ? WASM_OPCODE_I64_CONST : WASM_OPCODE_I32_CONST;
----------------
Just use normal variable names here? `opcode_const` and `opcode_add`?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82130/new/
https://reviews.llvm.org/D82130
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
