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

Add i686-pc-windows-gnullvm target #2960

Closed
jeremyd2019 opened this issue Mar 31, 2024 · 13 comments · Fixed by #2961
Closed

Add i686-pc-windows-gnullvm target #2960

jeremyd2019 opened this issue Mar 31, 2024 · 13 comments · Fixed by #2961
Labels
enhancement New feature or request

Comments

@jeremyd2019
Copy link
Contributor

jeremyd2019 commented Mar 31, 2024

Suggestion

This was not added in #1883 due to oversight according to @mati865:

Bummer, I totally forgot about it so it's not added to windows-rs at all.

Originally posted by @mati865 in msys2/MINGW-packages#20397 (comment)

i686 requires doing the same things as in #1883 and then waiting for the new versions of windows-rs to propagate...

Originally posted by @mati865 in msys2/MINGW-packages#20397 (comment)

Haven't tested but using RUSTFLAGS="--cfg windows_raw_dylib" could work without copying the libs.

Originally posted by @mati865 in msys2/MINGW-packages#20397 (comment)

I feel bad about the proliferation of very similar targets though ☹️ Maybe at some point windows_raw_dylib will be on by default, and we can just say i686-pc-windows-gnullvm is only usable on rust versions that support that?

@jeremyd2019 jeremyd2019 added the enhancement New feature or request label Mar 31, 2024
@jeremyd2019
Copy link
Contributor Author

jeremyd2019 commented Mar 31, 2024

