Add RunTimeLang to Class and Enumeration DIBuilder functions #72011

augusto2112 · 2023-11-11T01:46:20Z

RunTimeLang is already supported by DICompositeType, and already used by structs and unions. Add a new parameter in the class and enumeration create methods, so they can use the RunTimeLang attribute as well.

llvmbot · 2023-11-11T02:51:08Z

@llvm/pr-subscribers-clang-codegen
@llvm/pr-subscribers-debuginfo

@llvm/pr-subscribers-llvm-ir

Author: Augusto Noronha (augusto2112)

Changes

RunTimeLang is already supported by DICompositeType, and already used by structs and unions. Add a new parameter in the class and enumeration create methods, so they can use the RunTimeLang attribute as well.

Full diff: https://github.com/llvm/llvm-project/pull/72011.diff

4 Files Affected:

(modified) clang/lib/CodeGen/CGDebugInfo.cpp (+3-3)
(modified) llvm/include/llvm/IR/DIBuilder.h (+6-4)
(modified) llvm/lib/IR/DIBuilder.cpp (+11-8)
(modified) llvm/lib/IR/DebugInfo.cpp (+6-7)

diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index 84a166d3ac3659c..fc8088da998ddbd 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3385,9 +3385,9 @@ llvm::DIType *CGDebugInfo::CreateTypeDefinition(const EnumType *Ty) {
   unsigned Line = getLineNumber(ED->getLocation());
   llvm::DIScope *EnumContext = getDeclContextDescriptor(ED);
   llvm::DIType *ClassTy = getOrCreateType(ED->getIntegerType(), DefUnit);
-  return DBuilder.createEnumerationType(EnumContext, ED->getName(), DefUnit,
-                                        Line, Size, Align, EltArray, ClassTy,
-                                        Identifier, ED->isScoped());
+  return DBuilder.createEnumerationType(
+      EnumContext, ED->getName(), DefUnit, Line, Size, Align, EltArray, ClassTy,
+      /*RunTimeLang=*/0, Identifier, ED->isScoped());
 }
 
 llvm::DIMacro *CGDebugInfo::CreateMacro(llvm::DIMacroFile *Parent,
diff --git a/llvm/include/llvm/IR/DIBuilder.h b/llvm/include/llvm/IR/DIBuilder.h
index ecd6dd7b0a4f822..9c8646c8a544b74 100644
--- a/llvm/include/llvm/IR/DIBuilder.h
+++ b/llvm/include/llvm/IR/DIBuilder.h
@@ -424,6 +424,7 @@ namespace llvm {
     /// \param OffsetInBits Member offset.
     /// \param Flags        Flags to encode member attribute, e.g. private
     /// \param Elements     class members.
+    /// \param RunTimeLang  Optional parameter, Objective-C runtime version.
     /// \param VTableHolder Debug info of the base class that contains vtable
     ///                     for this type. This is used in
     ///                     DW_AT_containing_type. See DWARF documentation
@@ -434,8 +435,8 @@ namespace llvm {
         DIScope *Scope, StringRef Name, DIFile *File, unsigned LineNumber,
         uint64_t SizeInBits, uint32_t AlignInBits, uint64_t OffsetInBits,
         DINode::DIFlags Flags, DIType *DerivedFrom, DINodeArray Elements,
-        DIType *VTableHolder = nullptr, MDNode *TemplateParms = nullptr,
-        StringRef UniqueIdentifier = "");
+        unsigned RunTimeLang = 0, DIType *VTableHolder = nullptr,
+        MDNode *TemplateParms = nullptr, StringRef UniqueIdentifier = "");
 
     /// Create debugging information entry for a struct.
     /// \param Scope        Scope in which this struct is defined.
@@ -578,13 +579,14 @@ namespace llvm {
     /// \param AlignInBits    Member alignment.
     /// \param Elements       Enumeration elements.
     /// \param UnderlyingType Underlying type of a C++11/ObjC fixed enum.
+    /// \param RunTimeLang  Optional parameter, Objective-C runtime version.
     /// \param UniqueIdentifier A unique identifier for the enum.
     /// \param IsScoped Boolean flag indicate if this is C++11/ObjC 'enum class'.
     DICompositeType *createEnumerationType(
         DIScope *Scope, StringRef Name, DIFile *File, unsigned LineNumber,
         uint64_t SizeInBits, uint32_t AlignInBits, DINodeArray Elements,
-        DIType *UnderlyingType, StringRef UniqueIdentifier = "", bool IsScoped = false);
-
+        DIType *UnderlyingType, unsigned RunTimeLang = 0,
+        StringRef UniqueIdentifier = "", bool IsScoped = false);
     /// Create debugging information entry for a set.
     /// \param Scope          Scope in which this set is defined.
     /// \param Name           Set name.
diff --git a/llvm/lib/IR/DIBuilder.cpp b/llvm/lib/IR/DIBuilder.cpp
index 1ce8c17f8a880f6..b466018e9a7d998 100644
--- a/llvm/lib/IR/DIBuilder.cpp
+++ b/llvm/lib/IR/DIBuilder.cpp
@@ -477,14 +477,15 @@ DICompositeType *DIBuilder::createClassType(
     DIScope *Context, StringRef Name, DIFile *File, unsigned LineNumber,
     uint64_t SizeInBits, uint32_t AlignInBits, uint64_t OffsetInBits,
     DINode::DIFlags Flags, DIType *DerivedFrom, DINodeArray Elements,
-    DIType *VTableHolder, MDNode *TemplateParams, StringRef UniqueIdentifier) {
+    unsigned RunTimeLang, DIType *VTableHolder, MDNode *TemplateParams,
+    StringRef UniqueIdentifier) {
   assert((!Context || isa<DIScope>(Context)) &&
          "createClassType should be called with a valid Context");
 
   auto *R = DICompositeType::get(
       VMContext, dwarf::DW_TAG_structure_type, Name, File, LineNumber,
       getNonCompileUnitScope(Context), DerivedFrom, SizeInBits, AlignInBits,
-      OffsetInBits, Flags, Elements, 0, VTableHolder,
+      OffsetInBits, Flags, Elements, RunTimeLang, VTableHolder,
       cast_or_null<MDTuple>(TemplateParams), UniqueIdentifier);
   trackIfUnresolved(R);
   return R;
@@ -535,15 +536,17 @@ DISubroutineType *DIBuilder::createSubroutineType(DITypeRefArray ParameterTypes,
   return DISubroutineType::get(VMContext, Flags, CC, ParameterTypes);
 }
 
-DICompositeType *DIBuilder::createEnumerationType(
-    DIScope *Scope, StringRef Name, DIFile *File, unsigned LineNumber,
-    uint64_t SizeInBits, uint32_t AlignInBits, DINodeArray Elements,
-    DIType *UnderlyingType, StringRef UniqueIdentifier, bool IsScoped) {
+DICompositeType *
+DIBuilder::createEnumerationType(DIScope *Scope, StringRef Name, DIFile *File,
+                                 unsigned LineNumber, uint64_t SizeInBits,
+                                 uint32_t AlignInBits, DINodeArray Elements,
+                                 DIType *UnderlyingType, unsigned RunTimeLang,
+                                 StringRef UniqueIdentifier, bool IsScoped) {
   auto *CTy = DICompositeType::get(
       VMContext, dwarf::DW_TAG_enumeration_type, Name, File, LineNumber,
       getNonCompileUnitScope(Scope), UnderlyingType, SizeInBits, AlignInBits, 0,
-      IsScoped ? DINode::FlagEnumClass : DINode::FlagZero, Elements, 0, nullptr,
-      nullptr, UniqueIdentifier);
+      IsScoped ? DINode::FlagEnumClass : DINode::FlagZero, Elements,
+      RunTimeLang, nullptr, nullptr, UniqueIdentifier);
   AllEnumTypes.emplace_back(CTy);
   trackIfUnresolved(CTy);
   return CTy;
diff --git a/llvm/lib/IR/DebugInfo.cpp b/llvm/lib/IR/DebugInfo.cpp
index 390a27c4bc0c4dd..501eec3dd99c1e5 100644
--- a/llvm/lib/IR/DebugInfo.cpp
+++ b/llvm/lib/IR/DebugInfo.cpp
@@ -1467,13 +1467,12 @@ LLVMMetadataRef LLVMDIBuilderCreateClassType(LLVMDIBuilderRef Builder,
   auto Elts = unwrap(Builder)->getOrCreateArray({unwrap(Elements),
                                                  NumElements});
   return wrap(unwrap(Builder)->createClassType(
-                  unwrapDI<DIScope>(Scope), {Name, NameLen},
-                  unwrapDI<DIFile>(File), LineNumber,
-                  SizeInBits, AlignInBits, OffsetInBits,
-                  map_from_llvmDIFlags(Flags), unwrapDI<DIType>(DerivedFrom),
-                  Elts, unwrapDI<DIType>(VTableHolder),
-                  unwrapDI<MDNode>(TemplateParamsNode),
-                  {UniqueIdentifier, UniqueIdentifierLen}));
+      unwrapDI<DIScope>(Scope), {Name, NameLen}, unwrapDI<DIFile>(File),
+      LineNumber, SizeInBits, AlignInBits, OffsetInBits,
+      map_from_llvmDIFlags(Flags), unwrapDI<DIType>(DerivedFrom), Elts,
+      /*RunTimeLang=*/0, unwrapDI<DIType>(VTableHolder),
+      unwrapDI<MDNode>(TemplateParamsNode),
+      {UniqueIdentifier, UniqueIdentifierLen}));
 }
 
 LLVMMetadataRef

github-actions · 2023-11-11T02:59:31Z

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff c05ab7b8507a33d5e069ebbe10a5c607995c9c11 8cebb68070de258e6f581e83c3b0541087f53570 -- clang/lib/CodeGen/CGDebugInfo.cpp llvm/include/llvm/IR/DIBuilder.h llvm/lib/IR/DIBuilder.cpp llvm/lib/IR/DebugInfo.cpp

View the diff from clang-format here.

diff --git a/llvm/lib/IR/DebugInfo.cpp b/llvm/lib/IR/DebugInfo.cpp
index 501eec3dd9..1d2f938090 100644
--- a/llvm/lib/IR/DebugInfo.cpp
+++ b/llvm/lib/IR/DebugInfo.cpp
@@ -1315,17 +1315,15 @@ LLVMDIBuilderCreateUnspecifiedType(LLVMDIBuilderRef Builder, const char *Name,
   return wrap(unwrap(Builder)->createUnspecifiedType({Name, NameLen}));
 }
 
-LLVMMetadataRef
-LLVMDIBuilderCreateStaticMemberType(
+LLVMMetadataRef LLVMDIBuilderCreateStaticMemberType(
     LLVMDIBuilderRef Builder, LLVMMetadataRef Scope, const char *Name,
     size_t NameLen, LLVMMetadataRef File, unsigned LineNumber,
     LLVMMetadataRef Type, LLVMDIFlags Flags, LLVMValueRef ConstantVal,
     uint32_t AlignInBits) {
   return wrap(unwrap(Builder)->createStaticMemberType(
-                  unwrapDI<DIScope>(Scope), {Name, NameLen},
-                  unwrapDI<DIFile>(File), LineNumber, unwrapDI<DIType>(Type),
-                  map_from_llvmDIFlags(Flags), unwrap<Constant>(ConstantVal),
-                  AlignInBits));
+      unwrapDI<DIScope>(Scope), {Name, NameLen}, unwrapDI<DIFile>(File),
+      LineNumber, unwrapDI<DIType>(Type), map_from_llvmDIFlags(Flags),
+      unwrap<Constant>(ConstantVal), AlignInBits));
 }
 
 LLVMMetadataRef

augusto2112 · 2023-11-14T23:32:47Z

ping :)

felipepiovezan

Sorry for the delay!

Let's add the [DebugInfo][CGDebugInfo] tags the commit title so that the git log is more searchable.

I think we should have at least one test in this patch as well

felipepiovezan · 2023-11-14T23:54:12Z

llvm/include/llvm/IR/DIBuilder.h

@@ -424,6 +424,7 @@ namespace llvm {
    /// \param OffsetInBits Member offset.
    /// \param Flags        Flags to encode member attribute, e.g. private
    /// \param Elements     class members.
+    /// \param RunTimeLang  Optional parameter, Objective-C runtime version.


let's use an optional type to denote an optional value

I think your entire patch would be a lot simpler if this were the last parameter of the function, as you wouldn't need to change as many callsites and have inline comments explaining the parameter. Did you have a motivation in mind for placing the argument here?

I think your entire patch would be a lot simpler if this were the last parameter of the function, as you wouldn't need to change as many callsites and have inline comments explaining the parameter. Did you have a motivation in mind for placing the argument here?

I'm following the pattern on the ordering from the existing functions, for example:

DICompositeType *createStructType( DIScope *Scope, StringRef Name, DIFile *File, unsigned LineNumber, uint64_t SizeInBits, uint32_t AlignInBits, DINode::DIFlags Flags, DIType *DerivedFrom, DINodeArray Elements, unsigned RunTimeLang = 0, DIType *VTableHolder = nullptr, StringRef UniqueIdentifier = ""); DICompositeType *createUnionType(DIScope *Scope, StringRef Name, DIFile *File, unsigned LineNumber, uint64_t SizeInBits, uint32_t AlignInBits, DINode::DIFlags Flags, DINodeArray Elements, unsigned RunTimeLang = 0, StringRef UniqueIdentifier = "");

Ah, that's fair then

augusto2112 · 2023-11-15T01:06:15Z

@felipepiovezan I don't this patch as it is has any changes that are testable. I haven't added any code that will emit a RuntimeLang, only added the option to do that so I can add those callers downstream. In retrospect I should've marked it as NFC.

felipepiovezan · 2023-11-15T01:12:25Z

@felipepiovezan I don't this patch as it is has any changes that are testable. I haven't added any code that will emit a RuntimeLang, only added the option to do that so I can add those callers downstream. In retrospect I should've marked it as NFC.

I see. I think this is yet another argument for flipping the argument order: if the new thing were the last argument, it would have been even more evident this is NFC :)

RunTimeLang is already supported by DICompositeType, and already used by structs and unions. Add a new parameter in the class and enumeration create methods, so they can use the RunTimeLang attribute as well.

In LLVM-18, RunTimeLang is added to enums and classes: llvm/llvm-project#72011 Rather than passing the parameter through, we default it to 0 since it will not reliably do anything until Rust requires LLVM 18, and we always pass 0 to the APIs for similar debug structures.

…ype() Added in LLVM in llvm/llvm-project#72011.

augusto2112 requested review from adrian-prantl and Michael137 November 11, 2023 01:46

llvmbot added clang Clang issues not falling into any other category clang:codegen debuginfo llvm:ir labels Nov 11, 2023

augusto2112 force-pushed the runtime-lang-class branch from 82e6fef to 051bd74 Compare November 13, 2023 17:48

augusto2112 requested a review from felipepiovezan November 13, 2023 21:46

felipepiovezan reviewed Nov 15, 2023

View reviewed changes

[DebugInfo][CGDebugInfo] Add RunTimeLang to more DIBuilder functions

8cebb68

RunTimeLang is already supported by DICompositeType, and already used by structs and unions. Add a new parameter in the class and enumeration create methods, so they can use the RunTimeLang attribute as well.

augusto2112 force-pushed the runtime-lang-class branch from 051bd74 to 8cebb68 Compare November 15, 2023 20:45

augusto2112 merged commit d0ae239 into llvm:main Nov 15, 2023
1 of 3 checks passed

aeubanks mentioned this pull request Nov 15, 2023

llvm-wrapper: Pass newly added param to DIBuilder::createStaticMemberType() rust-lang/rust#117948

Merged

maurer mentioned this pull request Nov 15, 2023

llvm: Default new RunTimeLang to 0 for enums rust-lang/rust#117952

Closed

aeubanks added a commit to aeubanks/rust that referenced this pull request Nov 15, 2023

llvm-wrapper: Pass newly added param to DIBuilder::createEnumerationT…

7cde2ce

…ype() Added in LLVM in llvm/llvm-project#72011.

aeubanks mentioned this pull request Nov 15, 2023

llvm-wrapper: Pass newly added param to DIBuilder::createEnumerationType() rust-lang/rust#117954

Merged

bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 16, 2023

Auto merge of rust-lang#117954 - aeubanks:dibuilder2, r=durin42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add RunTimeLang to Class and Enumeration DIBuilder functions #72011

Add RunTimeLang to Class and Enumeration DIBuilder functions #72011

augusto2112 commented Nov 11, 2023

llvmbot commented Nov 11, 2023 •

edited

Loading

github-actions bot commented Nov 11, 2023 •

edited

Loading

augusto2112 commented Nov 14, 2023

felipepiovezan left a comment •

edited

Loading

felipepiovezan Nov 14, 2023

felipepiovezan Nov 15, 2023

augusto2112 Nov 15, 2023

felipepiovezan Nov 15, 2023

augusto2112 commented Nov 15, 2023

felipepiovezan commented Nov 15, 2023

Add RunTimeLang to Class and Enumeration DIBuilder functions #72011

Add RunTimeLang to Class and Enumeration DIBuilder functions #72011

Conversation

augusto2112 commented Nov 11, 2023

llvmbot commented Nov 11, 2023 • edited Loading

github-actions bot commented Nov 11, 2023 • edited Loading

augusto2112 commented Nov 14, 2023

felipepiovezan left a comment • edited Loading

Choose a reason for hiding this comment

felipepiovezan Nov 14, 2023

Choose a reason for hiding this comment

felipepiovezan Nov 15, 2023

Choose a reason for hiding this comment

augusto2112 Nov 15, 2023

Choose a reason for hiding this comment

felipepiovezan Nov 15, 2023

Choose a reason for hiding this comment

augusto2112 commented Nov 15, 2023

felipepiovezan commented Nov 15, 2023

llvmbot commented Nov 11, 2023 •

edited

Loading

github-actions bot commented Nov 11, 2023 •

edited

Loading

felipepiovezan left a comment •

edited

Loading