Linker: speedup and debug info preservation

[Firestar99]

Requires #20 to be merged first. Contains a lot of WIP commits.

Some improvements to the linker by @eddyb:

• mem2reg speedups, which has been known to be exponentially slow
• preserves debug info when the linker does inlining

This PR contains a lot of WIP commits, but it has worked flawlessly in tests and in my Project.

[schell]

@Firestar99 I'm trying this out on renderling...

[schell]

I ran into this compilation error with ahash: tkaitchuck/aHash#200

[schell]

Running cargo update -p ahash seems to have fixed that error.

[schell]

I'm getting a lot of one error (over and over) when building crabslab:

error: cannot cast between pointer types
       from `*f32`
         to `*f32x4`
   --> /Users/schell/.cargo/registry/src/index.crates.io-6f17d22bba15001f/crabslab-0.5.1/src/impl_slab_item/mod.rs:37:25
    |
37  |         let mut array = [T::default(); N];
    |                         ^^^^^^^^^^^^^^^^^
    |

The code is pretty innocuous, and compiled on the previous nightly:

impl<T: SlabItem + Copy + Default, const N: usize> SlabItem for [T; N] {
    const SLAB_SIZE: usize = { <T as SlabItem>::SLAB_SIZE * N };

    fn read_slab(index: usize, slab: &[u32]) -> Self {
        let mut array = [T::default(); N];
        for i in 0..N {
            let j = index + i * T::SLAB_SIZE;
            let t = T::read_slab(j, slab);
            let a: &mut T = crate::array_index_mut(&mut array, i);
            *a = t;
        }
        array
    }

    fn write_slab(&self, mut index: usize, slab: &mut [u32]) -> usize {
        for i in 0..N {
            let n = crate::slice_index(self, i);
            index = n.write_slab(index, slab);
        }
        index
    }
}

[schell]

Now I guess the question is - is this related to changes in rust-gpu or is this something in the standard library or something in rustc?

EDIT: obviously not in the standard library as this is no_std 🤦

[schell]

Adding a [T; N]: Default constraint to the impl and then changing line 37 to this:

        let mut array: [T; N] = Default::default();

got it compiling. After that everything seems to work as expected in my project.

But I think [T::default(); N] should work and it should probably be a synonym for Default::<[T; N]>::default(), do you agree?

[schell]

The great news is that my shaders compile much faster on this branch. 5 seconds vs the previous 40 seconds! An ~87% reduction - that's pretty epic!

[Firestar99]

If [T::default(); N] works the same as vec! does, it would expand to:

let t = ...;
[t.clone(), t.clone(), t.clone(), ..., t.clone()]
drop(t)

But curiously this code from my codebase compiles just fine: EDIT: I don't think any shader actually includes this code

#[inline]
unsafe fn read(from: Self::Transfer) -> Self {
    unsafe {
	let mut ret = [T::default(); N];
	for i in 0..N {
		*ret.index_unchecked_mut(i) = T::read(*from.index_unchecked(i));
	}
	ret
    }
}

Could you check your array_index_mut()? Can it potentially panic, cause that has been my main trouble maker in rust-gpu in the past (don't know about this new branch though). Maybe try spirv_std::arch::IndexUnchecked::index_unchecked?

[schell]

If [T::default(); N] works the same as vec! does...

I don't think it would, as vec! is a macro but [T::default(); N] would get parsed as is.

Could you check your array_index_mut()? Can it potentially panic, cause that has been my main trouble maker in rust-gpu in the past (don't know about this new branch though). Maybe try spirv_std::arch::IndexUnchecked::index_unchecked?

array_index_mut is indeed using index_unchecked_mut, see here.

I'm not going to make a fuss, but something changed, and I think we should be able to initialize arrays this way, but it doesn't block me, and I've already released a new crabslab.

[LegNeato]

I think we should at the very least track down what changed and why and then decide the way forward. Good catch!

[LegNeato]

Minimal repro of what I think is the same issue:

use spirv_std::spirv;

#[spirv(fragment)]
pub fn main() {
    let x = [[1; 2]; 1];
}

Edit: If this is the same issue, it repros without this PR and bisects to 02cd324 . Filed #46.

[LegNeato]

Lame, I screwed up the merge because I used github's UI.

[Firestar99]

Rebased it for you :D

[LegNeato]

@Firestar99 do you think these WIP commits are landable? I actually started working on a different speedup for this section of the code before I remembered about this PR ha.

[LegNeato]

They look fine to me, and there are some tests, and it is fairly self-contained...but 🤷

[Firestar99]

I've been using the non-rebased version for the entirety of my masters and have not encountered a single issue. That's all the guarantees I can give you, but I'd still say this is mergeable.

[LegNeato]

Sweet, let's do it!

[LegNeato]

yolo