[all-commits] [llvm/llvm-project] 4497ec: [clang][CGRecordLayout] Remove dependency on isZer...

[Michael Buch via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 4497ec293a6e745be817dc88027169bd5e4f7246
      https://github.com/llvm/llvm-project/commit/4497ec293a6e745be817dc88027169bd5e4f7246
  Author: Michael Buch <michaelbuch12 at gmail.com>
  Date:   2024-07-16 (Tue, 16 Jul 2024)

  Changed paths:
    M clang/lib/CodeGen/ABIInfoImpl.cpp
    M clang/lib/CodeGen/ABIInfoImpl.h
    M clang/lib/CodeGen/CGClass.cpp
    M clang/lib/CodeGen/CGExpr.cpp
    M clang/lib/CodeGen/CGExprConstant.cpp
    M clang/lib/CodeGen/CGOpenMPRuntime.cpp
    M clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
    M clang/lib/CodeGen/CodeGenTBAA.cpp
    M clang/test/CodeGen/2009-06-14-anonymous-union-init.c
    M clang/test/CodeGen/X86/x86_64-vaarg.c
    M clang/test/CodeGen/paren-list-agg-init.cpp
    M clang/test/CodeGen/voidptr-vaarg.c
    M clang/test/CodeGenCXX/2011-12-19-init-list-ctor.cpp
    M clang/test/CodeGenCXX/bitfield-access-empty.cpp
    M clang/test/CodeGenCXX/class-layout.cpp
    M clang/test/CodeGenCXX/compound-literals.cpp
    M clang/test/CodeGenCXX/exceptions.cpp
    M clang/test/CodeGenCXX/lambda-deterministic-captures.cpp
    M clang/test/CodeGenCXX/partial-destruction.cpp
    M clang/test/CodeGenCXX/pod-member-memcpys.cpp
    M clang/test/CodeGenCXX/pr18962.cpp
    M clang/test/CodeGenCXX/references.cpp
    M clang/test/CodeGenCXX/temporaries.cpp
    A clang/test/CodeGenCXX/zero-init-empty-virtual.cpp
    M clang/test/CodeGenObjCXX/lambda-to-block.mm
    M clang/test/OpenMP/irbuilder_for_iterator.cpp
    M clang/test/OpenMP/irbuilder_for_rangefor.cpp
    M clang/test/OpenMP/task_member_call_codegen.cpp

  Log Message:
  -----------
  [clang][CGRecordLayout] Remove dependency on isZeroSize (#96422)

This is a follow-up from the conversation starting at
https://github.com/llvm/llvm-project/pull/93809#issuecomment-2173729801

The root problem that motivated the change are external AST sources that
compute `ASTRecordLayout`s themselves instead of letting Clang compute
them from the AST. One such example is LLDB using DWARF to get the
definitive offsets and sizes of C++ structures. Such layouts should be
considered correct (modulo buggy DWARF), but various assertions and
lowering logic around the `CGRecordLayoutBuilder` relies on the AST
having `[[no_unique_address]]` attached to them. This is a
layout-altering attribute which is not encoded in DWARF. This causes us
LLDB to trip over the various LLVM<->Clang layout consistency checks.
There has been precedent for avoiding such layout-altering attributes
from affecting lowering with externally-provided layouts (e.g., packed
structs).

This patch proposes to replace the `isZeroSize` checks in
`CGRecordLayoutBuilder` (which roughly means "empty field with
[[no_unique_address]]") with checks for
`CodeGen::isEmptyField`/`CodeGen::isEmptyRecord`.

**Details**
The main strategy here was to change the `isZeroSize` check in
`CGRecordLowering::accumulateFields` and
`CGRecordLowering::accumulateBases` to use the `isEmptyXXX` APIs
instead, preventing empty fields from being added to the `Members` and
`Bases` structures. The rest of the changes fall out from here, to
prevent lookups into these structures (for field numbers or base
indices) from failing.

Added `isEmptyRecordForLayout` and `isEmptyFieldForLayout` (open to
better naming suggestions). The main difference to the existing
`isEmptyRecord`/`isEmptyField` APIs, is that the `isEmptyXXXForLayout`
counterparts don't have special treatment for `unnamed bitfields`/arrays
and also treat fields of empty types as if they had
`[[no_unique_address]]` (i.e., just like the `AsIfNoUniqueAddr` in
`isEmptyField` does).



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications