[DebugInfoMetadata] Move main subprogram DIFlag into DISPFlags

[djtodoro]

Summary

Moving subprogram specific flags into DISPFlags makes IR code more readable.
In addition, we provide free space in DIFlags for other 'non-subprogram-specific' debug info flags.

[vsk]

+ Steven, is debug info bitcode backwards compatibility important for the App Store? (I’m not 100% sure, but I think it is, in which case we’d need to 1) detect old DI metadata and 2) auto upgrade it?)

[aprantl]

Yes. Can you please add upgrade functionality into MetadataLoader and add a bitcode upgrade test into test/Bitcode?

You can look at llvm/test/Bitcode/DISubprogram-v4.ll and llvm/test/Bitcode/DISubprogram-v4.ll.bc as an example.

[probinson]

include/llvm-c/DebugInfo.h

53

Better to leave an unused bit in the middle? No need to reassign existing flags to new bits. The next time somebody adds a new flag, it can use the previously unused bit. This will simplify the auto-upgrade as well.

[djtodoro]

Thank you all for comments!

@aprantl

Yes. Can you please add upgrade functionality into MetadataLoader and add a bitcode upgrade test into test/Bitcode?

Sure.

include/llvm-c/DebugInfo.h

53

It is better idea. We can do that way. Thanks!

[djtodoro]

Yes. Can you please add upgrade functionality into MetadataLoader and add a bitcode upgrade test into test/Bitcode?
You can look at llvm/test/Bitcode/DISubprogram-v4.ll and llvm/test/Bitcode/DISubprogram-v4.ll.bc as an example.

@aprantl In addition to this, I guess we want support for this in LLParser as well?

[aprantl]

We don't typically require autoupgrade functionality in the IR parser, as there are no compatibility guarantees for textual LLVM IR. Implementing bitcode support is all that is required. Sometimes it can be convenient to support older formats in LLParser to avoid updating hundreds of tests, but it's definitely not required.

[djtodoro]

Thanks for the comment! I understand.

[djtodoro]

• Add support in Metadataloader for older metadata
• Add bitcode upgrade test

[aprantl]

IIUC this upgrade is sweeping the variant of DISubprograms that have SPflags and do not yet have this patch under the rug? The way the Swift compiler branches work out I can live with that but other downstream users may have a problem with that as it could break LTO builds that include object files versions of clang between 347239 and this patch.

lib/Bitcode/Reader/MetadataLoader.cpp

1421 ↗

