Move feature descriptions out of std.Target.Cpu.Feature #20977
2,093
−1,864
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
squeek502
force-pushed
the
cpufeature-separate-desc
branch
2 times, most recently
from
892b13f
to
67b1a21
Compare
Oof, that is a good question. Currently its not terribly important, this part needs a little bit more thinking anyway. I'm fine with the current changes. |
|
Sorry but I want to instead solve this with #21010 which is a low-priority enhancement. |
|
Would it suffice to add in some |
squeek502
force-pushed
the
cpufeature-separate-desc
branch
from
67b1a21
to
6e0a0c7
Compare
This allows users of std.Target to not include all the description strings in their program if they don't actually use them.
Counting all the bytes of the description strings after they were separated out using this code:
comptime {
@setEvalBranchQuota(10000000);
var total_byte_count: usize = 0;
for (std.enums.values(Arch)) |arch| {
const descs = allFeaturesDescList(arch);
for (descs) |desc| {
total_byte_count = desc.len;
}
}
@compilelog(total_byte_count);
}
gives a total size of 146898 bytes or ~143 KiB. However, I have not seen quite that much saved in compiled binaries in practice. A test program like this:
const std = @import("std");
pub fn main() !void {
const target = try std.zig.system.resolveTargetQuery(.{});
std.debug.print("{any}\n", .{target});
}
compiled in ReleaseSmall on Windows gives these results:
- Before: 321024 bytes (313.5 KiB)
- After: 219648 bytes (214.5 KiB)
- Delta: 101376 bytes ( 99.0 KiB)
Note: The spirv features were not intended to be changed, but I'm not sure what exact versions of the Headers/Registry were used to generate the current `Target/spirv.zig`. I used commits around the time of the last `Target/spirv.zig` update in May 2021 (98dc8eb); in particular, I used commits 60af2c93c46294a2bc9758889a90d935b6f9325f for SPIRV-Registry and ba29b3f59633836c6bb160b951007c8fc3842dee for SPIRV-Headers. Newer Registry/Headers versions were not used because (1) there are a *lot* more features, more than the current Feature.Set.needed_bit_count and (2) the current update_spirv_features.zig cannot handle some of the new files, e.g. https://raw.githubusercontent.com/KhronosGroup/SPIRV-Registry/main/extensions/EXT/SPV_EXT_relaxed_printf_string_address_space.asciidoc
squeek502
force-pushed
the
cpufeature-separate-desc
branch
from
6e0a0c7
to
e460a1e
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I happened to notice that there were a bunch of these description strings in the data section of compiled resinator binaries. It turns out they were coming from Aro because Aro uses
std.Target. Aro does not actually use the description strings, though, so in both Aro and resinator, those strings are wasted space in the binary.So, this PR allows users of std.Target to not include all the description strings in their program if they don't actually use them.
Counting all the bytes of the description strings after they were separated out using this code:
gives a total size of 146898 bytes or ~143 KiB. However, I have not seen quite that much saved in compiled binaries in practice. A test program like this:
compiled in ReleaseSmall with
-fstripon Windows gives these results:Before: 321024 bytes (313.5 KiB)After: 219648 bytes (214.5 KiB)Delta: 101376 bytes ( 99.0 KiB)Note: The spirv features were not intended to be changed, but I'm not sure what exact versions of the Headers/Registry were used to generate the current
Target/spirv.zig. I used commits around the time of the lastTarget/spirv.zigupdate in May 2021 (98dc8eb); in particular, I used commits 60af2c93c46294a2bc9758889a90d935b6f9325f for SPIRV-Registry and ba29b3f59633836c6bb160b951007c8fc3842dee for SPIRV-Headers. Newer Registry/Headers versions were not used because (1) there are a lot more features, more than the currentFeature.Set.needed_bit_countand (2) the current update_spirv_features.zig cannot handle some of the new files, e.g. https://raw.githubusercontent.com/KhronosGroup/SPIRV-Registry/main/extensions/EXT/SPV_EXT_relaxed_printf_string_address_space.asciidoccc @Snektron