Clean up WASM_BACKEND usage in emcc.py #11967
Conversation
tools/shared.py
Outdated
| @@ -1372,61 +1371,6 @@ def add_emscripten_metadata(js_file, wasm_file): | |||
| f.write(contents) | |||
| f.write(orig[8:]) | |||
|
|
|||
| @staticmethod | |||
| def add_dylink_section(wasm_file, needed_dynlibs): | |||
There was a problem hiding this comment.
I have a PR in progress that modifies this method to work with the wasm backend. So can you maybe leave it in for now?
There was a problem hiding this comment.
Ok, I'll restore add_dylink_section.
| @@ -2876,12 +2723,6 @@ def do_binaryen(target, asm_target, options, memfile, wasm_binary_target, | |||
| building.save_intermediate(wasm_binary_target, 'pre-ctors.wasm') | |||
| building.eval_ctors(final, wasm_binary_target, binaryen_bin, debug_info=intermediate_debug_info) | |||
|
|
|||
| # after generating the wasm, do some final operations | |||
| if shared.Settings.SIDE_MODULE and not shared.Settings.WASM_BACKEND: | |||
There was a problem hiding this comment.
Could you leave this in but maybe commented or with a TODO?
| if shared.Settings.SIDE_MODULE and not shared.Settings.WASM_BACKEND: | ||
| shared.WebAssembly.add_dylink_section(wasm_binary_target, shared.Settings.RUNTIME_LINKED_LIBS) | ||
| if not DEBUG: | ||
| os.unlink(asm_target) # we don't need the asm.js, it can just confuse |
There was a problem hiding this comment.
You can remove these last two lines though.
| # We allow this error to be supressed by the environment so that we can run the test | ||
| # suite against fastcomp for the time being. | ||
| # See: https://github.com/emscripten-core/emscripten/issues/11319 | ||
| if not shared.Settings.WASM_BACKEND and 'EMCC_ALLOW_FASTCOMP' not in os.environ: |
There was a problem hiding this comment.
Hmm, why would we want to look at EMCC_ALLOW_FASTCOMP? Fastcomp won't work with all the removed code anyhow, and we aren't running those tests.
Or do you mean we want an error for trying to use fastcomp? I don't think it's possible to get to this code location. We error anyhow on trying to set WASM_BACKEND - we always have, before this PR, it's autodetected. And we've even removed that autodetection code?
There was a problem hiding this comment.
I guess I mean we still need the error case... but you can drop the EMCC_ALLOW_FASTCOMP part
This option was already essentially ignored is the value of options.llvm_opts was written but never read by the compiler driver. The last use of thie option was removed back in #11967 but this went unnoticed.
This option was already essentially ignored is the value of options.llvm_opts was written but never read by the compiler driver. The last use of thie option was removed back in #11967 but this went unnoticed.
This option was already essentially ignored is the value of `options.llvm_opts` was written but never read by the compiler driver. The last use of this option was removed back in #11967 but this went unnoticed.
This option was already essentially ignored is the value of `options.llvm_opts` was written but never read by the compiler driver. The last use of this option was removed back in #11967 but this went unnoticed.
See #11860
Btw, just to confirm,
add_dylink_sectionseems no longer used - I guess thewasm backend emits that section for us? (do we not need to update it?)