(On Diff #190834)

Better to use a local variable instead of a macro here.

1423 ↗

(On Diff #190834)

I think it would be more clear to write this as:

if (!HasSPFlags) {
  const unsigned DIFlagMainSubprogram = 1 << 21;
  bool HasOldMainSubprogramFlag = Flags & DIFlagMainSubprogram;
  // Remove old DIFlagMainSubprogram from DIFlags.
  // Note: This assumes that any future use of bit 21 defaults to it being 0.
  Flags &= ~static_cast<DINode::DIFlags>(DIFlagMainSubprogram);

  DISubprogram::toSPFlags(
                    /*IsLocalToUnit=*/Record[7], /*IsDefinition=*/Record[8],
                    /*IsOptimized=*/Record[14], /*Virtuality=*/Record[11],
                    /*DIFlagMainSubprogram*/HasOldMainSubprogramFlag);
}

[djtodoro]

IIUC this upgrade is sweeping the variant of DISubprograms that have SPflags and do not yet have this patch under the rug? The way the Swift compiler branches work out I can live with that but other downstream users may have a problem with that as it could break LTO builds that include object files versions of clang between 347239 and this patch.

I think this should support both DISubprogram versions, with/out SPFlags.
That's why before this upgrade functionality I put reading of SPFlags (if any) and just update it, in the case of HasOldMainSubprogramFlag, with
if (HasSPFlags) SPFlags |= DISubprogram::SPFlagMainSubprogram;

lib/Bitcode/Reader/MetadataLoader.cpp

1421 ↗

(On Diff #190834)

Yes, I agree.

1423 ↗

(On Diff #190834)

This won't work for the case you describe (when DISubprogram has SPFlags).
This will work only in the case of old DISubprogram, with no SPFlags at all.

[djtodoro]

-Refactor upgrade metadata functionality

[djtodoro]

-Rebase

include/llvm-c/DebugInfo.h

typedef enum {
   LLVMDIFlagRValueReference = 1 << 14,
   LLVMDIFlagReserved = 1 << 15,
   LLVMDIFlagSingleInheritance = 1 << 16,
   LLVMDIFlagMultipleInheritance = 2 << 16,
   LLVMDIFlagVirtualInheritance = 3 << 16,
   LLVMDIFlagIntroducedVirtual = 1 << 18,
   LLVMDIFlagBitField = 1 << 19,
   LLVMDIFlagNoReturn = 1 << 20,
-  LLVMDIFlagMainSubprogram = 1 << 21,
-  LLVMDIFlagTypePassByValue = 1 << 22,
-  LLVMDIFlagTypePassByReference = 1 << 23,
-  LLVMDIFlagEnumClass = 1 << 24,
+  LLVMDIFlagTypePassByValue = 1 << 21,
+  LLVMDIFlagTypePassByReference = 1 << 22,
+  LLVMDIFlagEnumClass = 1 << 23,
   LLVMDIFlagFixedEnum = LLVMDIFlagEnumClass, // Deprecated.
-  LLVMDIFlagThunk = 1 << 25,
-  LLVMDIFlagTrivial = 1 << 26,
-  LLVMDIFlagBigEndian = 1 << 27,
-  LLVMDIFlagLittleEndian = 1 << 28,
+  LLVMDIFlagThunk = 1 << 24,
+  LLVMDIFlagTrivial = 1 << 25,
+  LLVMDIFlagBigEndian = 1 << 26,
+  LLVMDIFlagLittleEndian = 1 << 27,
   LLVMDIFlagIndirectVirtualBase = (1 << 2) | (1 << 5),
   LLVMDIFlagAccessibility = LLVMDIFlagPrivate | LLVMDIFlagProtected |
                             LLVMDIFlagPublic,
   LLVMDIFlagPtrToMemberRep = LLVMDIFlagSingleInheritance |
                              LLVMDIFlagMultipleInheritance |
                              LLVMDIFlagVirtualInheritance
 } LLVMDIFlags;
 

include/llvm/IR/DebugInfoFlags.def

 // 15 was formerly ExternalTypeRef, but this was never used.
 HANDLE_DI_FLAG((1 << 15), Reserved)
 HANDLE_DI_FLAG((1 << 16), SingleInheritance)
 HANDLE_DI_FLAG((2 << 16), MultipleInheritance)
 HANDLE_DI_FLAG((3 << 16), VirtualInheritance)
 HANDLE_DI_FLAG((1 << 18), IntroducedVirtual)
 HANDLE_DI_FLAG((1 << 19), BitField)
 HANDLE_DI_FLAG((1 << 20), NoReturn)
-HANDLE_DI_FLAG((1 << 21), MainSubprogram)
-HANDLE_DI_FLAG((1 << 22), TypePassByValue)
-HANDLE_DI_FLAG((1 << 23), TypePassByReference)
-HANDLE_DI_FLAG((1 << 24), EnumClass)
-HANDLE_DI_FLAG((1 << 25), Thunk)
-HANDLE_DI_FLAG((1 << 26), Trivial)
-HANDLE_DI_FLAG((1 << 27), BigEndian)
-HANDLE_DI_FLAG((1 << 28), LittleEndian)
-HANDLE_DI_FLAG((1 << 29), AllCallsDescribed)
-HANDLE_DI_FLAG((1 << 30), NonTrivial)
+HANDLE_DI_FLAG((1 << 21), TypePassByValue)
+HANDLE_DI_FLAG((1 << 22), TypePassByReference)
+HANDLE_DI_FLAG((1 << 23), EnumClass)
+HANDLE_DI_FLAG((1 << 24), Thunk)
+HANDLE_DI_FLAG((1 << 25), Trivial)
+HANDLE_DI_FLAG((1 << 26), BigEndian)
+HANDLE_DI_FLAG((1 << 27), LittleEndian)
+HANDLE_DI_FLAG((1 << 28), AllCallsDescribed)
+HANDLE_DI_FLAG((1 << 29), NonTrivial)
 
 // To avoid needing a dedicated value for IndirectVirtualBase, we use
 // the bitwise or of Virtual and FwdDecl, which does not otherwise
 // make sense for inheritance.
 HANDLE_DI_FLAG((1 << 2) | (1 << 5), IndirectVirtualBase)
 
 #ifdef DI_FLAG_LARGEST_NEEDED
 // intended to be used with ADT/BitmaskEnum.h
 // NOTE: always must be equal to largest flag, check this when adding new flag
-HANDLE_DI_FLAG((1 << 30), Largest)
+HANDLE_DI_FLAG((1 << 29), Largest)
 #undef DI_FLAG_LARGEST_NEEDED
 #endif
 
 // Subprogram-specific flags kept in DISubprogram.
 
 // Use this as a zero/initialization value.
 // For example: void foo(DISPFlags Flags = SPFlagZero).
 HANDLE_DISP_FLAG(0, Zero)
 // Virtuality is a two-bit enum field in the LSB of the word.
 // Values should match DW_VIRTUALITY_*.
 HANDLE_DISP_FLAG(1u, Virtual)
 HANDLE_DISP_FLAG(2u, PureVirtual)
 HANDLE_DISP_FLAG((1u << 2), LocalToUnit)
 HANDLE_DISP_FLAG((1u << 3), Definition)
 HANDLE_DISP_FLAG((1u << 4), Optimized)
+HANDLE_DISP_FLAG((1u << 5), MainSubprogram)
 
 #ifdef DISP_FLAG_LARGEST_NEEDED
 // Intended to be used with ADT/BitmaskEnum.h.
 // NOTE: Always must be equal to largest flag, check this when adding new flags.
-HANDLE_DISP_FLAG((1 << 4), Largest)
+HANDLE_DISP_FLAG((1 << 5), Largest)
 #undef DISP_FLAG_LARGEST_NEEDED
 #endif
 
 #undef HANDLE_DI_FLAG
 #undef HANDLE_DISP_FLAG

include/llvm/IR/DebugInfoMetadata.h

public:
   unsigned getVirtualIndex() const { return VirtualIndex; }
   int getThisAdjustment() const { return ThisAdjustment; }
   unsigned getScopeLine() const { return ScopeLine; }
   DIFlags getFlags() const { return Flags; }
   DISPFlags getSPFlags() const { return SPFlags; }
   bool isLocalToUnit() const { return getSPFlags() & SPFlagLocalToUnit; }
   bool isDefinition() const { return getSPFlags() & SPFlagDefinition; }
   bool isOptimized() const { return getSPFlags() & SPFlagOptimized; }
+  bool isMainSubprogram() const { return getSPFlags() & SPFlagMainSubprogram; }
 
   bool isArtificial() const { return getFlags() & FlagArtificial; }
   bool isPrivate() const {
     return (getFlags() & FlagAccessibility) == FlagPrivate;
   }
   bool isProtected() const {
     return (getFlags() & FlagAccessibility) == FlagProtected;
   }
   bool isPublic() const {
     return (getFlags() & FlagAccessibility) == FlagPublic;
   }
   bool isExplicit() const { return getFlags() & FlagExplicit; }
   bool isPrototyped() const { return getFlags() & FlagPrototyped; }
   bool areAllCallsDescribed() const {
     return getFlags() & FlagAllCallsDescribed;
   }
-  bool isMainSubprogram() const { return getFlags() & FlagMainSubprogram; }
 
   /// Check if this is reference-qualified.
   ///
   /// Return true if this subprogram is a C++11 reference-qualified non-static
   /// member function (void foo() &).
   bool isLValueReference() const { return getFlags() & FlagLValueReference; }
 
   /// Check if this is rvalue-reference-qualified.

test/DebugInfo/Generic/containing-type-extension.ll

 attributes #0 = { nounwind uwtable }
 
 !llvm.dbg.cu = !{!0}
 !llvm.module.flags = !{!9, !11}
 
 !0 = distinct !DICompileUnit(language: DW_LANG_Rust, producer: "clang version 3.4 (trunk 185475)", isOptimized: false, emissionKind: FullDebug, file: !1, enums: !2, retainedTypes: !2, globals: !15, imports: !2)
 !1 = !DIFile(filename: "CodeGen/dwarf-version.c", directory: "test")
 !2 = !{}
-!4 = distinct !DISubprogram(name: "main", line: 6, isLocal: false, isDefinition: true, virtualIndex: 6, flags: DIFlagPrototyped | DIFlagMainSubprogram, isOptimized: false, unit: !0, scopeLine: 6, file: !1, scope: !5, type: !6, retainedNodes: !2)
+!4 = distinct !DISubprogram(name: "main", line: 6, isLocal: false, isDefinition: true, virtualIndex: 6, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagMainSubprogram, isOptimized: false, unit: !0, scopeLine: 6, file: !1, scope: !5, type: !6, retainedNodes: !2)
 !5 = !DIFile(filename: "CodeGen/dwarf-version.c", directory: "test")
 !6 = !DISubroutineType(types: !7)
 !7 = !{!8}
 !8 = !DIBasicType(tag: DW_TAG_base_type, name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
 !9 = !{i32 2, !"Dwarf Version", i32 4}
 !10 = !DILocation(line: 7, scope: !4)
 !11 = !{i32 1, !"Debug Info Version", i32 3}
 !12 = !DICompositeType(tag: DW_TAG_structure_type, name: "vtable", size: 8, align: 8, elements: !2, identifier: "vtable", vtableHolder: !8)
 !13 = !DIGlobalVariableExpression(var: !14, expr: !DIExpression())
 !14 = !DIGlobalVariable(name: "vtable", linkageName: "vtable", scope: null, file: !1, line: 1, type: !12, isLocal: true, isDefinition: true)
 !15 = !{!13}

test/DebugInfo/Generic/discriminated-union.ll

 !llvm.module.flags = !{!0, !1}
 !llvm.dbg.cu = !{!2}
 
 !0 = !{i32 1, !"PIE Level", i32 2}
 !1 = !{i32 2, !"Debug Info Version", i32 3}
 !2 = distinct !DICompileUnit(language: DW_LANG_Rust, file: !3, producer: "clang LLVM (rustc version 1.24.0-dev)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !4)
 !3 = !DIFile(filename: "e3.rs", directory: "/home/tromey/Rust")
 !4 = !{}
-!5 = distinct !DISubprogram(name: "main", linkageName: "_ZN2e34mainE", scope: !6, file: !3, line: 2, type: !8, isLocal: true, isDefinition: true, scopeLine: 2, flags: DIFlagPrototyped | DIFlagMainSubprogram, isOptimized: false, unit: !2, templateParams: !4, retainedNodes: !4)
+!5 = distinct !DISubprogram(name: "main", linkageName: "_ZN2e34mainE", scope: !6, file: !3, line: 2, type: !8, isLocal: true, isDefinition: true, scopeLine: 2, flags: DIFlagPrototyped, spFlags:  DISPFlagDefinition | DISPFlagMainSubprogram, isOptimized: false, unit: !2, templateParams: !4, retainedNodes: !4)
 !6 = !DINamespace(name: "e3", scope: null)
 !7 = !DIFile(filename: "<unknown>", directory: "")
 !8 = !DISubroutineType(types: !9)
 !9 = !{null}
 !10 = !DILocalVariable(name: "qq", scope: !11, file: !3, line: 3, type: !12, align: 8)
 !11 = distinct !DILexicalBlock(scope: !5, file: !3, line: 3, column: 4)
 !12 = !DICompositeType(tag: DW_TAG_structure_type, name: "F", scope: !6, file: !7, size: 128, align: 64, elements: !13, identifier: "7ce1efff6b82281ab9ceb730566e7e20")
 !13 = !{!14}

test/DebugInfo/Generic/mainsubprogram.ll

 attributes #0 = { nounwind uwtable "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "unsafe-fp-math"="false" "use-soft-float"="false" }
 
 !llvm.dbg.cu = !{!0}
 !llvm.module.flags = !{!9, !11}
 
 !0 = distinct !DICompileUnit(language: DW_LANG_C99, producer: "clang version 3.4 (trunk 185475)", isOptimized: false, emissionKind: FullDebug, file: !1, enums: !2, retainedTypes: !2, globals: !2, imports: !2)
 !1 = !DIFile(filename: "CodeGen/dwarf-version.c", directory: "test")
 !2 = !{}
-!4 = distinct !DISubprogram(name: "main", line: 6, isLocal: false, isDefinition: true, virtualIndex: 6, flags: DIFlagPrototyped | DIFlagMainSubprogram, isOptimized: false, unit: !0, scopeLine: 6, file: !1, scope: !5, type: !6, retainedNodes: !2)
+!4 = distinct !DISubprogram(name: "main", line: 6, isLocal: false, isDefinition: true, virtualIndex: 6, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagMainSubprogram, isOptimized: false, unit: !0, scopeLine: 6, file: !1, scope: !5, type: !6, retainedNodes: !2)
 !5 = !DIFile(filename: "CodeGen/dwarf-version.c", directory: "test")
 !6 = !DISubroutineType(types: !7)
 !7 = !{!8}
 !8 = !DIBasicType(tag: DW_TAG_base_type, name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
 !9 = !{i32 2, !"Dwarf Version", i32 4}
 !10 = !DILocation(line: 7, scope: !4)
 !11 = !{i32 1, !"Debug Info Version", i32 3}

test/DebugInfo/Generic/univariant-discriminated-union.ll

 !llvm.module.flags = !{!0, !1}
 !llvm.dbg.cu = !{!2}
 
 !0 = !{i32 1, !"PIE Level", i32 2}
 !1 = !{i32 2, !"Debug Info Version", i32 3}
 !2 = distinct !DICompileUnit(language: DW_LANG_Rust, file: !3, producer: "clang LLVM (rustc version 1.24.0-dev)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !4)
 !3 = !DIFile(filename: "e3.rs", directory: "/home/tromey/Rust")
 !4 = !{}
-!5 = distinct !DISubprogram(name: "main", linkageName: "_ZN2e34mainE", scope: !6, file: !3, line: 2, type: !8, isLocal: true, isDefinition: true, scopeLine: 2, flags: DIFlagPrototyped | DIFlagMainSubprogram, isOptimized: false, unit: !2, templateParams: !4, retainedNodes: !4)
+!5 = distinct !DISubprogram(name: "main", linkageName: "_ZN2e34mainE", scope: !6, file: !3, line: 2, type: !8, isLocal: true, isDefinition: true, scopeLine: 2, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagMainSubprogram, isOptimized: false, unit: !2, templateParams: !4, retainedNodes: !4)
 !6 = !DINamespace(name: "e3", scope: null)
 !7 = !DIFile(filename: "<unknown>", directory: "")
 !8 = !DISubroutineType(types: !9)
 !9 = !{null}
 !10 = !DILocalVariable(name: "qq", scope: !11, file: !3, line: 3, type: !12, align: 8)
 !11 = distinct !DILexicalBlock(scope: !5, file: !3, line: 3, column: 4)
 !12 = !DICompositeType(tag: DW_TAG_structure_type, name: "F", scope: !6, file: !7, size: 128, align: 64, elements: !13, identifier: "7ce1efff6b82281ab9ceb730566e7e20")
 !13 = !{!14}

test/DebugInfo/X86/containing-type-extension-rust.ll

 !17 = !DIDerivedType(tag: DW_TAG_pointer_type, name: "*mut f64", baseType: !5, size: 64, align: 64)
 !18 = !{!19}
 !19 = !DITemplateTypeParameter(name: "T", type: !5)
 !20 = !DILocalVariable(arg: 1, scope: !11, file: !21, line: 1, type: !17)
 !21 = !DIFile(filename: "t2.rs", directory: "")
 !22 = !DIExpression()
 !23 = !DILocation(line: 1, scope: !11)
 !24 = !DILocation(line: 59, scope: !11)
-!25 = distinct !DISubprogram(name: "main", linkageName: "_ZN2t24mainE", scope: !26, file: !9, line: 9, type: !27, isLocal: true, isDefinition: true, scopeLine: 9, flags: DIFlagPrototyped | DIFlagMainSubprogram, isOptimized: false, unit: !8, templateParams: !4, retainedNodes: !4)
+!25 = distinct !DISubprogram(name: "main", linkageName: "_ZN2t24mainE", scope: !26, file: !9, line: 9, type: !27, isLocal: true, isDefinition: true, scopeLine: 9, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagMainSubprogram, isOptimized: false, unit: !8, templateParams: !4, retainedNodes: !4)
 !26 = !DINamespace(name: "t2", scope: null)
 !27 = !DISubroutineType(types: !28)
 !28 = !{null}
 !29 = !DILocalVariable(name: "tu", scope: !30, file: !9, line: 10, type: !31, align: 8)
 !30 = distinct !DILexicalBlock(scope: !25, file: !9, line: 10, column: 4)
 !31 = !DICompositeType(tag: DW_TAG_structure_type, name: "&T", scope: !26, file: !2, size: 128, align: 64, elements: !32, identifier: "b9f642b757d8ad3984c1e721e3ce6016d14d9322")
 !32 = !{!33, !36}
 !33 = !DIDerivedType(tag: DW_TAG_member, name: "pointer", scope: !31, file: !2, baseType: !34, size: 64, align: 64, flags: DIFlagArtificial)
 !34 = !DIDerivedType(tag: DW_TAG_pointer_type, name: "*const u8", baseType: !35, size: 64, align: 64)
 !35 = !DIBasicType(name: "u8", size: 8, encoding: DW_ATE_unsigned)
 !36 = !DIDerivedType(tag: DW_TAG_member, name: "vtable", scope: !31, file: !2, baseType: !34, size: 64, align: 64, offset: 64, flags: DIFlagArtificial)
 !37 = !DILocation(line: 10, scope: !30)
 !38 = !DILocation(line: 11, scope: !25)