Skip to content
This repository was archived by the owner on Apr 28, 2025. It is now read-only.

Switch repository layout to use a virtual manifest #533

Merged
merged 2 commits into from
Apr 18, 2025

Conversation

tgross35
Copy link
Contributor

@tgross35 tgross35 commented Apr 9, 2025

Move the workspace configuration to a virtual manifest. This
reorganization makes a more clear separation between package contents
and support files that don't get distributed. It will also make it
easier to merge this repository with compiler-builtins if that is done
at some point, which had a similar update done in 1.

LICENSE.txt and README.md are symlinked into the new directory to ensure
they get included in the package.

ci: skip-extensive

@tgross35 tgross35 force-pushed the virtual-manifest branch 5 times, most recently from a9b32a3 to 0862b08 Compare April 9, 2025 06:08
@tgross35 tgross35 changed the title Virtual manifest Switch repository layout to use a virtual manifest Apr 9, 2025
@tgross35 tgross35 marked this pull request as ready for review April 9, 2025 06:14
@tgross35 tgross35 enabled auto-merge (rebase) April 9, 2025 06:14
@tgross35 tgross35 force-pushed the virtual-manifest branch 11 times, most recently from 7ff034f to 3b6fc3a Compare April 18, 2025 03:19
@tgross35 tgross35 disabled auto-merge April 18, 2025 03:26
@tgross35 tgross35 force-pushed the virtual-manifest branch 2 times, most recently from f06b2b3 to b40013e Compare April 18, 2025 08:06
tgross35 added a commit to tgross35/rust-libm that referenced this pull request Apr 18, 2025
Benchmarks for [1] seemed to indicate that repository organization for
some reason had an effect on performance, even though the exact same
rustc commands were running (though some with a different order). After
investigating more, it appears that dependencies may have an affect on
inlining thresholds for generic functions.

It is surprising that this happens, we more or less expect that public
functions will be standalone but everything they call will be inlined.
To help ensure this, mark all generic functions `#[inline]` if they
should be merged into the public function.

Zulip discussion at [2].

[1]: rust-lang#533
[2]: https://rust-lang.zulipchat.com/#narrow/channel/182449-t-compiler.2Fhelp/topic/Dependencies.20affecting.20codegen/with/513079387
tgross35 added a commit that referenced this pull request Apr 18, 2025
Benchmarks for [1] seemed to indicate that repository organization for
some reason had an effect on performance, even though the exact same
rustc commands were running (though some with a different order). After
investigating more, it appears that dependencies may have an affect on
inlining thresholds for generic functions.

It is surprising that this happens, we more or less expect that public
functions will be standalone but everything they call will be inlined.
To help ensure this, mark all generic functions `#[inline]` if they
should be merged into the public function.

Zulip discussion at [2].

[1]: #533
[2]: https://rust-lang.zulipchat.com/#narrow/channel/182449-t-compiler.2Fhelp/topic/Dependencies.20affecting.20codegen/with/513079387
@tgross35 tgross35 force-pushed the virtual-manifest branch 2 times, most recently from 18b79bb to c7d18c4 Compare April 18, 2025 20:33
@tgross35
Copy link
Contributor Author

This was originally showing a regression for fmaf128:

icount::icount_bench_fmaf128_group::icount_bench_fmaf128 logspace:setup_fmaf128()
Performance has regressed: Instructions (182151 > 169007) regressed by +7.77719% (>+5.00000)
  Baselines:                      softfloat|softfloat
  Instructions:                      182151|169007               (+7.77719%) [+1.07777x]
  L1 Hits:                           239887|223635               (+7.26720%) [+1.07267x]
  L2 Hits:                               57|72                   (-20.8333%) [-1.26316x]
  RAM Hits:                              66|71                   (-7.04225%) [-1.07576x]
  Total read+write:                  240010|223778               (+7.25362%) [+1.07254x]
  Estimated Cycles:                  242482|226480               (+7.06552%) [+1.07066x]

Plus a number of small improvements.

From zulip discussions at #general > Rust application 60% slower when built with Bazel and https://rust-lang.zulipchat.com/#narrow/channel/182449-t-compiler.2Fhelp/topic/Dependencies.20affecting.20codegen/with/513078956, it seems like that may have come from differences in inlining. #545 should make things more consistent.

@tgross35 tgross35 enabled auto-merge (rebase) April 18, 2025 20:42
@tgross35 tgross35 disabled auto-merge April 18, 2025 20:43
In preparation for switching to a virtual manifest, move the `libm`
crate into a subdirectory and update paths to match.

Updating `Cargo.toml` is done in the next commit so git tracks the moved
file correctly.
Move the workspace configuration to a virtual manifest. This
reorganization makes a more clear separation between package contents
and support files that don't get distributed. It will also make it
easier to merge this repository with `compiler-builtins` which is
planned (builtins had a similar update done in [1]).

LICENSE.txt and README.md are symlinkedinto the new directory to ensure
they get included in the package.

[1]: rust-lang/compiler-builtins#702
@tgross35 tgross35 enabled auto-merge (rebase) April 18, 2025 21:18
@tgross35 tgross35 merged commit 9064743 into rust-lang:master Apr 18, 2025
35 checks passed
@tgross35 tgross35 deleted the virtual-manifest branch April 18, 2025 21:51
tgross35 added a commit that referenced this pull request Apr 18, 2025
Benchmarks for [1] seemed to indicate that repository organization for
some reason had an effect on performance, even though the exact same
rustc commands were running (though some with a different order). After
investigating more, it appears that dependencies may have an affect on
inlining thresholds for generic functions.

It is surprising that this happens, we more or less expect that public
functions will be standalone but everything they call will be inlined.
To help ensure this, mark all generic functions `#[inline]` if they
should be merged into the public function.

Zulip discussion at [2].

[1]: #533
[2]: https://rust-lang.zulipchat.com/#narrow/channel/182449-t-compiler.2Fhelp/topic/Dependencies.20affecting.20codegen/with/513079387
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant