Skip to content

Prevent discard of value names for -g3 or above #8263

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Mar 19, 2019

Conversation

milizhang
Copy link
Contributor

This change is another part that aims to solve #7883, by adding "-fno-discard-value-names" to clang invocation calls from emcc.py, when debug levels >= 3.

It depends on emscripten-core/emscripten-fastcomp-clang#27.

@kripken
Copy link
Member

kripken commented Mar 12, 2019

I tagged 1.38.29 now, which will include the clang change in the binary build. Once the builds are done (usually takes a few hours), we can merge that to here, and then tests should pass.

kripken added a commit that referenced this pull request Mar 13, 2019
The mozilla build infrastructure is currently down due to a problem with the key not working. We're trying to sort that out, but it's not clear how long that will take (and whether it will happen before we replace those bots anyhow). Meanwhile, no new tags are being built, which blocks PRs like #8263 that need a new fastcomp binary. This PR unblocks them by using fastcomp from the waterfall, which is very up to date.

Implications:

* We depend on green builds on the chromium waterfall on linux. Those tend to be green pretty often these days.
* As a result, we are probably going to be more up to date than usual on fastcomp builds - we won't need to manually tag a version to get a new binary build. So this PR may actually get us to a more convenient state anyhow, and we can just keep it until we get the new builders.
Note that this doesn't affect binaryen etc., as the only thing we use from the waterfall on CI is the fastcomp LLVM+clang.

PR also contains:

* Test updates (funnily, we changed how variable names are emitted by asm.js in emscripten-core/emscripten-fastcomp-clang#27 and actually the backend uses code size to decide some things - so variable names actually matter...)
* Remove an old unneeded test for the native optimizer - we don't need to check the EMCC_NATIVE_OPTIMIZER env var any more. (I started to investigate why the test fails with the waterfall build, and it seems like a path issue of some sort, but I doubt it's worth figuring out if we just don't need that test anyhow.)
@kripken
Copy link
Member

kripken commented Mar 13, 2019

Sadly the 1.38.29 tag failed - but we just landed another change that should fix things. If you merge very latest incoming here now then tests should be green.

@kripken
Copy link
Member

kripken commented Mar 14, 2019

Thanks @milizhang , code looks good now. I don't see a merge of latest incoming into this PR, though? Please do that so that the tests can pass (it's the only way to use a new build of LLVM+clang with your changes, otherwise the new flag isn't recognized).

@milizhang
Copy link
Contributor Author

Seems like test_sdl2_timer failed on FireFox, but for a reason I do not really understand as the diff does not seem to show anything meaningful. @kripken How do you think about this?

@kripken
Copy link
Member

kripken commented Mar 18, 2019

That test failure is probably unrelated, it fails randomly now and then and we haven't figured out why yet. So I think everything looks good here! Except I see we forgot to ask you to add yourself to AUTHORS earlier, sorry about that - please do so before merge.

@kripken
Copy link
Member

kripken commented Mar 19, 2019

Perfect, thanks @milizhang!

@kripken kripken merged commit f90cd4d into emscripten-core:incoming Mar 19, 2019
VirtualTim pushed a commit to VirtualTim/emscripten that referenced this pull request May 21, 2019
This change is another part that aims to solve emscripten-core#7883, by adding "-fno-discard-value-names" to clang invocation calls from emcc.py, when debug levels >= 3.

It depends on emscripten-core/emscripten-fastcomp-clang#27.
VirtualTim added a commit to VirtualTim/emscripten that referenced this pull request May 23, 2019
VirtualTim added a commit to VirtualTim/emscripten that referenced this pull request May 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants