rust-lang / rustc_codegen_cranelift

Cranelift based backend for rustc
Apache License 2.0
1.62k stars 101 forks source link

Support imgui related abi #1129

Closed Uriopass closed 3 years ago

Uriopass commented 3 years ago

When trying to compile imgui-rs using cranelift, the build succeeds however there are some heavy artefacts.

Screen of the artefacts in my game, left: expected, right: actual: image

However when building, the only warnings are:

warning: Argument of type `sys::ImVec2` with pass mode `ByValPair(types::F32, types::F32)` is not yet supported for non-rust abi `"C"`. Calling this function may result in a crash.

(x32)

It seems like implementing ByValPair should do the trick, but I'm not sure what it would implicate.

bjorn3 commented 3 years ago

Interesting. It would expect the C abi incompatibility to crash your program when it is hit, not use a different background color. Fixing this abi incompatibility is not something that I can just do in a few hours. I have been planning to do a complete overhaul of the abi handling code in cg_clif to use the same abi calculation as cg_llvm. This requires some refactorings on the rustc side though. I have already started this about a month ago, but was busy with midterm exams for the past few weeks. Now that they are over I plan to continue on this soon.

Uriopass commented 3 years ago

It surprised me a lot too! I have no idea how it doesn't crash. Two imgui windows are missing however a lot of things work as expected, I guess it just stops rendering at some point and a half finished ui appears.

I have been planning to do a complete overhaul of the abi handling code in cg_clif to use the same abi calculation as cg_llvm. This requires some refactorings on the rustc side though. I have already started this about a month ago, but was busy with midterm exams for the past few weeks. Now that they are over I plan to continue on this soon.

I expected that the ByValPair would just me some small layer over the ByVal, but I guess abi is more complicated. :grin: Take your time, and thank you for your work!

bjorn3 commented 3 years ago

I expected that the ByValPair would just me some small layer over the ByVal, but I guess abi is more complicated. 😁

The x86_64 SystemV abi likes to aggressively pack structs into a small amount of registers. For example (u64, u64) would be stuffed into two registers, while (u8, u8) would only use a single register. No warning triggering doesn't mean that the abi is correct by the way.

Take your time, and thank you for your work!

Thanks a lot!

bjorn3 commented 3 years ago

@Uriopass Can you try the abi_compat branch? This branch should have fixed all abi incompatibilities I know of except for a small one that I don't think really matters on x86_64. If this isn't fixed please also tell me the exact commit you used. I may have fixed this abi incompatibility in the mean time.

Uriopass commented 3 years ago

Looks like it works! :tada:

manywindows

The incremental compile also takes one second less when adding a print to my main (from 8.7s to 7.8s). Also, does the cargo.sh script use the linker provided in the ~/.cargo/config file ?
EDIT: Looks like it doesn't. running using RUSTFLAGS="-Clink-arg=-fuse-ld=lld" ~/cranelift/cargo.sh run gives another 1.5s of improvement for patched incremental (from 7.8s to 6.26s)

The audio is heavily distorted though. I'm not sure if it has to do with abi (from cpal ?) or some weird rodio concurrency things. Expected (cargo run):
https://user-images.githubusercontent.com/5420739/106354170-f27e4780-62ef-11eb-9ba9-a5fd6a6b270c.mp4

Actual (cranelift built): :warning: very loud :warning: https://user-images.githubusercontent.com/5420739/106354176-fd38dc80-62ef-11eb-99f4-22d465513dc0.mp4

I might investigate a bit more the day I want audio working with cranelift :grin: Very nice work!

bjorn3 commented 3 years ago

Also, does the cargo.sh script use the linker provided in the ~/.cargo/config file ?

Yes, it is just a thin wrapper around cargo that uses cg_clif instead of rustc and adds a jit and lazy-jit subcommand.

The audio is heavily distorted though. I'm not sure if it has to do with abi (from cpal ?) or some weird rodio concurrency things.

Does it print any warnings to stdout like buffer under run? If so that would indicate that it is simply to slow with feeding new audio input to alsa. This may be due to the global lock I use to emulate atomics. In that case I would expect it to be more of a stuttering with clearly recognizable parts in between the pauses though. If that isn't the problem it may be helpful to make a smaller reproducing example. Maybe it also happens with one of the rodio or cpal example?

bjorn3 commented 3 years ago

EDIT: Looks like it doesn't. running using RUSTFLAGS="-Clink-arg=-fuse-ld=lld" ~/cranelift/cargo.sh run gives another 1.5s of improvement for patched incremental (from 7.8s to 6.26s)

Huh???

Uriopass commented 3 years ago

I thought I had my config misconfigured but... still for incr patched:


no ~/.cargo/config: cargo build: ~11.5s
cranelift build: ~7.7s


with ~/.cargo/config set to this:

[target.x86_64-unknown-linux-gnu]
rustflags = ["-Clink-arg=-fuse-ld=lld"]

cargo build: ~8.5s cranelift build: ~7.7s (doesn't rebuild compared to no config)


with RUSTFLAGS="-Clink-arg=-fuse-ld=lld" prefix and no ~/.cargo/config:

cargo build: ~8.5s (doesn't rebuild compared to with config) cranelift build: ~5.5s

Maybe the target I'm using is wrong?

Does it print any warnings to stdout like buffer under run?

No prints at all, I'll see if I can replicate with cpal examples. EDIT: no problems with cpal examples. I'll try rodio examples. EDIT2: no problems with rodio examples. I'll try to extract my audio code to something simpler and reproducible.

bjorn3 commented 3 years ago

Can you show executed rustc command for the final executable for both cases with cg_clif? (You can pass -v to cargo.sh to see it)

Uriopass commented 3 years ago

~/.cargo/config commented out:

Running `/media/uriopass/Data/Programmation/Rust/rustc_codegen_cranelift/build/bin/cg_clif --crate-name native_app --edition=2018 native_app/src/main.rs --error-format=json --json=diagnostic-rendered-ansi --crate-type bin --emit=dep-info,link -C embed-bitcode=no -C debuginfo=2 -C metadata=568f1b1f6be9f40d -C extra-filename=-568f1b1f6be9f40d --out-dir /home/uriopass/egregoria/target/debug/deps -C incremental=/home/uriopass/egregoria/target/debug/incremental -L dependency=/home/uriopass/egregoria/target/debug/deps --extern bytemuck=/home/uriopass/egregoria/target/debug/deps/libbytemuck-a71cd8e1c2042558.rlib --extern common=/home/uriopass/egregoria/target/debug/deps/libcommon-606830b0eeae625e.rlib --extern egregoria=/home/uriopass/egregoria/target/debug/deps/libegregoria-1eb1c891f775ddb1.rlib --extern flat_spatial=/home/uriopass/egregoria/target/debug/deps/libflat_spatial-0cefeac7c9b8e795.rlib --extern futures=/home/uriopass/egregoria/target/debug/deps/libfutures-de46680800a7e144.rlib --extern geom=/home/uriopass/egregoria/target/debug/deps/libgeom-13e9f83dde3f68d0.rlib --extern imgui=/home/uriopass/egregoria/target/debug/deps/libimgui-4777c9c92ad9b4e8.rlib --extern imgui_inspect=/home/uriopass/egregoria/target/debug/deps/libimgui_inspect-ea53d4f0ed2d8779.rlib --extern imgui_inspect_derive=/home/uriopass/egregoria/target/debug/deps/libimgui_inspect_derive-ca069d9b77316dc3.so --extern imgui_wgpu=/home/uriopass/egregoria/target/debug/deps/libimgui_wgpu-c143d6e5a78a54c1.rlib --extern imgui_winit_support=/home/uriopass/egregoria/target/debug/deps/libimgui_winit_support-1eda872f3a19a970.rlib --extern inline_tweak=/home/uriopass/egregoria/target/debug/deps/libinline_tweak-9acdcd9a4d43cf42.rlib --extern legion=/home/uriopass/egregoria/target/debug/deps/liblegion-7e05bb58fe656e70.rlib --extern log=/home/uriopass/egregoria/target/debug/deps/liblog-234353b44b36cc2c.rlib --extern log_panics=/home/uriopass/egregoria/target/debug/deps/liblog_panics-83ce70dc1d8b2398.rlib --extern map_model=/home/uriopass/egregoria/target/debug/deps/libmap_model-48364653c509a367.rlib --extern mint=/home/uriopass/egregoria/target/debug/deps/libmint-432b89ee772bd44f.rlib --extern ordered_float=/home/uriopass/egregoria/target/debug/deps/libordered_float-ee0466e3fea71342.rlib --extern rodio=/home/uriopass/egregoria/target/debug/deps/librodio-4df3296513049618.rlib --extern serde=/home/uriopass/egregoria/target/debug/deps/libserde-eed02fb19ea6a60a.rlib --extern slotmap=/home/uriopass/egregoria/target/debug/deps/libslotmap-435f070810af592d.rlib --extern wgpu_engine=/home/uriopass/egregoria/target/debug/deps/libwgpu_engine-727c17b9af991c50.rlib --extern winit=/home/uriopass/egregoria/target/debug/deps/libwinit-c9390b206f791d5d.rlib -L native=/home/uriopass/egregoria/target/debug/build/imgui-sys-f539c04a0f24cbc3/out -L native=/home/uriopass/egregoria/target/debug/build/mlua-f3f984a93896c59c/out/lua-build/lib -L native=/usr/lib/x86_64-linux-gnu -L native=/usr/lib/x86_64-linux-gnu`

with ~/.cargo/config: (identical to previous)

Running `/media/uriopass/Data/Programmation/Rust/rustc_codegen_cranelift/build/bin/cg_clif --crate-name native_app --edition=2018 native_app/src/main.rs --error-format=json --json=diagnostic-rendered-ansi --crate-type bin --emit=dep-info,link -C embed-bitcode=no -C debuginfo=2 -C metadata=568f1b1f6be9f40d -C extra-filename=-568f1b1f6be9f40d --out-dir /home/uriopass/egregoria/target/debug/deps -C incremental=/home/uriopass/egregoria/target/debug/incremental -L dependency=/home/uriopass/egregoria/target/debug/deps --extern bytemuck=/home/uriopass/egregoria/target/debug/deps/libbytemuck-a71cd8e1c2042558.rlib --extern common=/home/uriopass/egregoria/target/debug/deps/libcommon-606830b0eeae625e.rlib --extern egregoria=/home/uriopass/egregoria/target/debug/deps/libegregoria-1eb1c891f775ddb1.rlib --extern flat_spatial=/home/uriopass/egregoria/target/debug/deps/libflat_spatial-0cefeac7c9b8e795.rlib --extern futures=/home/uriopass/egregoria/target/debug/deps/libfutures-de46680800a7e144.rlib --extern geom=/home/uriopass/egregoria/target/debug/deps/libgeom-13e9f83dde3f68d0.rlib --extern imgui=/home/uriopass/egregoria/target/debug/deps/libimgui-4777c9c92ad9b4e8.rlib --extern imgui_inspect=/home/uriopass/egregoria/target/debug/deps/libimgui_inspect-ea53d4f0ed2d8779.rlib --extern imgui_inspect_derive=/home/uriopass/egregoria/target/debug/deps/libimgui_inspect_derive-ca069d9b77316dc3.so --extern imgui_wgpu=/home/uriopass/egregoria/target/debug/deps/libimgui_wgpu-c143d6e5a78a54c1.rlib --extern imgui_winit_support=/home/uriopass/egregoria/target/debug/deps/libimgui_winit_support-1eda872f3a19a970.rlib --extern inline_tweak=/home/uriopass/egregoria/target/debug/deps/libinline_tweak-9acdcd9a4d43cf42.rlib --extern legion=/home/uriopass/egregoria/target/debug/deps/liblegion-7e05bb58fe656e70.rlib --extern log=/home/uriopass/egregoria/target/debug/deps/liblog-234353b44b36cc2c.rlib --extern log_panics=/home/uriopass/egregoria/target/debug/deps/liblog_panics-83ce70dc1d8b2398.rlib --extern map_model=/home/uriopass/egregoria/target/debug/deps/libmap_model-48364653c509a367.rlib --extern mint=/home/uriopass/egregoria/target/debug/deps/libmint-432b89ee772bd44f.rlib --extern ordered_float=/home/uriopass/egregoria/target/debug/deps/libordered_float-ee0466e3fea71342.rlib --extern rodio=/home/uriopass/egregoria/target/debug/deps/librodio-4df3296513049618.rlib --extern serde=/home/uriopass/egregoria/target/debug/deps/libserde-eed02fb19ea6a60a.rlib --extern slotmap=/home/uriopass/egregoria/target/debug/deps/libslotmap-435f070810af592d.rlib --extern wgpu_engine=/home/uriopass/egregoria/target/debug/deps/libwgpu_engine-727c17b9af991c50.rlib --extern winit=/home/uriopass/egregoria/target/debug/deps/libwinit-c9390b206f791d5d.rlib -L native=/home/uriopass/egregoria/target/debug/build/imgui-sys-f539c04a0f24cbc3/out -L native=/home/uriopass/egregoria/target/debug/build/mlua-f3f984a93896c59c/out/lua-build/lib -L native=/usr/lib/x86_64-linux-gnu -L native=/usr/lib/x86_64-linux-gnu`

With prefix:

Running `/media/uriopass/Data/Programmation/Rust/rustc_codegen_cranelift/build/bin/cg_clif --crate-name native_app --edition=2018 native_app/src/main.rs --error-format=json --json=diagnostic-rendered-ansi --crate-type bin --emit=dep-info,link -C embed-bitcode=no -C debuginfo=2 -C metadata=568f1b1f6be9f40d -C extra-filename=-568f1b1f6be9f40d --out-dir /home/uriopass/egregoria/target/debug/deps -C incremental=/home/uriopass/egregoria/target/debug/incremental -L dependency=/home/uriopass/egregoria/target/debug/deps --extern bytemuck=/home/uriopass/egregoria/target/debug/deps/libbytemuck-a71cd8e1c2042558.rlib --extern common=/home/uriopass/egregoria/target/debug/deps/libcommon-606830b0eeae625e.rlib --extern egregoria=/home/uriopass/egregoria/target/debug/deps/libegregoria-1eb1c891f775ddb1.rlib --extern flat_spatial=/home/uriopass/egregoria/target/debug/deps/libflat_spatial-0cefeac7c9b8e795.rlib --extern futures=/home/uriopass/egregoria/target/debug/deps/libfutures-de46680800a7e144.rlib --extern geom=/home/uriopass/egregoria/target/debug/deps/libgeom-13e9f83dde3f68d0.rlib --extern imgui=/home/uriopass/egregoria/target/debug/deps/libimgui-4777c9c92ad9b4e8.rlib --extern imgui_inspect=/home/uriopass/egregoria/target/debug/deps/libimgui_inspect-ea53d4f0ed2d8779.rlib --extern imgui_inspect_derive=/home/uriopass/egregoria/target/debug/deps/libimgui_inspect_derive-ca069d9b77316dc3.so --extern imgui_wgpu=/home/uriopass/egregoria/target/debug/deps/libimgui_wgpu-c143d6e5a78a54c1.rlib --extern imgui_winit_support=/home/uriopass/egregoria/target/debug/deps/libimgui_winit_support-1eda872f3a19a970.rlib --extern inline_tweak=/home/uriopass/egregoria/target/debug/deps/libinline_tweak-9acdcd9a4d43cf42.rlib --extern legion=/home/uriopass/egregoria/target/debug/deps/liblegion-7e05bb58fe656e70.rlib --extern log=/home/uriopass/egregoria/target/debug/deps/liblog-234353b44b36cc2c.rlib --extern log_panics=/home/uriopass/egregoria/target/debug/deps/liblog_panics-83ce70dc1d8b2398.rlib --extern map_model=/home/uriopass/egregoria/target/debug/deps/libmap_model-48364653c509a367.rlib --extern mint=/home/uriopass/egregoria/target/debug/deps/libmint-432b89ee772bd44f.rlib --extern ordered_float=/home/uriopass/egregoria/target/debug/deps/libordered_float-ee0466e3fea71342.rlib --extern rodio=/home/uriopass/egregoria/target/debug/deps/librodio-4df3296513049618.rlib --extern serde=/home/uriopass/egregoria/target/debug/deps/libserde-eed02fb19ea6a60a.rlib --extern slotmap=/home/uriopass/egregoria/target/debug/deps/libslotmap-435f070810af592d.rlib --extern wgpu_engine=/home/uriopass/egregoria/target/debug/deps/libwgpu_engine-727c17b9af991c50.rlib --extern winit=/home/uriopass/egregoria/target/debug/deps/libwinit-c9390b206f791d5d.rlib -Clink-arg=-fuse-ld=lld -L native=/home/uriopass/egregoria/target/debug/build/imgui-sys-f539c04a0f24cbc3/out -L native=/home/uriopass/egregoria/target/debug/build/mlua-f3f984a93896c59c/out/lua-build/lib -L native=/usr/lib/x86_64-linux-gnu -L native=/usr/lib/x86_64-linux-gnu`
bjorn3 commented 3 years ago

cargo.sh always sets RUSTFLAGS, even if it is just a space in most cases. This probably overrides .cargo/config.

Uriopass commented 3 years ago

Replacing this line in config.sh:56

export RUSTFLAGS=$linker" "$RUSTFLAGS

with

if [ ! -z $linker ]; then
    export RUSTFLAGS=$linker" "$RUSTFLAGS
fi

did the trick for me :)

Uriopass commented 3 years ago

Couldn't reproduce the audio issue :/ I'm using a lot of atomics though so I guess it might help if they were native some day.

bjorn3 commented 3 years ago

Cranelift is currently moving towards a new backend framework. The new x86_64 backend supports atomic instructions, but the old one doesn't. I moved cg_clif to the new backend by default a few days ago, but I want to preserve compatibility with the old backend for a while just in case a serious problem shows up. Once I remove support for the old backend I will switch to native atomic instructions.