[PATCH] D62167: CodeView - add static data members to global variable debug info.

2019-05-20 Thread Amy Huang via Phabricator via cfe-commits
akhuang created this revision.
akhuang added a reviewer: rnk.
Herald added subscribers: llvm-commits, cfe-commits, aprantl.
Herald added projects: clang, LLVM.

Add static data members to IR debug info's list of global variables
so that they are emitted as S_CONSTANT records.

Related to https://bugs.llvm.org/show_bug.cgi?id=41615.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D62167

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  llvm/test/DebugInfo/COFF/global-constants.ll

Index: llvm/test/DebugInfo/COFF/global-constants.ll
===
--- llvm/test/DebugInfo/COFF/global-constants.ll
+++ llvm/test/DebugInfo/COFF/global-constants.ll
@@ -3,18 +3,25 @@
 
 ; C++ source to regenerate:
 ; const int Test1 = 1;
+; struct Foo { static const int Test2 = 2; };
 ; int main() {
-;   return Test1;
+;   return Test1 + Foo::Test2;
 ; }
 ; $ clang t.cpp -S -emit-llvm -g -gcodeview -o t.ll
 
-; ASM-LABEL:  .long 241  # Symbol subsection for globals
+; ASM-LABEL:  .long 241 # Symbol subsection for globals
 
-; ASM:.short {{.*-.*}}   # Record length
-; ASM:.short 4359# Record kind: S_CONSTANT
-; ASM-NEXT:   .long 4099 # Type
-; ASM-NEXT:   .byte 0x01, 0x00   # Value
-; ASM-NEXT:   .asciz "Test1" # Name
+; ASM:.short {{.*-.*}}  # Record length
+; ASM:.short 4359   # Record kind: S_CONSTANT
+; ASM-NEXT:   .long 4099# Type
+; ASM-NEXT:   .byte 0x01, 0x00  # Value
+; ASM-NEXT:   .asciz "Test1"# Name
+
+; ASM:.short {{.*-.*}}  # Record length
+; ASM:.short 4359   # Record kind: S_CONSTANT
+; ASM:.long 4099# Type
+; ASM:.byte 0x02, 0x00  # Value
+; ASM:.asciz "Test2"# Name
 
 ; OBJ:CodeViewDebugInfo [
 ; OBJ:  Section: .debug$S
@@ -27,6 +34,12 @@
 ; OBJ-NEXT: Value: 1
 ; OBJ-NEXT: Name: Test1
 ; OBJ-NEXT:   }
+; OBJ:ConstantSym {
+; OBJ-NEXT: Kind: S_CONSTANT (0x1107)
+; OBJ-NEXT: Type: const int (0x1003)
+; OBJ-NEXT: Value: 2
+; OBJ-NEXT: Name: Test2
+; OBJ-NEXT:   }
 
 ; ModuleID = 't.cpp'
 source_filename = "t.cpp"
@@ -34,31 +47,39 @@
 target triple = "x86_64-pc-windows-msvc"
 
 ; Function Attrs: noinline norecurse nounwind optnone
-define dso_local i32 @main() #0 !dbg !13 {
+define dso_local i32 @main() #0 !dbg !19 {
 entry:
   %retval = alloca i32, align 4
   store i32 0, i32* %retval, align 4
-  ret i32 1, !dbg !16
+  ret i32 3, !dbg !22
 }
 
+attributes #0 = { noinline norecurse nounwind optnone "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+
 !llvm.dbg.cu = !{!0}
-!llvm.module.flags = !{!9, !10, !11}
-!llvm.ident = !{!12}
+!llvm.module.flags = !{!15, !16, !17}
+!llvm.ident = !{!18}
 