Specifically, I used the libraries from i686-pc-windows-gnu to unblock my build, but apparently one could also use the msvc libraries instead (I know that's what we did for aarch64 before support was added here, as there was and still is no aarch64-pc-windows-gnu). llvm is compatible with either format.

@riverar
Copy link
Collaborator

riverar commented Apr 1, 2024

Last I looked into this, i686 required mingw packages that are configured/built to use DWARF-2 exception handling (instead of SJLJ). Not all hosts (e.g. macOS) at the time had packages that supported this and working with the fragmented ecosystem required a few hoop jumps. I haven't checked recently though.

Regarding libs: I'm not a fan of the idea of feeding non-llvm-generated libs to llvm tooling, feels like a ticking time bomb. It's especially unattractive when we could just use the llvm tooling as intended. (#1961 (comment), #1846 (review)). Happy to reconsider / hear everyone else's thoughts though.

@jeremyd2019
Copy link
Contributor Author

jeremyd2019 commented Apr 1, 2024

Last I looked into this, i686 required mingw packages that are configured/built to use DWARF-2 exception handling (instead of SJLJ).

Hmm, I wonder why that would be. This crate just ships import libraries, right? Those shouldn't have any stack unwinding, or really any code to speak of, with the possible exception of the import thunks, which are a single instruction, of the form:

foo:
    jmp DWORD PTR [__imp_foo]

Even these wouldn't be present in the variant of import libraries used by LLVM or MSVC (so-called "short" import libaries), IIRC.

@riverar
Copy link
Collaborator

riverar commented Apr 1, 2024

Right, the mingw issue is unrelated to libs. But related to the addition of the i686 target.

@jeremyd2019
Copy link
Contributor Author

jeremyd2019 commented Apr 1, 2024

OK. I'm not aware of an llvm-mingw variant that uses any different exception handling mechanism, but perhaps @mstorsjo would know if this is something to be concerned about for -gnullvm as it apparently was for -gnu (his llvm-mingw and the CLANG32 MSYSTEM of msys2 are the toolchains I am familiar with and neither use a different exception handling configuration)

@riverar
Copy link
Collaborator

riverar commented Apr 1, 2024

Oh hm, re-reviewing our cross compile tests (https://github.com/microsoft/windows-rs/blob/master/.github/workflows/cross.yml#L15-L34), I think I'm conflating gnu and gnullvm. Let's try it and see what blows up!

@kennykerr kennykerr added question Further information is requested and removed enhancement New feature or request labels Apr 1, 2024
@jeremyd2019
Copy link
Contributor Author

Let's try it and see what blows up!

Is that an offer or an invitation for me to make a PR? Cause I could probably figure it out just by grepping for other gnullvm references, but I don't really know what I'm doing 😁

@jeremyd2019
Copy link
Contributor Author

jeremyd2019 commented Apr 1, 2024

I added i686-pc-windows-gnullvm to cross.yml in my fork, and it oddly didn't fail. I expected to get the same error I see trying to build rustc itself for that target, that -lwindows.0.x.y is not found.

Oh, crates/libs/targets/Cargo.toml does not have a not(target_abi = "llvm") yet, and nightly supports those target_abi conditions, so it went ahead and worked there. I guess those conditions still don't work in stable.

@mstorsjo
Copy link

mstorsjo commented Apr 2, 2024

Last I looked into this, i686 required mingw packages that are configured/built to use DWARF-2 exception handling (instead of SJLJ). Not all hosts (e.g. macOS) at the time had packages that supported this and working with the fragmented ecosystem required a few hoop jumps. I haven't checked recently though.

For GCC based mingw toolchains, many of them have used SJLJ for i686 for quite some time. IIRC the debian provided ones were using SJLJ until "quite recently": https://salsa.debian.org/mingw-w64-team/gcc-mingw-w64/-/commit/8da432fefb792fbe2f9fec2f15278d02e86e6281

Note how one of the reasons seems to be "to allow the Rust toolchain to provide 32-bit cross-compilers".

So for longer term distributions, I think e.g. Ubuntu LTS switched from SJLJ to Dwarf in this configuration since 22.04.

OK. I'm not aware of an llvm-mingw variant that uses any different exception handling mechanism, but perhaps @mstorsjo would know if this is something to be concerned about for -gnullvm as it apparently was for -gnu (his llvm-mingw and the CLANG32 MSYSTEM of msys2 are the toolchains I am familiar with and neither use a different exception handling configuration)

FWIW, we did use SJLJ for i686 for llvm-mingw earlier as well. Not for matching any other specific toolchain (we're diverging from many existing ones by using UCRT anyway), but because the Dwarf generation for Windows had a couple of bugs earlier.

First I switched from SJLJ to dwarf in 2018 after getting one bug fixed: mstorsjo/llvm-mingw@ca43ab5
Then I had to revert back to SJLJ a couple months later, after a report with another issue with dwarf: mstorsjo/llvm-mingw@f0aa7c4
Then in 2019 I was able to switch back to dwarf after getting that resolved: mstorsjo/llvm-mingw@f9ce177

I guess these qualify as ancient history at this point though :-)

@kennykerr kennykerr added enhancement New feature or request and removed question Further information is requested labels Apr 2, 2024
@mati865
Copy link
Contributor

mati865 commented Apr 2, 2024

Oh, crates/libs/targets/Cargo.toml does not have a not(target_abi = "llvm") yet, and nightly supports those target_abi conditions, so it went ahead and worked there. I guess those conditions still don't work in stable.

Yeah, target_abi will become stable starting with 1.78 but additional changes might be required for Cargo.
Some context is available here: #2515

@jeremyd2019
Copy link
Contributor Author

Oh, crates/libs/targets/Cargo.toml does not have a not(target_abi = "llvm") yet, and nightly supports those target_abi conditions, so it went ahead and worked there. I guess those conditions still don't work in stable.

Yeah, target_abi will become stable starting with 1.78 but additional changes might be required for Cargo. Some context is available here: #2515

Oh, then it might have just started working in 1.78 the way it does in nightly: the conditions in

[target.'cfg(all(target_arch = "x86", target_env = "gnu", not(windows_raw_dylib)))'.dependencies]
would be satisfied by gnullvm as well, and it would have pulled in the i686_gnu lib.

@mati865
Copy link
Contributor

mati865 commented Apr 2, 2024

Looks like it should pull in windows_i686_gnu no matter if building with 1.77, 1,78 or nightly since target_abi is absent.

@jeremyd2019
Copy link
Contributor Author

jeremyd2019 commented Apr 3, 2024

I don't know why, it definitely failed with 1.77, but I am testing with 1.78.0-beta.4 (from https://static.rust-lang.org/dist/2024-03-30/rustc-beta-src.tar.gz) and it is pulling in windows_i686_gnu. So I guess we could just wait for 1.78? 🤔

Oops, spoke too soon, got error finding -lwindows.0.52.0 building cargo.

Ah, windows_i686_gnu-0.52.0/build.rs still had

    if target != "i686-pc-windows-gnu" && target != "i686-uwp-windows-gnu" {
        return;
    }

Just to be clear, I did more build testing, and confirmed that it had nothing to do with 1.78 or nightly, but everything to do with #2774's changes to windows_i686_gnu/build.rs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants