Skip to content

Move to legalization in the JS backend (emscripten) #3873

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 1 commit into from
Oct 27, 2015

Conversation

kripken
Copy link
Member

@kripken kripken commented Oct 23, 2015

This is part of 3 pulls that move us to performing legalization in the backend. This moves the PNaCl passes from lib/Transform to the backend dir.

Benefits:

  1. Smaller/more focused diff against upstream LLVM, as more of our different code is in our target dir.
  2. Simpler compilation process. Instead of telling opt to legalize, we just call the backend and it does it all for us.
  3. Puts us within striking distance of doing everything in the backend, i.e., avoiding any call to opt beforehand. This should give a significant speedup to compile times.

Downsides:

  1. Apparently doing the PNaCl passes in the backend leads to different results in some cases. This seems like an LLVM oddity - you would think that save/load of an LLVM module would have no effect, but it does. This led to different IR being run through the backend, which uncovered a few real bugs. They should all be fixed, but apparently this change does come with risks.

@kripken
Copy link
Member Author

kripken commented Oct 23, 2015

@sunfishcode, @juj, thoughts?

@juj
Copy link
Collaborator

juj commented Oct 26, 2015

Sounds very reasonable to me, although I'm probably not the expert to comment on the details here. It sounds like we want to pull out call to opt altogether before compilation for speed benefits? Does that look feasible on the long run?

@kripken
Copy link
Member Author

kripken commented Oct 26, 2015

Yes, that's the target - once we no longer call opt, llc and instead just call llc, we'll avoid serializing and deserializing the entire LLVM module. This should be easy after this pull. (Previous step was removing llvm-link, we used to have llvm-link, opt, llc.)

@sunfishcode
Copy link
Collaborator

This sounds reasonable to me.

Module serialization/deserialization can change the use-list order. There are command-line parameters to opt which request that it preserve the use-list order, however it's still preferable to fix any bugs due to depending on use-list order anyway, because there is no guarantee that uses will be in any particular order in any case.

@kripken
Copy link
Member Author

kripken commented Oct 27, 2015

Aside from use-list order changes, I noticed that ExpandVarArgs creates a bitcast on each function (that has varargs), as it creates legalized versions of them. But if the function is not used, it still has a use, in that dangling bitcast. Apparently serialization/deserialization fixes that, since the bitcast "goes away". I guess that's a bug in ExpandVarArgs? (I avoided it by making it ignore functions with no uses.)

@kripken kripken merged commit 4348ec6 into incoming Oct 27, 2015
@kripken kripken deleted the legalize-in-backend branch October 27, 2015 01:28
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