Skip to content
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

rust: fix rust cross-compile #89582

Merged
merged 3 commits into from Jun 8, 2020
Merged

Conversation

cleverca22
Copy link
Contributor

reasoning:
sjlj (short jump long jump) exception handling makes no sense on x86_64, it's forcably slowing programs down as it produces a constant overhead. On x86_64 we have SEH (Structured Exception Handling) and we should use that. On i686, we do not have SEH, and have to use sjlj with dwarf2. Hence it's now conditional on x86_32

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

reasoning:
sjlj (short jump long jump) exception handling makes no sense on x86_64, it's forcably slowing programs down as it produces a constant overhead. On x86_64 we have SEH (Structured Exception Handling) and we should use that. On i686, we do not have SEH, and have to use sjlj with dwarf2. Hence it's now conditional on x86_32
@angerman
Copy link
Contributor

angerman commented Jun 6, 2020

/cc @Ericson2314

@angerman
Copy link
Contributor

angerman commented Jun 6, 2020

refs

In the end building sjlj for x86_64 will just cause a forced performance penalty, as well as make building rustc impossible.

@arcnmx
Copy link
Member

arcnmx commented Jun 7, 2020

I believe --disable-sjlj-exceptions is necessary to cross compile rust on i686 - at least is with the binary releases. It might make sense to be able to customize the exception type in the platform config?

@angerman
Copy link
Contributor

angerman commented Jun 7, 2020

I believe --disable-sjlj-exceptions is necessary to cross compile rust on i686 - at least is with the binary releases. It might make sense to be able to customize the exception type in the platform config?

i686 is 32bit and that’s where this patch leaves sjlj enabled. It only does not force enable sjlj on x86_64.

@Ericson2314
Copy link
Member

Sorry didn't see this yet. I know @cleverca22 has been stuck with this far too long! reading now.

@Ericson2314 Ericson2314 merged commit b668fe8 into NixOS:master Jun 8, 2020
@arcnmx
Copy link
Member

arcnmx commented Jun 8, 2020

i686 is 32bit and that’s where this patch leaves sjlj enabled. It only does not force enable sjlj on x86_64.

Right, what I meant is that this change trades one hard-coded config for another, and that there are similar reasons one might want to change the i686 config flags in a similar way. It seems like it'd be useful to be able to use something like crossSystem.platform.gcc.exceptionHandler = "sjlj" so that the seh/sjlj/dwarf alternatives can be selected where they're needed.

@cleverca22 cleverca22 deleted the fix-rust-cross branch June 8, 2020 05:34
@angerman
Copy link
Contributor

angerman commented Jun 8, 2020

i686 is 32bit and that’s where this patch leaves sjlj enabled. It only does not force enable sjlj on x86_64.

Right, what I meant is that this change trades one hard-coded config for another, and that there are similar reasons one might want to change the i686 config flags in a similar way. It seems like it'd be useful to be able to use something like crossSystem.platform.gcc.exceptionHandler = "sjlj" so that the seh/sjlj/dwarf alternatives can be selected where they're needed.

Fair. It's a bit orthogonal to this change. If you feel like making that change to make the exceptionHandler a parameter, I'm happy to support that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants