- User Since
- Sep 16 2016, 10:22 AM (100 w, 2 d)
Fri, Aug 17
Thu, Aug 16
Wed, Aug 15
Please ignore this change, I had thought we wanted size_t to match either int32_t or int64_t, but it turns out that isn't a requirement and that code that expects this to be true needs fixing.
Tue, Aug 14
- add check for -r
Perhaps I should make this -z compress although I'm not sure what the policy is in using -z keywords vs regular flags.
- fix type mismatch
- add test
Wed, Aug 8
Hmm. I don't know the convention. Not a huge and of "The" prefix. LGTM otherwise.
I think you are right about the optimized builds. One thing I thought we might want to preserve is that data objects from the same section should be grouped together contiguously.. but I think we can probably do that while at the same time putting it all in a single segment.
Should we change int32_t as well? Or does the above code just need fixing? I'm a little concerned because this code is very portable which implies that WebAssembly might be doing something unusual here.
I'm running into an issue with an internal codebase that assumes that size_t is equivalent to either int32_t or int64_t which this change invalidates.
I agree this feels a bit awkward, but I prefer to custom bounds checking I think.
Tue, Aug 7
- update tests
- update test
I see. This seems a little strange to me. I'm not sure it makes sense for CLANG_DEFAULT_LINKER to effect targets like WebAssembly that only have single supported linker. I'm OK with landing this now but I think we might need to look into a better solution.
Mon, Aug 6
But in the CL description you say "..when configuring clang to use a different linker by default". How is this possible? i.e. do you have a config where these tests are currently failing?
Fri, Aug 3
- update test
Second attempt at this change since I think it makes sense. Otherwise emscripten would need to pass --export=foo and --undefined=foo for each symbol which seems redundant.
- --undefined should work with -r
- revert part
- Handle --undefined like the ELF linker
- Update comment
Ok, lets try that approach: https://reviews.llvm.org/D50279
Oh, I didn't realize that. In that case perhaps we need to bring --undefined in line with the ELF linker?
A bit more detail: Emscripten has flag called "-s EXPORTED_FUNCTIONS=<list>". By default this list includes just main. But those symbols can either come from native code or from JS libraries, so they might not exist and link time. Ideally we would know which ones we need at link time and with --export=foo and --undefined=foo which would force all these symbols to exist. But right now we don't have that knowledge so we use --allow-undefined with --export=foo, which roughly equates to "export these symbols if they are found at link time but don't error out if they are not".
Emscripten uses --export in order to choose which symbols to export. It does this for main too with --export=main. We can't use --undefined in general since (for complicated reasons) some of these symbols might actually be missing at link time.
Thu, Aug 2
rL338703 made this redundant
- update comment
Wed, Aug 1
- remove logging
gererate runtime calls to abort rather than failing to compile
add more tests
Mon, Jul 30
How do you configure clang to use a different linker? It looks like getDefaultLinker() is hardcoded, but maybe I'm missing something.
Wed, Jul 25
https://reviews.llvm.org/D49825 seems to fix it.
FYI, this broke the WebAssembly waterfall, where the library is no longer being installed: https://wasm-stat.us/builders/linux/builds/34191
Mon, Jul 23
Interesting, so you are proposing delaying the error until link time but still producing a valid wasm binary?
- remove overload
I did consider that, but the COFF version is slightly different. Perhaps as a followup to keep this change strictly wasm-only?
- revert change to wasm addLazy
Jul 19 2018
Do did enforcement of this rule increase recently? I wonder if they knew they would break existing code.
When you say not all types are valid to cast to all others.. how would I know which are valid? And how would this code fail if they are invalid? Would llvm bail out during the call to getCastOpcode()?
My goal here is not so much to make it "work" as to not produce invalid webassembly. i.e. it should compile, even it if fails at runtime to do anything useful/expected.
Jul 18 2018
I'm still not sure I understand why writing zero here is not a good idea.
Jul 17 2018
Jul 16 2018
I think we can still do that library option, with this as an intermediate step. This change has the nice property of not requiring an emscripten-side change.
Anybody feeling LG'ing this?
Jul 14 2018
Jul 12 2018
I agree it that the other approach is more elegant. However, I decided to go with this one for now since:
(1) Its a smaller incremental change
(2) Hopefully all this code will be removed pretty soon anyway
Abandoning in favor of weak linking approach: https://reviews.llvm.org/D49263