Michael137 wrote:
> Here's the smallest patch that would put explicit alignment on any packed
> structure:
>
> ```
> diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp
> b/clang/lib/CodeGen/CGDebugInfo.cpp
> index a072475ba770..bbb13ddd593b 100644
> --- a/clang/lib/CodeGen/CGDebugInfo.cpp
> +++ b/clang/lib/CodeGen/CGDebugInfo.cpp
> @@ -64,7 +64,7 @@ static uint32_t getTypeAlignIfRequired(const Type *Ty,
> const ASTContext &Ctx) {
> // MaxFieldAlignmentAttr is the attribute added to types
> // declared after #pragma pack(n).
> if (auto *Decl = Ty->getAsRecordDecl())
> - if (Decl->hasAttr<MaxFieldAlignmentAttr>())
> + if (Decl->hasAttr<MaxFieldAlignmentAttr>() ||
> Decl->hasAttr<PackedAttr>())
> return TI.Align;
>
> return 0;
> ```
>
> But I don't think that's the right approach - I think what we should do is
> compute the natural alignment of the structure, then compare that to the
> actual alignment - and if they differ, we should put an explicit alignment on
> the structure. This avoids the risk that other alignment-influencing effects
> might be missed (and avoids the case of putting alignment on a structure
> that, when packed, just has the same alignment anyway - which is a minor
> issue, but nice to get right (eg: packed struct of a single char probably
> shouldn't have an explicit alignment - since it's the same as the implicit
> alignment anyway))
Thanks for the analysis! If we can emit alignment for packed attributes
consistently then we probably can get rid of most of the `InferAlignment` logic
in the `RecordLayoutBuilder` (it seems to me most of that logic was put
introduced there for the purpose of packed structs), which would address the
issue I saw with laying out `[[no_unique_address]]` fields. Trying this now
https://github.com/llvm/llvm-project/pull/93809
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
