[RFC, V1] try to exclude packed attr for types that contain aligned types#2769
Closed
bertschingert wants to merge 1 commit intorust-lang:mainfrom
bertschingert:packed-aligned-take1
Closed
[RFC, V1] try to exclude packed attr for types that contain aligned types#2769bertschingert wants to merge 1 commit intorust-lang:mainfrom bertschingert:packed-aligned-take1
bertschingert wants to merge 1 commit intorust-lang:mainfrom
bertschingert:packed-aligned-take1
Conversation
This patch handles some (but not all) cases of types with a `packed` attribute that contain a type with an `align(N)` attribute. This uses information available in the Clang IR about types' layout to determine if a given type is likely to have an alignment attribute placed on it during the code generation phase. This is just a heuristic; the decision to actually place an alignment attribute depends on information that is not known until code generation, so the logic here may result in both false positives and false negatives. Using the real information from codegen on whether an alignment attribute was placed on a child type is not possible in general, because the order in which types are generated is not guaranteed. In some cases code may be generated for parent types before code is generated for child types contained in that parent. Then, it will be impossible to make an accurate decision regarding whether to remove the `packed` attribute from the parent type. The impact of a false negative is that an outer type may have a `packed` attr even when an inner type as an `align` attr. Such a type would not compile under rustc, but it already would not compile so this has no harmful impact. The impact of a false positive is that an outer type may have its `packed` attr stripped needlessly, because no inner types actually have an `align` attr. Because we only remove the `packed` attr when we can confirm that there will be no change to the type's layout, this also should have no harmful impact.
45 tasks
|
☔ The latest upstream changes (presumably 7e90434) made this pull request unmergeable. Please resolve the merge conflicts. |
This file contains hidden or 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
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.
NOTE: I know this can use some cleanups and more documentation / comments, etc. but for now I'm just uploading it as an RFC to see if you like the general approach. If you do, then I'll clean it up, but if not I won't bother.
This patch handles some (but not all) cases of types with a
packedattribute that contain a type with analign(N)attribute.This uses information available in the Clang IR about types' layout to determine if a given type is likely to have an alignment attribute placed on it during the code generation phase. This is just a heuristic; the decision to actually place an alignment attribute depends on information that is not known until code generation, so the logic here may result in both false positives and false negatives.
Using the real information from codegen on whether an alignment attribute was placed on a child type is not possible in general, because the order in which types are generated is not guaranteed. In some cases code may be generated for parent types before code is generated for child types contained in that parent. Then, it will be impossible to make an accurate decision regarding whether to remove the
packedattribute from the parent type.The impact of a false negative is that an outer type may have a
packedattr even when an inner type as analignattr. Such a type would not compile under rustc, but it already would not compile so this has no harmful impact.The impact of a false positive is that an outer type may have its
packedattr stripped needlessly, because no inner types actually have analignattr. Because we only remove thepackedattr when we can confirm that there will be no change to the type's layout, this also should have no harmful impact.