-!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 9.0.0 (https://github.com/llvm/llvm-project.git 4a1902b6739e3087a03c0ac7ab85b640764e9335)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, globals: !3, nameTableKind: None)
-!1 = !DIFile(filename: "", directory: "C:\5Csrc\5Ctest", checksumkind: CSK_MD5, checksum: "0d5ef00bdd80bdb409a3deac9938f20d")
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 9.0.0 (https://github.com/llvm/llvm-project.git 2b66a49044196d8b90d95d7d3b5246ccbe3abc05)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !3, globals: !10, nameTableKind: None)
+!1 = !DIFile(filename: "", directory: "C:\5Csrc\5Ctest", checksumkind: CSK_MD5, checksum: "77cff5e1c7b260440ed03b23c18809c3")
 !2 = !{}
 !3 = !{!4}
-!4 = !DIGlobalVariableExpression(var: !5, expr: !DIExpression(DW_OP_constu, 1, DW_OP_stack_value))
-!5 = distinct !DIGlobalVariable(name: "Test1", scope: !0, file: !6, line: 1, type: !7, isLocal: true, isDefinition: true)
-!6 = !DIFile(filename: "t.cpp", directory: "C:\5Csrc\5Ctest", checksumkind: CSK_MD5, checksum: "0d5ef00bdd80bdb409a3deac9938f20d")
-!7 = !DIDerivedType(tag: DW_TAG_const_type, baseType: !8)
-!8 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
-!9 = !{i32 2, !"CodeView", i32 1}
-!10 = !{i32 2, !"Debug Info Version", i32 3}
-!11 = !{i32 1, !"wchar_size", i32 2}
-!12 = !{!"clang version 9.0.0 (https://github.com/llvm/llvm-project.git 4a1902b6739e3087a03c0ac7ab85b640764e9335)"}
-!13 = distinct !DISubprogram(name: "main", scope: !6, file: !6, line: 3, type: !14, scopeLine: 3, flags: DIFlagPrototype

[PATCH] D62167: CodeView - add static data members to global variable debug info.

2019-05-21 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

This needs a clang-side test to show that clang generates the expected IR for 
static data members.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62167/new/

https://reviews.llvm.org/D62167



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D62167: CodeView - add static data members to global variable debug info.

2019-05-21 Thread Amy Huang via Phabricator via cfe-commits
akhuang marked an inline comment as done.
akhuang added a subscriber: dblaikie.
akhuang added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4385
+// Use the global scope for static members.
+DContext = getContextDescriptor(
+   cast(CGM.getContext().getTranslationUnitDecl()), TheCU);

@dblaikie I'm using the global scope here because if the class type is used as 
the scope it runs into the [[ 
https://github.com/llvm/llvm-project/blob/a2ee80b084e5c0b20364ed4379384706f5e059b1/llvm/lib/IR/DIBuilder.cpp#L630
 | assert message ]] `Context of a global variable should not be a type with 
identifier`. Is there a reason for the assert? 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62167/new/

https://reviews.llvm.org/D62167



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D62167: CodeView - add static data members to global variable debug info.

2019-05-22 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4385
+// Use the global scope for static members.
+DContext = getContextDescriptor(
+   cast(CGM.getContext().getTranslationUnitDecl()), TheCU);

akhuang wrote:
> @dblaikie I'm using the global scope here because if the class type is used 
> as the scope it runs into the [[ 
> https://github.com/llvm/llvm-project/blob/a2ee80b084e5c0b20364ed4379384706f5e059b1/llvm/lib/IR/DIBuilder.cpp#L630
>  | assert message ]] `Context of a global variable should not be a type with 
> identifier`. Is there a reason for the assert? 
I think it's generally how LLVM handles definitions for the most part - putting 
them at the global scope.

Though I'm confused by this patch - it has a source change in clang, but a test 
in LLVM. Generally there should be testing to cover where the source change is, 
I think?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62167/new/

https://reviews.llvm.org/D62167



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D62167: CodeView - add static data members to global variable debug info.

2019-05-23 Thread Amy Huang via Phabricator via cfe-commits
akhuang updated this revision to Diff 201004.
akhuang added a comment.

Add llvm IR test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62167/new/

https://reviews.llvm.org/D62167

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/debug-info-static-member.cpp
  llvm/test/DebugInfo/COFF/global-constants.ll

Index: llvm/test/DebugInfo/COFF/global-constants.ll
===
--- llvm/test/DebugInfo/COFF/global-constants.ll
+++ llvm/test/DebugInfo/COFF/global-constants.ll
@@ -3,18 +3,25 @@
 
 ; C++ source to regenerate:
 ; const int Test1 = 1;
+; struct Foo { static const int Test2 = 2; };
 ; int main() {
-;   return Test1;
+;   return Test1 + Foo::Test2;
 ; }
 ; $ clang t.cpp -S -emit-llvm -g -gcodeview -o t.ll
 
-; ASM-LABEL:  .long 241  # Symbol subsection for globals
+; ASM-LABEL:  .long 241 # Symbol subsection for globals
 
-; ASM:.short {{.*-.*}}   # Record length
-; ASM:.short 4359# Record kind: S_CONSTANT
-; ASM-NEXT:   .long 4099 # Type
-; ASM-NEXT:   .byte 0x01, 0x00   # Value
-; ASM-NEXT:   .asciz "Test1" # Name
+; ASM:.short {{.*-.*}}  # Record length
+; ASM:.short 4359   # Record kind: S_CONSTANT
+; ASM-NEXT:   .long 4099# Type
+; ASM-NEXT:   .byte 0x01, 0x00  # Value
+; ASM-NEXT:   .asciz "Test1"# Name
+
+; ASM:.short {{.*-.*}}  # Record length
+; ASM:.short 4359   # Record kind: S_CONSTANT
+; ASM:.long 4099# Type
+; ASM:.byte 0x02, 0x00  # Value
+; ASM:.asciz "Test2"# Name
 
 ; OBJ:CodeViewDebugInfo [
 ; OBJ:  Section: .debug$S
@@ -27,6 +34,12 @@
 ; OBJ-NEXT: Value: 1
 ; OBJ-NEXT: Name: Test1
 ; OBJ-NEXT:   }
+; OBJ:ConstantSym {
+; OBJ-NEXT: Kind: S_CONSTANT (0x1107)
+; OBJ-NEXT: Type: const int (0x1003)
+; OBJ-NEXT: Value: 2
+; OBJ-NEXT: Name: Test2
+; OBJ-NEXT:   }
 
 ; ModuleID = 't.cpp'
 source_filename = "t.cpp"
@@ -34,31 +47,39 @@
 target triple = "x86_64-pc-windows-msvc"
 
 ; Function Attrs: noinline norecurse nounwind optnone
-define dso_local i32 @main() #0 !dbg !13 {
+define dso_local i32 @main() #0 !dbg !19 {
 entry:
   %retval = alloca i32, align 4
   store i32 0, i32* %retval, align 4
-  ret i32 1, !dbg !16
+  ret i32 3, !dbg !22
 }
 
+attributes #0 = { noinline norecurse nounwind optnone "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+
 !llvm.dbg.cu = !{!0}
-!llvm.module.flags = !{!9, !10, !11}
-!llvm.ident = !{!12}
+!llvm.module.flags = !{!15, !16, !17}
+!llvm.ident = !{!18}
 
-!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 9.0.0 (https://github.com/llvm/llvm-project.git 4a1902b6739e3087a03c0ac7ab85b640764e9335)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, globals: !3, nameTableKind: None)
-!1 = !DIFile(filename: "", directory: "C:\5Csrc\5Ctest", checksumkind: CSK_MD5, checksum: "0d5ef00bdd80bdb409a3deac9938f20d")
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 9.0.0 (https://github.com/llvm/llvm-project.git 2b66a49044196d8b90d95d7d3b5246ccbe3abc05)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !3, globals: !10, nameTableKind: None)
+!1 = !DIFile(filename: "", directory: "C:\5Csrc\5Ctest", checksumkind: CSK_MD5, checksum: "77cff5e1c7b260440ed03b23c18809c3")
 !2 = !{}
 !3 = !{!4}
-!4 = !DIGlobalVariableExpression(var: !5, expr: !DIExpression(DW_OP_constu, 1, DW_OP_stack_value))
-!5 = distinct !DIGlobalVariable(name: "Test1", scope: !0, file: !6, line: 1, type: !7, isLocal: true, isDefinition: true)
-!6 = !DIFile(filename: "t.cpp", directory: "C:\5Csrc\5Ctest", checksumkind: CSK_MD5, checksum: "0d5ef00bdd80bdb409a3deac9938f20d")
-!7 = !DIDerivedType(tag: DW_TAG_const_type, baseType: !8)
-!8 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
-!9 = !{i32 2, !"CodeView", i32 1}
-!10 = !{i32 2, !"Debug Info Version", i32 3}
-!11 = !{i32 1, !"wchar_size", i32 2}
-!12 = !{!"clang version 9.0.0 (https://github.com/llvm/llvm-project.git 4a1902b6739e3087a03c0ac7ab85b640764e9335)"}
-!13 = distinct !DISubprogram(name: "main", scope: !6, file: !6, line: 3, type: !14, scopeLine: 3, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
-!14 = !DISubroutineType(types: !15)
-!15 = !{!8}
-!16 = !DILoca

[PATCH] D62167: CodeView - add static data members to global variable debug info.

2019-05-23 Thread Amy Huang via Phabricator via cfe-commits
akhuang marked an inline comment as done.
akhuang added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4385
+// Use the global scope for static members.
+DContext = getContextDescriptor(
+   cast(CGM.getContext().getTranslationUnitDecl()), TheCU);

dblaikie wrote:
> akhuang wrote:
> > @dblaikie I'm using the global scope here because if the class type is used 
> > as the scope it runs into the [[ 
> > https://github.com/llvm/llvm-project/blob/a2ee80b084e5c0b20364ed4379384706f5e059b1/llvm/lib/IR/DIBuilder.cpp#L630
> >  | assert message ]] `Context of a global variable should not be a type 
> > with identifier`. Is there a reason for the assert? 
> I think it's generally how LLVM handles definitions for the most part - 
> putting them at the global scope.
> 
> Though I'm confused by this patch - it has a source change in clang, but a 
> test in LLVM. Generally there should be testing to cover where the source 
> change is, I think?
yep, there should be a test for this - added now. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62167/new/

https://reviews.llvm.org/D62167



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D62167: CodeView - add static data members to global variable debug info.

2019-05-24 Thread Amy Huang via Phabricator via cfe-commits
akhuang marked an inline comment as done.
akhuang added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4385
+// Use the global scope for static members.
+DContext = getContextDescriptor(
+   cast(CGM.getContext().getTranslationUnitDecl()), TheCU);

akhuang wrote:
> dblaikie wrote:
> > akhuang wrote:
> > > @dblaikie I'm using the global scope here because if the class type is 
> > > used as the scope it runs into the [[ 
> > > https://github.com/llvm/llvm-project/blob/a2ee80b084e5c0b20364ed4379384706f5e059b1/llvm/lib/IR/DIBuilder.cpp#L630
> > >  | assert message ]] `Context of a global variable should not be a type 
> > > with identifier`. Is there a reason for the assert? 
> > I think it's generally how LLVM handles definitions for the most part - 
> > putting them at the global scope.
> > 
> > Though I'm confused by this patch - it has a source change in clang, but a 
> > test in LLVM. Generally there should be testing to cover where the source 
> > change is, I think?
> yep, there should be a test for this - added now. 
So for the global scope, it seems like [[ 
https://github.com/llvm-mirror/clang/blob/900624ef605b60c613342dac071228539a402ce9/lib/CodeGen/CGDebugInfo.cpp#L3274
 | elsewhere ]], when creating the debug info for a static data member global 
variable, the scope is set to be the global scope. But it would be helpful for 
codeview debug info if the scope remained as the class type, partially so we 
can use the class type information in the variable name that we emit.

Does it seem reasonable to remove the assert for global variable scope so that 
the scope can be the class here?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62167/new/

https://reviews.llvm.org/D62167



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D62167: CodeView - add static data members to global variable debug info.

2019-05-28 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added subscribers: manmanren, probinson.
rnk added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4385
+// Use the global scope for static members.
+DContext = getContextDescriptor(
+   cast(CGM.getContext().getTranslationUnitDecl()), TheCU);

akhuang wrote:
> akhuang wrote:
> > dblaikie wrote:
> > > akhuang wrote:
> > > > @dblaikie I'm using the global scope here because if the class type is 
> > > > used as the scope it runs into the [[ 
> > > > https://github.com/llvm/llvm-project/blob/a2ee80b084e5c0b20364ed4379384706f5e059b1/llvm/lib/IR/DIBuilder.cpp#L630
> > > >  | assert message ]] `Context of a global variable should not be a type 
> > > > with identifier`. Is there a reason for the assert? 
> > > I think it's generally how LLVM handles definitions for the most part - 
> > > putting them at the global scope.
> > > 
> > > Though I'm confused by this patch - it has a source change in clang, but 
> > > a test in LLVM. Generally there should be testing to cover where the 
> > > source change is, I think?
> > yep, there should be a test for this - added now. 
> So for the global scope, it seems like [[ 
> https://github.com/llvm-mirror/clang/blob/900624ef605b60c613342dac071228539a402ce9/lib/CodeGen/CGDebugInfo.cpp#L3274
>  | elsewhere ]], when creating the debug info for a static data member global 
> variable, the scope is set to be the global scope. But it would be helpful 
> for codeview debug info if the scope remained as the class type, partially so 
> we can use the class type information in the variable name that we emit.
> 
> Does it seem reasonable to remove the assert for global variable scope so 
> that the scope can be the class here?
I think the assertion in question is this one here in DIBuilder:
https://github.com/llvm/llvm-project/blob/master/llvm/lib/IR/DIBuilder.cpp#L634

It looks like @manmanren added this in 2014:
https://github.com/llvm/llvm-project/commit/bfd2b829d912ecd89f73ae32d4c683856dd677a3

@dblaikie do you know if this check is still necessary? It seems like the DWARF 
clang generages today corresponds to C++ written this way:
```
struct Foo { static const int Test2; };
const int Foo::Test2 = 4;
```
When oftentimes the source looks more like this:
```
struct Foo { static const int Test2 = 4; };
```
I saw a conversation between you and @probinson fixing clang irgen to avoid 
this assert sometime back, so I figured you might be a good point of reference.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62167/new/

https://reviews.llvm.org/D62167



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D62167: CodeView - add static data members to global variable debug info.

2019-05-28 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.

Is this the change we were discussing the other week semi-related to preserved 
enums? It sounded to me like the conclusion was somewhat to go ahead without 
adding all these things to the constant list and see how it goes? Is this case 
different from the enum case in some way?




Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4385
+// Use the global scope for static members.
+DContext = getContextDescriptor(
+   cast(CGM.getContext().getTranslationUnitDecl()), TheCU);

rnk wrote:
> akhuang wrote:
> > akhuang wrote:
> > > dblaikie wrote:
> > > > akhuang wrote:
> > > > > @dblaikie I'm using the global scope here because if the class type 
> > > > > is used as the scope it runs into the [[ 
> > > > > https://github.com/llvm/llvm-project/blob/a2ee80b084e5c0b20364ed4379384706f5e059b1/llvm/lib/IR/DIBuilder.cpp#L630
> > > > >  | assert message ]] `Context of a global variable should not be a 
> > > > > type with identifier`. Is there a reason for the assert? 
> > > > I think it's generally how LLVM handles definitions for the most part - 
> > > > putting them at the global scope.
> > > > 
> > > > Though I'm confused by this patch - it has a source change in clang, 
> > > > but a test in LLVM. Generally there should be testing to cover where 
> > > > the source change is, I think?
> > > yep, there should be a test for this - added now. 
> > So for the global scope, it seems like [[ 
> > https://github.com/llvm-mirror/clang/blob/900624ef605b60c613342dac071228539a402ce9/lib/CodeGen/CGDebugInfo.cpp#L3274
> >  | elsewhere ]], when creating the debug info for a static data member 
> > global variable, the scope is set to be the global scope. But it would be 
> > helpful for codeview debug info if the scope remained as the class type, 
> > partially so we can use the class type information in the variable name 
> > that we emit.
> > 
> > Does it seem reasonable to remove the assert for global variable scope so 
> > that the scope can be the class here?
> I think the assertion in question is this one here in DIBuilder:
> https://github.com/llvm/llvm-project/blob/master/llvm/lib/IR/DIBuilder.cpp#L634
> 
> It looks like @manmanren added this in 2014:
> https://github.com/llvm/llvm-project/commit/bfd2b829d912ecd89f73ae32d4c683856dd677a3
> 
> @dblaikie do you know if this check is still necessary? It seems like the 
> DWARF clang generages today corresponds to C++ written this way:
> ```
> struct Foo { static const int Test2; };
> const int Foo::Test2 = 4;
> ```
> When oftentimes the source looks more like this:
> ```
> struct Foo { static const int Test2 = 4; };
> ```
> I saw a conversation between you and @probinson fixing clang irgen to avoid 
> this assert sometime back, so I figured you might be a good point of 
> reference.
My expectation is that this characterization is probably a pragmatic one of 
"it's what GCC does/we're not sure if GDB could cope with it otherwise" (we 
don't always adhere strictly to what GCC does - for instance GCC always puts 
function definitions at the CU scope, and if the function was defined in a 
namespace it puts a declaration in the namespace and references that from the 
global scoped definition - Clang goes the other way and puts the definition in 
the namespace scope (even if the function was declared there but defined from 
outside the namespace))

If that's the case (worth testing) then arguably the frontend should emit the 
more accurate metadata and the DWARF backend could fudge this - or like some of 
the other stuff we do we could emit different metadata in the frontend 
depending on the DWARF v CodeView split.


On further experimentation, I'm somewhat confused what this change is about. In 
ToT, this code:

  struct foo {
static const int bar = 3;
  };
  int f() {
return foo::bar;
  }

Becomes DWARF that has a DICompositeType with elements pointing to a 
DIDerivedType:DW_TAG_member with the extraData of i32 3.

And that becomes this DWARF:

  TAG_structure_type
AT_name "foo"
TAG_member
  AT_name "foo"
  declaration true
  const_value 3

which seems accurate to me - this isn't a variable definition, it's a 
declaration with a value.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62167/new/

https://reviews.llvm.org/D62167



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D62167: CodeView - add static data members to global variable debug info.

2019-05-29 Thread Amy Huang via Phabricator via cfe-commits
akhuang updated this revision to Diff 202027.
akhuang added a comment.
Herald added a subscriber: hiraditya.

Append class name to static data member debug info name.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62167/new/

https://reviews.llvm.org/D62167

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/debug-info-static-member.cpp
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
  llvm/test/DebugInfo/COFF/global-constants.ll

Index: llvm/test/DebugInfo/COFF/global-constants.ll
===
--- llvm/test/DebugInfo/COFF/global-constants.ll
+++ llvm/test/DebugInfo/COFF/global-constants.ll
@@ -3,18 +3,25 @@
 
 ; C++ source to regenerate:
 ; const int Test1 = 1;
+; struct Foo { static const int Test2 = 2; };
 ; int main() {
-;   return Test1;
+;   return Test1 + Foo::Test2;
 ; }
 ; $ clang t.cpp -S -emit-llvm -g -gcodeview -o t.ll
 
-; ASM-LABEL:  .long 241  # Symbol subsection for globals
+; ASM-LABEL:  .long 241 # Symbol subsection for globals
 
-; ASM:.short {{.*-.*}}   # Record length
-; ASM:.short 4359# Record kind: S_CONSTANT
-; ASM-NEXT:   .long 4099 # Type
-; ASM-NEXT:   .byte 0x01, 0x00   # Value
-; ASM-NEXT:   .asciz "Test1" # Name
+; ASM:.short {{.*-.*}}  # Record length
+; ASM:.short 4359   # Record kind: S_CONSTANT
+; ASM-NEXT:   .long 4099# Type
+; ASM-NEXT:   .byte 0x01, 0x00  # Value
+; ASM-NEXT:   .asciz "Test1"# Name
+
+; ASM:.short {{.*-.*}}  # Record length
+; ASM:.short 4359   # Record kind: S_CONSTANT
+; ASM:.long 4099# Type
+; ASM:.byte 0x02, 0x00  # Value
+; ASM:.asciz "Foo::Test2"   # Name
 
 ; OBJ:CodeViewDebugInfo [
 ; OBJ:  Section: .debug$S
@@ -27,6 +34,12 @@
 ; OBJ-NEXT: Value: 1
 ; OBJ-NEXT: Name: Test1
 ; OBJ-NEXT:   }
+; OBJ:ConstantSym {
+; OBJ-NEXT: Kind: S_CONSTANT (0x1107)
+; OBJ-NEXT: Type: const int (0x1003)
+; OBJ-NEXT: Value: 2
+; OBJ-NEXT: Name: Foo::Test2
+; OBJ-NEXT:   }
 
 ; ModuleID = 't.cpp'
 source_filename = "t.cpp"
@@ -34,31 +47,39 @@
 target triple = "x86_64-pc-windows-msvc"
 
 ; Function Attrs: noinline norecurse nounwind optnone
-define dso_local i32 @main() #0 !dbg !13 {
+define dso_local i32 @main() #0 !dbg !19 {
 entry:
   %retval = alloca i32, align 4
   store i32 0, i32* %retval, align 4
-  ret i32 1, !dbg !16
+  ret i32 3, !dbg !22
 }
 
+attributes #0 = { noinline norecurse nounwind optnone "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+
 !llvm.dbg.cu = !{!0}
-!llvm.module.flags = !{!9, !10, !11}
-!llvm.ident = !{!12}
+!llvm.module.flags = !{!15, !16, !17}
+!llvm.ident = !{!18}
 
-!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 9.0.0 (https://github.com/llvm/llvm-project.git 4a1902b6739e3087a03c0ac7ab85b640764e9335)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, globals: !3, nameTableKind: None)
-!1 = !DIFile(filename: "", directory: "C:\5Csrc\5Ctest", checksumkind: CSK_MD5, checksum: "0d5ef00bdd80bdb409a3deac9938f20d")
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 9.0.0 (https://github.com/llvm/llvm-project.git 2b66a49044196d8b90d95d7d3b5246ccbe3abc05)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !3, globals: !10, nameTableKind: None)
+!1 = !DIFile(filename: "", directory: "C:\5Csrc\5Ctest", checksumkind: CSK_MD5, checksum: "77cff5e1c7b260440ed03b23c18809c3")
 !2 = !{}
 !3 = !{!4}
-!4 = !DIGlobalVariableExpression(var: !5, expr: !DIExpression(DW_OP_constu, 1, DW_OP_stack_value))
-!5 = distinct !DIGlobalVariable(name: "Test1", scope: !0, file: !6, line: 1, type: !7, isLocal: true, isDefinition: true)
-!6 = !DIFile(filename: "t.cpp", directory: "C:\5Csrc\5Ctest", checksumkind: CSK_MD5, checksum: "0d5ef00bdd80bdb409a3deac9938f20d")
-!7 = !DIDerivedType(tag: DW_TAG_const_type, baseType: !8)
-!8 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
-!9 = !{i32 2, !"CodeView", i32 1}
-!10 = !{i32 2, !"Debug Info Version", i32 3}
-!11 = !{i32 1, !"wchar_size", i32 2}
-!12 = !{!"clang version 9.0.0 (https://github.com/llvm/llvm-project.git 4a1902b6739e3087a03c0ac7ab85b640764e9335)"}
-!13 = distinct !DISubprogram(name: "main", scope: !6, file: !6, line: 3, type: !14, scopeLine: 3, flags: DIFlagProt

[PATCH] D62167: CodeView - add static data members to global variable debug info.

2019-05-29 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62167/new/

https://reviews.llvm.org/D62167



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D62167: CodeView - add static data members to global variable debug info.

2019-05-29 Thread Amy Huang via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL362038: CodeView - add static data members to global 
variable debug info. (authored by akhuang, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D62167?vs=202027&id=202063#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62167/new/

https://reviews.llvm.org/D62167

Files:
  cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
  cfe/trunk/test/CodeGenCXX/debug-info-static-member.cpp
  llvm/trunk/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
  llvm/trunk/test/DebugInfo/COFF/global-constants.ll

Index: llvm/trunk/test/DebugInfo/COFF/global-constants.ll
===
--- llvm/trunk/test/DebugInfo/COFF/global-constants.ll
+++ llvm/trunk/test/DebugInfo/COFF/global-constants.ll
@@ -3,18 +3,25 @@
 
 ; C++ source to regenerate:
 ; const int Test1 = 1;
+; struct Foo { static const int Test2 = 2; };
 ; int main() {
-;   return Test1;
+;   return Test1 + Foo::Test2;
 ; }
 ; $ clang t.cpp -S -emit-llvm -g -gcodeview -o t.ll
 
-; ASM-LABEL:  .long 241  # Symbol subsection for globals
+; ASM-LABEL:  .long 241 # Symbol subsection for globals
 
-; ASM:.short {{.*-.*}}   # Record length
-; ASM:.short 4359# Record kind: S_CONSTANT
-; ASM-NEXT:   .long 4099 # Type
-; ASM-NEXT:   .byte 0x01, 0x00   # Value
-; ASM-NEXT:   .asciz "Test1" # Name
+; ASM:.short {{.*-.*}}  # Record length
+; ASM:.short 4359   # Record kind: S_CONSTANT
+; ASM-NEXT:   .long 4099# Type
+; ASM-NEXT:   .byte 0x01, 0x00  # Value
+; ASM-NEXT:   .asciz "Test1"# Name
+
+; ASM:.short {{.*-.*}}  # Record length
+; ASM:.short 4359   # Record kind: S_CONSTANT
+; ASM:.long 4099# Type
+; ASM:.byte 0x02, 0x00  # Value
+; ASM:.asciz "Foo::Test2"   # Name
 
 ; OBJ:CodeViewDebugInfo [
 ; OBJ:  Section: .debug$S
@@ -27,6 +34,12 @@
 ; OBJ-NEXT: Value: 1
 ; OBJ-NEXT: Name: Test1
 ; OBJ-NEXT:   }
+; OBJ:ConstantSym {
+; OBJ-NEXT: Kind: S_CONSTANT (0x1107)
+; OBJ-NEXT: Type: const int (0x1003)
+; OBJ-NEXT: Value: 2
+; OBJ-NEXT: Name: Foo::Test2
+; OBJ-NEXT:   }
 
 ; ModuleID = 't.cpp'
 source_filename = "t.cpp"
@@ -34,31 +47,39 @@
 target triple = "x86_64-pc-windows-msvc"
 
 ; Function Attrs: noinline norecurse nounwind optnone
-define dso_local i32 @main() #0 !dbg !13 {
+define dso_local i32 @main() #0 !dbg !19 {
 entry:
   %retval = alloca i32, align 4
   store i32 0, i32* %retval, align 4
-  ret i32 1, !dbg !16
+  ret i32 3, !dbg !22
 }
 
+attributes #0 = { noinline norecurse nounwind optnone "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+
 !llvm.dbg.cu = !{!0}
-!llvm.module.flags = !{!9, !10, !11}
-!llvm.ident = !{!12}
+!llvm.module.flags = !{!15, !16, !17}
+!llvm.ident = !{!18}
 
-!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 9.0.0 (https://github.com/llvm/llvm-project.git 4a1902b6739e3087a03c0ac7ab85b640764e9335)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, globals: !3, nameTableKind: None)
-!1 = !DIFile(filename: "", directory: "C:\5Csrc\5Ctest", checksumkind: CSK_MD5, checksum: "0d5ef00bdd80bdb409a3deac9938f20d")
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 9.0.0 (https://github.com/llvm/llvm-project.git 2b66a49044196d8b90d95d7d3b5246ccbe3abc05)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !3, globals: !10, nameTableKind: None)
+!1 = !DIFile(filename: "", directory: "C:\5Csrc\5Ctest", checksumkind: CSK_MD5, checksum: "77cff5e1c7b260440ed03b23c18809c3")
 !2 = !{}
 !3 = !{!4}
-!4 = !DIGlobalVariableExpression(var: !5, expr: !DIExpression(DW_OP_constu, 1, DW_OP_stack_value))
-!5 = distinct !DIGlobalVariable(name: "Test1", scope: !0, file: !6, line: 1, type: !7, isLocal: true, isDefinition: true)
-!6 = !DIFile(filename: "t.cpp", directory: "C:\5Csrc\5Ctest", checksumkind: CSK_MD5, checksum: "0d5ef00bdd80bdb409a3deac9938f20d")
-!7 = !DIDerivedType(tag: DW_TAG_const_type, baseType: !8)
-!8 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
-!9 = !{i32 2, !"CodeView", i32 1}
-!10 = !{i32 2, !"Debug Info Version", i32 3}
-!11 = !{i32 1, !"wchar_size", i32 2}
-!12 = !{!"clang version 9.0.0 (https://github.com/llvm/llvm-project.git 4a1902b6739e3087