llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-codegen Author: Tamir Duberstein (tamird) <details> <summary>Changes</summary> This separates the Clang producer issue identified while reviewing #<!-- -->226717 from BTF’s handling of C++ record elements. The intrinsic contract is unchanged, and the producer correction can be reviewed independently of the backend changes. Assisted-by: OpenAI Codex --- Full diff: https://github.com/llvm/llvm-project/pull/226790.diff 5 Files Affected: - (modified) clang/lib/CodeGen/CGDebugInfo.cpp (+16-5) - (modified) clang/lib/CodeGen/CGDebugInfo.h (+6-2) - (modified) clang/lib/CodeGen/CGExpr.cpp (+13-33) - (modified) clang/lib/CodeGen/CodeGenFunction.h (-3) - (added) clang/test/CodeGenCXX/builtin-preserve-access-index.cpp (+68) ``````````diff diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index b6418753a6a65..42fc602ac1a8c 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -2108,6 +2108,7 @@ void CGDebugInfo::CollectRecordLambdaFields( llvm::DIFile *VUnit = getOrCreateFile(Loc); + FieldIndexCache[*Field] = elements.size(); elements.push_back(createFieldType( GetLambdaCaptureName(Capture), Field->getType(), Loc, Field->getAccess(), FieldOffset, Align, VUnit, RecordTy, CXXDecl)); @@ -2228,6 +2229,7 @@ void CGDebugInfo::CollectRecordNormalField( OffsetInBits, Align, tunit, RecordTy, RD, Annotations); } + FieldIndexCache[field] = elements.size(); elements.push_back(FieldType); } @@ -3087,11 +3089,20 @@ void CGDebugInfo::CollectVTableInfo(const CXXRecordDecl *RD, llvm::DIFile *Unit, EltTys.push_back(VPtrMember); } -llvm::DIType *CGDebugInfo::getOrCreateRecordType(QualType RTy, - SourceLocation Loc) { - assert(CGM.getCodeGenOpts().hasReducedDebugInfo()); - llvm::DIType *T = getOrCreateType(RTy, getOrCreateFile(Loc)); - return T; +std::pair<llvm::DIType *, unsigned> +CGDebugInfo::getOrCreateRecordField(QualType Ty, const FieldDecl *Field) { + const RecordDecl *RD = Field->getParent(); + // Preserve-access intrinsics need complete layouts, including base classes. + if (const auto *CXXRD = dyn_cast<CXXRecordDecl>(RD)) + CXXRD->forallBases([this](const CXXRecordDecl *Base) { + completeClass(Base); + return true; + }); + completeClass(RD); + llvm::DIType *T = getOrCreateStandaloneType(Ty, RD->getLocation()); + auto I = FieldIndexCache.find(Field); + assert(I != FieldIndexCache.end() && "Missing field debug information"); + return {T, I->second}; } llvm::DIType *CGDebugInfo::getOrCreateInterfaceType(QualType D, diff --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h index 8a46e3f0e60bb..fca587517a035 100644 --- a/clang/lib/CodeGen/CGDebugInfo.h +++ b/clang/lib/CodeGen/CGDebugInfo.h @@ -103,6 +103,9 @@ class CGDebugInfo { /// Cache of previously constructed Types. llvm::DenseMap<const void *, llvm::TrackingMDRef> TypeCache; + /// DI element indices used by preserve-access intrinsics. + llvm::DenseMap<const FieldDecl *, unsigned> FieldIndexCache; + /// Cache that maps VLA types to size expressions for that type, /// represented by instantiated Metadata nodes. llvm::SmallDenseMap<QualType, llvm::Metadata *> SizeExprCache; @@ -623,8 +626,9 @@ class CGDebugInfo { /// Emit C++ namespace alias. llvm::DIImportedEntity *EmitNamespaceAlias(const NamespaceAliasDecl &NA); - /// Emit record type's standalone debug info. - llvm::DIType *getOrCreateRecordType(QualType Ty, SourceLocation L); + /// Emit complete record debug info and return the field's element index. + std::pair<llvm::DIType *, unsigned> + getOrCreateRecordField(QualType Ty, const FieldDecl *Field); /// Emit an Objective-C interface type standalone debug info. llvm::DIType *getOrCreateInterfaceType(QualType Ty, SourceLocation Loc); diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index 4a481c01f6a68..fe90ae0af73c2 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -5851,23 +5851,6 @@ LValue CodeGenFunction::EmitLValueForLambdaField(const FieldDecl *Field) { return EmitLValueForLambdaField(Field, CXXABIThisValue); } -/// Get the field index in the debug info. The debug info structure/union -/// will ignore the unnamed bitfields. -unsigned CodeGenFunction::getDebugInfoFIndex(const RecordDecl *Rec, - unsigned FieldIndex) { - unsigned I = 0, Skipped = 0; - - for (auto *F : Rec->getDefinition()->fields()) { - if (I == FieldIndex) - break; - if (F->isUnnamedBitField()) - Skipped++; - I++; - } - - return FieldIndex - Skipped; -} - /// Get the address of a zero-sized field within a record. The resulting /// address doesn't necessarily have the right type. static Address emitAddrOfZeroSizeField(CodeGenFunction &CGF, Address Base, @@ -5930,14 +5913,14 @@ static Address emitAddrOfFieldStorage(CodeGenFunction &CGF, Address base, static Address emitPreserveStructAccess(CodeGenFunction &CGF, LValue base, Address addr, const FieldDecl *field) { const RecordDecl *rec = field->getParent(); - llvm::DIType *DbgInfo = CGF.getDebugInfo()->getOrCreateStandaloneType( - base.getType(), rec->getLocation()); + auto [DbgInfo, DIIndex] = + CGF.getDebugInfo()->getOrCreateRecordField(base.getType(), field); unsigned idx = CGF.CGM.getTypes().getCGRecordLayout(rec).getLLVMFieldNo(field); - return CGF.Builder.CreatePreserveStructAccessIndex( - addr, idx, CGF.getDebugInfoFIndex(rec, field->getFieldIndex()), DbgInfo); + return CGF.Builder.CreatePreserveStructAccessIndex(addr, idx, DIIndex, + DbgInfo); } static bool hasAnyVptr(const QualType Type, const ASTContext &Context) { @@ -5989,11 +5972,10 @@ LValue CodeGenFunction::EmitLValueForField(LValue base, const FieldDecl *field, Addr = Builder.CreateStructGEP(Addr, Idx, field->getName()); } } else { - llvm::DIType *DbgInfo = getDebugInfo()->getOrCreateRecordType( - getContext().getCanonicalTagType(rec), rec->getLocation()); - Addr = Builder.CreatePreserveStructAccessIndex( - Addr, Idx, getDebugInfoFIndex(rec, field->getFieldIndex()), - DbgInfo); + auto [DbgInfo, DIIndex] = getDebugInfo()->getOrCreateRecordField( + getContext().getCanonicalTagType(rec), field); + Addr = Builder.CreatePreserveStructAccessIndex(Addr, Idx, DIIndex, + DbgInfo); } } const unsigned SS = @@ -6069,13 +6051,11 @@ LValue CodeGenFunction::EmitLValueForField(LValue base, const FieldDecl *field, if (IsInPreservedAIRegion || (getDebugInfo() && rec->hasAttr<BPFPreserveAccessIndexAttr>())) { // Remember the original union field index - llvm::DIType *DbgInfo = getDebugInfo()->getOrCreateStandaloneType(base.getType(), - rec->getLocation()); - addr = - Address(Builder.CreatePreserveUnionAccessIndex( - addr.emitRawPointer(*this), - getDebugInfoFIndex(rec, field->getFieldIndex()), DbgInfo), - addr.getElementType(), addr.getAlignment()); + auto [DbgInfo, DIIndex] = + getDebugInfo()->getOrCreateRecordField(base.getType(), field); + addr = Address(Builder.CreatePreserveUnionAccessIndex( + addr.emitRawPointer(*this), DIIndex, DbgInfo), + addr.getElementType(), addr.getAlignment()); } if (FieldType->isReferenceType()) diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index 653e883012229..fe0704e517a95 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -3491,9 +3491,6 @@ class CodeGenFunction : public CodeGenTypeCache { /// Converts Location to a DebugLoc, if debug information is enabled. llvm::DebugLoc SourceLocToDebugLoc(SourceLocation Location); - /// Get the record field index as represented in debug info. - unsigned getDebugInfoFIndex(const RecordDecl *Rec, unsigned FieldIndex); - //===--------------------------------------------------------------------===// // Declaration Emission //===--------------------------------------------------------------------===// diff --git a/clang/test/CodeGenCXX/builtin-preserve-access-index.cpp b/clang/test/CodeGenCXX/builtin-preserve-access-index.cpp new file mode 100644 index 0000000000000..2a8fbb1562f8a --- /dev/null +++ b/clang/test/CodeGenCXX/builtin-preserve-access-index.cpp @@ -0,0 +1,68 @@ +// RUN: %clang_cc1 -triple bpfel -emit-llvm -debug-info-kind=limited -disable-llvm-passes %s -o - | FileCheck %s +// RUN: %clang_cc1 -triple x86_64 -emit-llvm -debug-info-kind=limited -disable-llvm-passes %s -o - | FileCheck %s +// RUN: %clang_cc1 -triple bpfel -emit-llvm -debug-info-kind=constructor -disable-llvm-passes %s -o - | FileCheck %s + +struct Base { + Base(); + int base; +}; +struct Record : Base { + static int first; + int field; + static int second; + unsigned bits : 3; + void method(); +}; +union Union { + int first; + static int member; + long second; +}; +struct Dynamic { + virtual void method(); + int field; +}; + +int *field(Record *p) { + return __builtin_preserve_access_index(&p->field); +} +// CHECK: call ptr @llvm.preserve.struct.access.index.p0.p0({{.*}}, i32 1, i32 2), {{.*}}!llvm.preserve.access.index ![[RECORD:[0-9]+]] + +unsigned bits(Record *p) { + return __builtin_preserve_access_index(p->bits); +} +// CHECK: call ptr @llvm.preserve.struct.access.index.p0.p0({{.*}}, i32 2, i32 4), {{.*}}!llvm.preserve.access.index ![[RECORD]] + +long *member(Union *p) { + return __builtin_preserve_access_index(&p->second); +} +// CHECK: call ptr @llvm.preserve.union.access.index.p0.p0({{.*}}, i32 2), {{.*}}!llvm.preserve.access.index ![[UNION:[0-9]+]] + +int *dynamic(Dynamic *p) { + return __builtin_preserve_access_index(&p->field); +} +// CHECK: call ptr @llvm.preserve.struct.access.index.p0.p0({{.*}}, i32 1, i32 1), {{.*}}!llvm.preserve.access.index ![[DYNAMIC:[0-9]+]] + +int lambda(int a, int b) { + return [a, b] { return __builtin_preserve_access_index(b); }(); +} +// CHECK: call ptr @llvm.preserve.struct.access.index.p0.p0({{.*}}, i32 1, i32 1), {{.*}}!llvm.preserve.access.index ![[LAMBDA:[0-9]+]] + +// The intrinsic indices refer to the complete DI element lists, including +// bases, static members, and the vtable pointer before the accessed fields. +// CHECK-DAG: ![[RECORD]] = distinct !DICompositeType({{.*}}name: "Record", {{.*}}elements: ![[RECORD_ELEMENTS:[0-9]+]] +// CHECK-DAG: ![[RECORD_ELEMENTS]] = !{!{{[0-9]+}}, !{{[0-9]+}}, ![[FIELD:[0-9]+]], !{{[0-9]+}}, ![[BITS:[0-9]+]], !{{[0-9]+}}} +// CHECK-DAG: ![[FIELD]] = !DIDerivedType(tag: DW_TAG_member, name: "field" +// CHECK-DAG: ![[BITS]] = !DIDerivedType(tag: DW_TAG_member, name: "bits" +// CHECK-DAG: ![[UNION]] = distinct !DICompositeType({{.*}}name: "Union", {{.*}}elements: ![[UNION_ELEMENTS:[0-9]+]] +// CHECK-DAG: ![[UNION_ELEMENTS]] = !{!{{[0-9]+}}, !{{[0-9]+}}, ![[SECOND:[0-9]+]]} +// CHECK-DAG: ![[SECOND]] = !DIDerivedType(tag: DW_TAG_member, name: "second" +// CHECK-DAG: ![[DYNAMIC]] = distinct !DICompositeType({{.*}}name: "Dynamic", {{.*}}elements: ![[DYNAMIC_ELEMENTS:[0-9]+]] +// CHECK-DAG: ![[DYNAMIC_ELEMENTS]] = !{!{{[0-9]+}}, ![[DYNAMIC_FIELD:[0-9]+]], !{{[0-9]+}}} +// CHECK-DAG: ![[DYNAMIC_FIELD]] = !DIDerivedType(tag: DW_TAG_member, name: "field" +// CHECK-DAG: ![[LAMBDA]] = distinct !DICompositeType({{.*}}elements: ![[LAMBDA_ELEMENTS:[0-9]+]] +// CHECK-DAG: ![[LAMBDA_ELEMENTS]] = !{!{{[0-9]+}}, ![[CAPTURE:[0-9]+]]} +// CHECK-DAG: ![[CAPTURE]] = !DIDerivedType(tag: DW_TAG_member, name: "b" +// CHECK-DAG: !DICompositeType({{.*}}name: "Base", {{.*}}elements: ![[BASE_ELEMENTS:[0-9]+]] +// CHECK-DAG: ![[BASE_ELEMENTS]] = !{![[BASE_FIELD:[0-9]+]], !{{[0-9]+}}} +// CHECK-DAG: ![[BASE_FIELD]] = !DIDerivedType(tag: DW_TAG_member, name: "base" `````````` </details> https://github.com/llvm/llvm-project/pull/226790 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
