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

Apply polymorphization to Instance substitutions #75336

Open
davidtwco opened this issue Aug 9, 2020 · 1 comment
Open

Apply polymorphization to Instance substitutions #75336

davidtwco opened this issue Aug 9, 2020 · 1 comment
Labels
-Zpolymorphize Unstable option: Polymorphization. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@davidtwco
Copy link
Member

davidtwco commented Aug 9, 2020

When #75255 landed, all Instance substitutions were polymorphized which enabled optimisation of the following test case:

// compile-flags:-Zpolymorphize=on -Zprint-mono-items=lazy -Copt-level=1
// ignore-tidy-linelength

#![crate_type = "rlib"]

// Test that only one copy of `Iter::map` and `iter::repeat` are generated.

fn unused<T>() -> u64 {
    42
}

fn foo<T>() {
    let x = [1, 2, 3, std::mem::size_of::<T>()];
    x.iter().map(|_| ());
}

//~ MONO_ITEM fn core::iter[0]::adapters[0]::{{impl}}[29]::new[0]<core::slice[0]::Iter[0]<usize>, pr_75255::foo[0]::{{closure}}[0]<T>> @@ pr_75255-cgu.0[External]
//~ MONO_ITEM fn core::iter[0]::traits[0]::iterator[0]::Iterator[0]::map[0]<core::slice[0]::Iter[0]<usize>, (), pr_75255::foo[0]::{{closure}}[0]<T>> @@ pr_75255-cgu.1[Internal]

fn bar<T>() {
    std::iter::repeat(unused::<T>);
}

//~ MONO_ITEM fn core::iter[0]::sources[0]::repeat[0]<fn() -> u64> @@ pr_75255-cgu.1[Internal]

pub fn dispatch() {
    foo::<String>();
    foo::<Vec<String>>();

    bar::<String>();
    bar::<Vec<String>>();
}

// These are all the items that aren't relevant to the test.
//~ MONO_ITEM fn core::mem[0]::size_of[0]<alloc::string[0]::String[0]> @@ pr_75255-cgu.1[Internal]
//~ MONO_ITEM fn core::mem[0]::size_of[0]<alloc::vec[0]::Vec[0]<alloc::string[0]::String[0]>> @@ pr_75255-cgu.1[Internal]
//~ MONO_ITEM fn core::mem[0]::size_of[0]<usize> @@ pr_75255-cgu.1[Internal]
//~ MONO_ITEM fn core::ptr[0]::const_ptr[0]::{{impl}}[0]::add[0]<usize> @@ pr_75255-cgu.1[Internal]
//~ MONO_ITEM fn core::ptr[0]::const_ptr[0]::{{impl}}[0]::is_null[0]<usize> @@ pr_75255-cgu.1[Internal]
//~ MONO_ITEM fn core::ptr[0]::const_ptr[0]::{{impl}}[0]::offset[0]<usize> @@ pr_75255-cgu.1[Internal]
//~ MONO_ITEM fn core::ptr[0]::const_ptr[0]::{{impl}}[0]::wrapping_add[0]<u8> @@ pr_75255-cgu.1[Internal]
//~ MONO_ITEM fn core::ptr[0]::const_ptr[0]::{{impl}}[0]::wrapping_offset[0]<u8> @@ pr_75255-cgu.1[Internal]
//~ MONO_ITEM fn core::ptr[0]::non_null[0]::{{impl}}[3]::new_unchecked[0]<usize> @@ pr_75255-cgu.1[Internal]
//~ MONO_ITEM fn core::ptr[0]::null[0]<u8> @@ pr_75255-cgu.1[Internal]
//~ MONO_ITEM fn core::slice[0]::{{impl}}[0]::as_ptr[0]<usize> @@ pr_75255-cgu.1[Internal]
//~ MONO_ITEM fn core::slice[0]::{{impl}}[0]::iter[0]<usize> @@ pr_75255-cgu.1[Internal]
//~ MONO_ITEM fn core::slice[0]::{{impl}}[0]::len[0]<usize> @@ pr_75255-cgu.1[Internal]
//~ MONO_ITEM fn pr_75255::dispatch[0] @@ pr_75255-cgu.1[External]
//~ MONO_ITEM fn pr_75255::foo[0]<alloc::string[0]::String[0]> @@ pr_75255-cgu.1[Internal]
//~ MONO_ITEM fn pr_75255::foo[0]<alloc::vec[0]::Vec[0]<alloc::string[0]::String[0]>> @@ pr_75255-cgu.1[Internal]
//~ MONO_ITEM fn pr_75255::bar[0]<alloc::string[0]::String[0]> @@ pr_75255-cgu.1[Internal]
//~ MONO_ITEM fn pr_75255::bar[0]<alloc::vec[0]::Vec[0]<alloc::string[0]::String[0]>> @@ pr_75255-cgu.1[Internal]

However, due to an oversight, it wasn't noticed that this made other tests and stage2 bootstrapping fail when polymorphisation was enabled (cursory investigation suggested regressions were related to #69925), so it was restricted in #75337 to cases that didn't result in regressions. We should investigate what we can do to re-enable this.

@davidtwco davidtwco added the -Zpolymorphize Unstable option: Polymorphization. label Aug 9, 2020
@davidtwco
Copy link
Member Author

#75346 fixes one of the regressions that #75255 introduced (a out-of-bounds substitution when compiling rustc_metadata in stage2) but stage2 compilation later fails with a linker error compiling rustc_driver and it fixes neither of the UI test failures.

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Aug 10, 2020
…xes, r=eddyb

instance: only polymorphize upvar substs

This PR restricts the substitution polymorphization added in rust-lang#75255 to only apply to the tupled upvar substitution, rather than all substitutions, fixing a bunch of regressions when polymorphization is
enabled.

Due to an oversight on my part, when landing rust-lang#75260 and rust-lang#75255, some tests started failing when polymorphization was enabled that I didn't notice until after landing - this PR fixes the regressions from rust-lang#75255. rust-lang#75336 has been filed to make sure that we don't forget to try make this change again in future, as it does enable some optimisations.

r? @lcnr
@Enselic Enselic added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Dec 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-Zpolymorphize Unstable option: Polymorphization. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

2 participants