subject:"\[PATCH\] D42736\: \[DebugInfo\] Improvements to representation of enumeration types \(PR36168\)"

[PATCH] D42736: [DebugInfo] Improvements to representation of enumeration types (PR36168)

2018-02-07 Thread Momchil Velikov via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rC324490: [DebugInfo] Improvements to representation of 
enumeration types (PR36168) (authored by chill, committed by ).

Repository:
  rL LLVM

https://reviews.llvm.org/D42736

Files:
  lib/CodeGen/CGDebugInfo.cpp
  test/CodeGen/debug-info-enum.cpp
  test/CodeGenCXX/debug-info-enum-class.cpp
  test/CodeGenCXX/debug-info-enum.cpp
  test/Modules/ModuleDebugInfo.cpp

Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -2492,22 +2492,24 @@
   // Create elements for each enumerator.
   SmallVector Enumerators;
   ED = ED->getDefinition();
+  bool IsSigned = ED->getIntegerType()->isSignedIntegerType();
   for (const auto *Enum : ED->enumerators()) {
-Enumerators.push_back(DBuilder.createEnumerator(
-Enum->getName(), Enum->getInitVal().getSExtValue()));
+const auto &InitVal = Enum->getInitVal();
+auto Value = IsSigned ? InitVal.getSExtValue() : InitVal.getZExtValue();
+Enumerators.push_back(
+DBuilder.createEnumerator(Enum->getName(), Value, !IsSigned));
   }
 
   // Return a CompositeType for the enum itself.
   llvm::DINodeArray EltArray = DBuilder.getOrCreateArray(Enumerators);
 
   llvm::DIFile *DefUnit = getOrCreateFile(ED->getLocation());
   unsigned Line = getLineNumber(ED->getLocation());
   llvm::DIScope *EnumContext = getDeclContextDescriptor(ED);
-  llvm::DIType *ClassTy =
-  ED->isFixed() ? getOrCreateType(ED->getIntegerType(), DefUnit) : nullptr;
+  llvm::DIType *ClassTy = getOrCreateType(ED->getIntegerType(), DefUnit);
   return DBuilder.createEnumerationType(EnumContext, ED->getName(), DefUnit,
 Line, Size, Align, EltArray, ClassTy,
-FullName);
+FullName, ED->isFixed());
 }
 
 llvm::DIMacro *CGDebugInfo::CreateMacro(llvm::DIMacroFile *Parent,
Index: test/Modules/ModuleDebugInfo.cpp
===
--- test/Modules/ModuleDebugInfo.cpp
+++ test/Modules/ModuleDebugInfo.cpp
@@ -48,7 +48,7 @@
 // CHECK: !DICompositeType(tag: DW_TAG_enumeration_type,
 // CHECK-NOT:  name:
 // CHECK-SAME: )
-// CHECK: !DIEnumerator(name: "e5", value: 5)
+// CHECK: !DIEnumerator(name: "e5", value: 5, isUnsigned: true)
 
 // CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "B",
 // no mangled name here yet.
Index: test/CodeGen/debug-info-enum.cpp
===
--- test/CodeGen/debug-info-enum.cpp
+++ test/CodeGen/debug-info-enum.cpp
@@ -0,0 +1,100 @@
+// Test enumeration representation in debuig info metadata:
+// * test value representation for each possible underlying integer type
+// * test the integer type is as expected
+// * test the DW_AT_enum_class attribute is present (resp. absent) as expected.
+
+// RUN: %clang -target x86_64-linux -g -S -emit-llvm -o - %s | FileCheck %s
+
+
+enum class E0 : signed char {
+  A0 = -128,
+  B0 = 127,
+} x0;
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E0"
+// CHECK-SAME: baseType: ![[SCHAR:[0-9]+]]
+// CHECK-SAME: DIFlagFixedEnum
+// CHECK-SAME: elements: ![[ELTS0:[0-9]+]]
+// CHECK: ![[SCHAR]] = !DIBasicType(name: "signed char", size: 8, encoding: DW_ATE_signed_char)
+// CHECK: ![[ELTS0]] = !{![[A0:[0-9]+]], ![[B0:[0-9]+]]}
+// CHECK: ![[A0]] = !DIEnumerator(name: "A0", value: -128)
+// CHECK: ![[B0]] = !DIEnumerator(name: "B0", value: 127)
+
+enum class E1 : unsigned char { A1 = 255 } x1;
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E1"
+// CHECK-SAME: baseType: ![[UCHAR:[0-9]+]]
+// CHECK-SAME: DIFlagFixedEnum
+// CHECK-SAME: elements: ![[ELTS1:[0-9]+]]
+// CHECK: ![[UCHAR]] = !DIBasicType(name: "unsigned char", size: 8, encoding: DW_ATE_unsigned_char)
+// CHECK: ![[ELTS1]] = !{![[A1:[0-9]+]]}
+// CHECK: ![[A1]] = !DIEnumerator(name: "A1", value: 255, isUnsigned: true)
+
+enum class E2 : signed short {
+  A2 = -32768,
+  B2 = 32767,
+} x2;
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E2"
+// CHECK-SAME: baseType: ![[SHORT:[0-9]+]]
+// CHECK-SAME: DIFlagFixedEnum
+// CHECK-SAME: elements: ![[ELTS2:[0-9]+]]
+// CHECK: ![[SHORT]] = !DIBasicType(name: "short", size: 16, encoding: DW_ATE_signed)
+// CHECK: ![[ELTS2]] = !{![[A2:[0-9]+]], ![[B2:[0-9]+]]}
+// CHECK: ![[A2]] = !DIEnumerator(name: "A2", value: -32768)
+// CHECK: ![[B2]] = !DIEnumerator(name: "B2", value: 32767)
+
+enum class E3 : unsigned short { A3 = 65535 } x3;
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E3"
+// CHECK-SAME: baseType: ![[USHORT:[0-9]+]]
+// CHECK-SAME: DIFlagFixedEnum
+// CHECK-SAME: elements: ![[ELTS3:[0-9]+]]
+// CHECK: ![[USHORT]] = !DIBasicType(name: "unsigned short", size: 16, encoding: DW_ATE_unsigned)
+// CHECK: ![[ELTS3]] = !{![[A

[PATCH] D42736: [DebugInfo] Improvements to representation of enumeration types (PR36168)

2018-02-07 Thread Momchil Velikov via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL324490: [DebugInfo] Improvements to representation of 
enumeration types (PR36168) (authored by chill, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D42736?vs=132822&id=133228#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D42736

Files:
  cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
  cfe/trunk/test/CodeGen/debug-info-enum.cpp
  cfe/trunk/test/CodeGenCXX/debug-info-enum-class.cpp
  cfe/trunk/test/CodeGenCXX/debug-info-enum.cpp
  cfe/trunk/test/Modules/ModuleDebugInfo.cpp

Index: cfe/trunk/test/CodeGenCXX/debug-info-enum.cpp
===
--- cfe/trunk/test/CodeGenCXX/debug-info-enum.cpp
+++ cfe/trunk/test/CodeGenCXX/debug-info-enum.cpp
@@ -11,7 +11,7 @@
 // CHECK-SAME:  identifier: "_ZTSN5test11eE"
 // CHECK: [[TEST1]] = !DINamespace(name: "test1"
 // CHECK: [[TEST1_ENUMS]] = !{[[TEST1_E:![0-9]*]]}
-// CHECK: [[TEST1_E]] = !DIEnumerator(name: "E", value: 0)
+// CHECK: [[TEST1_E]] = !DIEnumerator(name: "E", value: 0, isUnsigned: true)
 enum e { E };
 void foo() {
   int v = E;
Index: cfe/trunk/test/CodeGenCXX/debug-info-enum-class.cpp
===
--- cfe/trunk/test/CodeGenCXX/debug-info-enum-class.cpp
+++ cfe/trunk/test/CodeGenCXX/debug-info-enum-class.cpp
@@ -15,20 +15,20 @@
 // CHECK-SAME: baseType: ![[INT:[0-9]+]]
 // CHECK-SAME: size: 32
 // CHECK-NOT:  offset:
-// CHECK-NOT:  flags:
+// CHECK-SAME: flags: DIFlagFixedEnum
 // CHECK-SAME: ){{$}}
 // CHECK: ![[INT]] = !DIBasicType(name: "int"
 // CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "B"
 // CHECK-SAME: line: 4
 // CHECK-SAME: baseType: ![[ULONG:[0-9]+]]
 // CHECK-SAME: size: 64
 // CHECK-NOT:  offset:
-// CHECK-NOT:  flags:
+// CHECK-SAME: flags: DIFlagFixedEnum
 // CHECK-SAME: ){{$}}
 // CHECK: ![[ULONG]] = !DIBasicType(name: "long unsigned int"
 // CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "C"
 // CHECK-SAME: line: 5
-// CHECK-NOT:  baseType:
+// CHECK-SAME: baseType: ![[ULONG:[0-9]+]]
 // CHECK-SAME: size: 32
 // CHECK-NOT:  offset:
 // CHECK-NOT:  flags:
Index: cfe/trunk/test/Modules/ModuleDebugInfo.cpp
===
--- cfe/trunk/test/Modules/ModuleDebugInfo.cpp
+++ cfe/trunk/test/Modules/ModuleDebugInfo.cpp
@@ -48,7 +48,7 @@
 // CHECK: !DICompositeType(tag: DW_TAG_enumeration_type,
 // CHECK-NOT:  name:
 // CHECK-SAME: )
-// CHECK: !DIEnumerator(name: "e5", value: 5)
+// CHECK: !DIEnumerator(name: "e5", value: 5, isUnsigned: true)
 
 // CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "B",
 // no mangled name here yet.
Index: cfe/trunk/test/CodeGen/debug-info-enum.cpp
===
--- cfe/trunk/test/CodeGen/debug-info-enum.cpp
+++ cfe/trunk/test/CodeGen/debug-info-enum.cpp
@@ -0,0 +1,100 @@
+// Test enumeration representation in debuig info metadata:
+// * test value representation for each possible underlying integer type
+// * test the integer type is as expected
+// * test the DW_AT_enum_class attribute is present (resp. absent) as expected.
+
+// RUN: %clang -target x86_64-linux -g -S -emit-llvm -o - %s | FileCheck %s
+
+
+enum class E0 : signed char {
+  A0 = -128,
+  B0 = 127,
+} x0;
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E0"
+// CHECK-SAME: baseType: ![[SCHAR:[0-9]+]]
+// CHECK-SAME: DIFlagFixedEnum
+// CHECK-SAME: elements: ![[ELTS0:[0-9]+]]
+// CHECK: ![[SCHAR]] = !DIBasicType(name: "signed char", size: 8, encoding: DW_ATE_signed_char)
+// CHECK: ![[ELTS0]] = !{![[A0:[0-9]+]], ![[B0:[0-9]+]]}
+// CHECK: ![[A0]] = !DIEnumerator(name: "A0", value: -128)
+// CHECK: ![[B0]] = !DIEnumerator(name: "B0", value: 127)
+
+enum class E1 : unsigned char { A1 = 255 } x1;
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E1"
+// CHECK-SAME: baseType: ![[UCHAR:[0-9]+]]
+// CHECK-SAME: DIFlagFixedEnum
+// CHECK-SAME: elements: ![[ELTS1:[0-9]+]]
+// CHECK: ![[UCHAR]] = !DIBasicType(name: "unsigned char", size: 8, encoding: DW_ATE_unsigned_char)
+// CHECK: ![[ELTS1]] = !{![[A1:[0-9]+]]}
+// CHECK: ![[A1]] = !DIEnumerator(name: "A1", value: 255, isUnsigned: true)
+
+enum class E2 : signed short {
+  A2 = -32768,
+  B2 = 32767,
+} x2;
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E2"
+// CHECK-SAME: baseType: ![[SHORT:[0-9]+]]
+// CHECK-SAME: DIFlagFixedEnum
+// CHECK-SAME: elements: ![[ELTS2:[0-9]+]]
+// CHECK: ![[SHORT]] = !DIBasicType(name: "short", size: 16, encoding: DW_ATE_signed)
+// CHECK: ![[ELTS2]] = !{![[A2:[0-9]+]

[PATCH] D42736: [DebugInfo] Improvements to representation of enumeration types (PR36168)

2018-02-05 Thread Momchil Velikov via Phabricator via cfe-commits

chill updated this revision to Diff 132822.
chill added a comment.

Changes since last revision:

- Add/update test to check that each enumerator belongs to the right enumeration


https://reviews.llvm.org/D42736

Files:
  lib/CodeGen/CGDebugInfo.cpp
  test/CodeGen/debug-info-enum.cpp
  test/CodeGenCXX/debug-info-enum-class.cpp
  test/CodeGenCXX/debug-info-enum.cpp
  test/Modules/ModuleDebugInfo.cpp

Index: test/Modules/ModuleDebugInfo.cpp
===
--- test/Modules/ModuleDebugInfo.cpp
+++ test/Modules/ModuleDebugInfo.cpp
@@ -48,7 +48,7 @@
 // CHECK: !DICompositeType(tag: DW_TAG_enumeration_type,
 // CHECK-NOT:  name:
 // CHECK-SAME: )
-// CHECK: !DIEnumerator(name: "e5", value: 5)
+// CHECK: !DIEnumerator(name: "e5", value: 5, isUnsigned: true)
 
 // CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "B",
 // no mangled name here yet.
Index: test/CodeGenCXX/debug-info-enum.cpp
===
--- test/CodeGenCXX/debug-info-enum.cpp
+++ test/CodeGenCXX/debug-info-enum.cpp
@@ -11,7 +11,7 @@
 // CHECK-SAME:  identifier: "_ZTSN5test11eE"
 // CHECK: [[TEST1]] = !DINamespace(name: "test1"
 // CHECK: [[TEST1_ENUMS]] = !{[[TEST1_E:![0-9]*]]}
-// CHECK: [[TEST1_E]] = !DIEnumerator(name: "E", value: 0)
+// CHECK: [[TEST1_E]] = !DIEnumerator(name: "E", value: 0, isUnsigned: true)
 enum e { E };
 void foo() {
   int v = E;
Index: test/CodeGenCXX/debug-info-enum-class.cpp
===
--- test/CodeGenCXX/debug-info-enum-class.cpp
+++ test/CodeGenCXX/debug-info-enum-class.cpp
@@ -15,20 +15,20 @@
 // CHECK-SAME: baseType: ![[INT:[0-9]+]]
 // CHECK-SAME: size: 32
 // CHECK-NOT:  offset:
-// CHECK-NOT:  flags:
+// CHECK-SAME: flags: DIFlagFixedEnum
 // CHECK-SAME: ){{$}}
 // CHECK: ![[INT]] = !DIBasicType(name: "int"
 // CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "B"
 // CHECK-SAME: line: 4
 // CHECK-SAME: baseType: ![[ULONG:[0-9]+]]
 // CHECK-SAME: size: 64
 // CHECK-NOT:  offset:
-// CHECK-NOT:  flags:
+// CHECK-SAME: flags: DIFlagFixedEnum
 // CHECK-SAME: ){{$}}
 // CHECK: ![[ULONG]] = !DIBasicType(name: "long unsigned int"
 // CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "C"
 // CHECK-SAME: line: 5
-// CHECK-NOT:  baseType:
+// CHECK-SAME: baseType: ![[ULONG:[0-9]+]]
 // CHECK-SAME: size: 32
 // CHECK-NOT:  offset:
 // CHECK-NOT:  flags:
Index: test/CodeGen/debug-info-enum.cpp
===
--- /dev/null
+++ test/CodeGen/debug-info-enum.cpp
@@ -0,0 +1,100 @@
+// Test enumeration representation in debuig info metadata:
+// * test value representation for each possible underlying integer type
+// * test the integer type is as expected
+// * test the DW_AT_enum_class attribute is present (resp. absent) as expected.
+
+// RUN: %clang -target x86_64-linux -g -S -emit-llvm -o - %s | FileCheck %s
+
+
+enum class E0 : signed char {
+  A0 = -128,
+  B0 = 127,
+} x0;
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E0"
+// CHECK-SAME: baseType: ![[SCHAR:[0-9]+]]
+// CHECK-SAME: DIFlagFixedEnum
+// CHECK-SAME: elements: ![[ELTS0:[0-9]+]]
+// CHECK: ![[SCHAR]] = !DIBasicType(name: "signed char", size: 8, encoding: DW_ATE_signed_char)
+// CHECK: ![[ELTS0]] = !{![[A0:[0-9]+]], ![[B0:[0-9]+]]}
+// CHECK: ![[A0]] = !DIEnumerator(name: "A0", value: -128)
+// CHECK: ![[B0]] = !DIEnumerator(name: "B0", value: 127)
+
+enum class E1 : unsigned char { A1 = 255 } x1;
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E1"
+// CHECK-SAME: baseType: ![[UCHAR:[0-9]+]]
+// CHECK-SAME: DIFlagFixedEnum
+// CHECK-SAME: elements: ![[ELTS1:[0-9]+]]
+// CHECK: ![[UCHAR]] = !DIBasicType(name: "unsigned char", size: 8, encoding: DW_ATE_unsigned_char)
+// CHECK: ![[ELTS1]] = !{![[A1:[0-9]+]]}
+// CHECK: ![[A1]] = !DIEnumerator(name: "A1", value: 255, isUnsigned: true)
+
+enum class E2 : signed short {
+  A2 = -32768,
+  B2 = 32767,
+} x2;
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E2"
+// CHECK-SAME: baseType: ![[SHORT:[0-9]+]]
+// CHECK-SAME: DIFlagFixedEnum
+// CHECK-SAME: elements: ![[ELTS2:[0-9]+]]
+// CHECK: ![[SHORT]] = !DIBasicType(name: "short", size: 16, encoding: DW_ATE_signed)
+// CHECK: ![[ELTS2]] = !{![[A2:[0-9]+]], ![[B2:[0-9]+]]}
+// CHECK: ![[A2]] = !DIEnumerator(name: "A2", value: -32768)
+// CHECK: ![[B2]] = !DIEnumerator(name: "B2", value: 32767)
+
+enum class E3 : unsigned short { A3 = 65535 } x3;
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E3"
+// CHECK-SAME: baseType: ![[USHORT:[0-9]+]]
+// CHECK-SAME: DIFlagFixedEnum
+// CHECK-SAME: elements: ![[ELT

[PATCH] D42736: [DebugInfo] Improvements to representation of enumeration types (PR36168)

2018-02-02 Thread David Blaikie via Phabricator via cfe-commits

dblaikie accepted this revision.
dblaikie added a comment.

Few things that could tidy up the tests a bit, but otherwise looks good :)




Comment at: test/CodeGen/debug-info-enum.cpp:6-10
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E0"
+// CHECK-SAME: flags: DIFlagFixedEnum
+// CHECK: !DIBasicType(name: "signed char", size: 8, encoding: 
DW_ATE_signed_char)
+// CHECK: !DIEnumerator(name: "A0", value: -128)
+// CHECK: !DIEnumerator(name: "B0", value: 127)

chill wrote:
> dblaikie wrote:
> > Rather than relying on specific ordering of output, generally you should 
> > test that the actual references across different metadata records are 
> > correct (eg: check that the DICompositeType's member list elements are the 
> > DIEnumerators (use named matches, rather than hardcoding the metadata node 
> > numbers))
> > 
> > Though, admittedly, this is a lot easier to read as-is. 
> I've done that, to some extent, making sure each enumeration refers to the 
> expected underlying type. Not idea how to do that completely independent of 
> order.
Thanks! Maybe add a comment (like I suggested in the LLVM review too, just now) 
in each case to describe each test case. You have one up the top for the enum 
class over signed char, but someting similar for each enum might be good.

Yeah, it's impossible to do it entirely independent of order.

You could do a bit more than what you've got here - the DIEnumerators could be 
checked:

match the list from the DICompositeType enumeration_type (the same way you 
match the baseType) then match the elements of the list, then match those 
elements to the DIEnumerators same as you do for the DIBasicType) - does that 
make sense?


https://reviews.llvm.org/D42736



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

[PATCH] D42736: [DebugInfo] Improvements to representation of enumeration types (PR36168)

2018-02-02 Thread Momchil Velikov via Phabricator via cfe-commits

chill marked 3 inline comments as done.
chill added inline comments.



Comment at: test/CodeGen/debug-info-enum.cpp:2
+// RUN: %clang -target x86_64-linux -g -S -emit-llvm -o - %s | FileCheck %s
+enum class E0 : signed char {
+  A0 = -128,

dblaikie wrote:
> Could you summarize the purpose of each of these tests (possibly in a comment 
> above the enum in each case) - there look to be more test cases than I'd 
> imagine being necessary, but I haven't carefully analyzed them.
> 
> For example: I wouldn't expect to test every integer type, if the code 
> handling them is general enough to be demonstrated by one or two cases?
Yes, in hindsight, there are more tests that strictly necessary, I just 
enumerated all the different integer type sizes cross signed/usigned, for some 
there were no errors, for some there were always errors, for some there were 
sometimes errors only sometimes (i.e. depending on compiler target and/or 
options).




Comment at: test/CodeGen/debug-info-enum.cpp:6-10
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E0"
+// CHECK-SAME: flags: DIFlagFixedEnum
+// CHECK: !DIBasicType(name: "signed char", size: 8, encoding: 
DW_ATE_signed_char)
+// CHECK: !DIEnumerator(name: "A0", value: -128)
+// CHECK: !DIEnumerator(name: "B0", value: 127)

dblaikie wrote:
> Rather than relying on specific ordering of output, generally you should test 
> that the actual references across different metadata records are correct (eg: 
> check that the DICompositeType's member list elements are the DIEnumerators 
> (use named matches, rather than hardcoding the metadata node numbers))
> 
> Though, admittedly, this is a lot easier to read as-is. 
I've done that, to some extent, making sure each enumeration refers to the 
expected underlying type. Not idea how to do that completely independent of 
order.


https://reviews.llvm.org/D42736



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

[PATCH] D42736: [DebugInfo] Improvements to representation of enumeration types (PR36168)

2018-02-02 Thread Momchil Velikov via Phabricator via cfe-commits

chill updated this revision to Diff 132587.
chill added a comment.

Changes, relative to the previous revision:

- a few tweaks to the tests


https://reviews.llvm.org/D42736

Files:
  lib/CodeGen/CGDebugInfo.cpp
  test/CodeGen/debug-info-enum.cpp
  test/CodeGenCXX/debug-info-enum-class.cpp
  test/CodeGenCXX/debug-info-enum.cpp
  test/Modules/ModuleDebugInfo.cpp

Index: test/Modules/ModuleDebugInfo.cpp
===
--- test/Modules/ModuleDebugInfo.cpp
+++ test/Modules/ModuleDebugInfo.cpp
@@ -48,7 +48,7 @@
 // CHECK: !DICompositeType(tag: DW_TAG_enumeration_type,
 // CHECK-NOT:  name:
 // CHECK-SAME: )
-// CHECK: !DIEnumerator(name: "e5", value: 5)
+// CHECK: !DIEnumerator(name: "e5", value: 5, isUnsigned: true)
 
 // CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "B",
 // no mangled name here yet.
Index: test/CodeGenCXX/debug-info-enum.cpp
===
--- test/CodeGenCXX/debug-info-enum.cpp
+++ test/CodeGenCXX/debug-info-enum.cpp
@@ -11,7 +11,7 @@
 // CHECK-SAME:  identifier: "_ZTSN5test11eE"
 // CHECK: [[TEST1]] = !DINamespace(name: "test1"
 // CHECK: [[TEST1_ENUMS]] = !{[[TEST1_E:![0-9]*]]}
-// CHECK: [[TEST1_E]] = !DIEnumerator(name: "E", value: 0)
+// CHECK: [[TEST1_E]] = !DIEnumerator(name: "E", value: 0, isUnsigned: true)
 enum e { E };
 void foo() {
   int v = E;
Index: test/CodeGenCXX/debug-info-enum-class.cpp
===
--- test/CodeGenCXX/debug-info-enum-class.cpp
+++ test/CodeGenCXX/debug-info-enum-class.cpp
@@ -15,20 +15,20 @@
 // CHECK-SAME: baseType: ![[INT:[0-9]+]]
 // CHECK-SAME: size: 32
 // CHECK-NOT:  offset:
-// CHECK-NOT:  flags:
+// CHECK-SAME: flags: DIFlagFixedEnum
 // CHECK-SAME: ){{$}}
 // CHECK: ![[INT]] = !DIBasicType(name: "int"
 // CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "B"
 // CHECK-SAME: line: 4
 // CHECK-SAME: baseType: ![[ULONG:[0-9]+]]
 // CHECK-SAME: size: 64
 // CHECK-NOT:  offset:
-// CHECK-NOT:  flags:
+// CHECK-SAME: flags: DIFlagFixedEnum
 // CHECK-SAME: ){{$}}
 // CHECK: ![[ULONG]] = !DIBasicType(name: "long unsigned int"
 // CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "C"
 // CHECK-SAME: line: 5
-// CHECK-NOT:  baseType:
+// CHECK-SAME: baseType: ![[ULONG:[0-9]+]]
 // CHECK-SAME: size: 32
 // CHECK-NOT:  offset:
 // CHECK-NOT:  flags:
Index: test/CodeGen/debug-info-enum.cpp
===
--- /dev/null
+++ test/CodeGen/debug-info-enum.cpp
@@ -0,0 +1,80 @@
+// RUN: %clang -target x86_64-linux -g -S -emit-llvm -o - %s | FileCheck %s
+
+// Test enumerator value representation for each possible underlying integer
+// type; test the integer type and FixedEnum flags are as expected.
+enum class E0 : signed char {
+  A0 = -128,
+  B0 = 127,
+} x0;
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E0"
+// CHECK-SAME: baseType: ![[SCHAR:[0-9]+]]
+// CHECK-SAME: DIFlagFixedEnum
+// CHECK: ![[SCHAR]] = !DIBasicType(name: "signed char", size: 8, encoding: DW_ATE_signed_char)
+// CHECK: !DIEnumerator(name: "A0", value: -128)
+// CHECK: !DIEnumerator(name: "B0", value: 127)
+
+enum class E1 : unsigned char { A1 = 255 } x1;
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E1"
+// CHECK-SAME: baseType: ![[UCHAR:[0-9]+]]
+// CHECK-SAME: DIFlagFixedEnum
+// CHECK: ![[UCHAR]] = !DIBasicType(name: "unsigned char", size: 8, encoding: DW_ATE_unsigned_char)
+// CHECK: !DIEnumerator(name: "A1", value: 255, isUnsigned: true)
+
+enum class E2 : signed short {
+  A2 = -32768,
+  B2 = 32767,
+} x2;
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E2"
+// CHECK-SAME: baseType: ![[SHORT:[0-9]+]]
+// CHECK-SAME: DIFlagFixedEnum
+// CHECK: ![[SHORT]] = !DIBasicType(name: "short", size: 16, encoding: DW_ATE_signed)
+// CHECK: !DIEnumerator(name: "A2", value: -32768)
+// CHECK: !DIEnumerator(name: "B2", value: 32767)
+
+enum class E3 : unsigned short { A3 = 65535 } x3;
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E3"
+// CHECK-SAME: baseType: ![[USHORT:[0-9]+]]
+// CHECK-SAME: DIFlagFixedEnum
+// CHECK: ![[USHORT]] = !DIBasicType(name: "unsigned short", size: 16, encoding: DW_ATE_unsigned)
+// CHECK: !DIEnumerator(name: "A3", value: 65535, isUnsigned: true)
+
+enum class E4 : signed int { A4 = -2147483648, B4 = 2147483647 } x4;
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E4"
+// CHECK-SAME: baseType: ![[INT:[0-9]+]]
+// CHECK-SAME: DIFlagFixedEnum
+// CHECK: ![[INT]] = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+// CHECK: !DIEnumerator(name: "A4", value: -2147483648)
+//

[PATCH] D42736: [DebugInfo] Improvements to representation of enumeration types (PR36168)

2018-02-01 Thread David Blaikie via Phabricator via cfe-commits

dblaikie added inline comments.



Comment at: test/CodeGen/debug-info-enum.cpp:2
+// RUN: %clang -target x86_64-linux -g -S -emit-llvm -o - %s | FileCheck %s
+enum class E0 : signed char {
+  A0 = -128,

Could you summarize the purpose of each of these tests (possibly in a comment 
above the enum in each case) - there look to be more test cases than I'd 
imagine being necessary, but I haven't carefully analyzed them.

For example: I wouldn't expect to test every integer type, if the code handling 
them is general enough to be demonstrated by one or two cases?



Comment at: test/CodeGen/debug-info-enum.cpp:6-10
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E0"
+// CHECK-SAME: flags: DIFlagFixedEnum
+// CHECK: !DIBasicType(name: "signed char", size: 8, encoding: 
DW_ATE_signed_char)
+// CHECK: !DIEnumerator(name: "A0", value: -128)
+// CHECK: !DIEnumerator(name: "B0", value: 127)

Rather than relying on specific ordering of output, generally you should test 
that the actual references across different metadata records are correct (eg: 
check that the DICompositeType's member list elements are the DIEnumerators 
(use named matches, rather than hardcoding the metadata node numbers))

Though, admittedly, this is a lot easier to read as-is. 



Comment at: test/CodeGen/debug-info-enum.cpp:66
+// CHECK-SAME: baseType: ![[INT]]
+// CHECK-NOT: flags: DIFlagFixedEnum
+// CHECK: !DIEnumerator(name: "A8", value: -128)

Probably drop the "flags: " part of this NOT check - so that if other flags are 
added, this won't be overly constrained (eg: if this type ended up with "flags: 
X | DIFlagFixedEnum" this NOT check as-is wouldn't catch the regression)


https://reviews.llvm.org/D42736



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

[PATCH] D42736: [DebugInfo] Improvements to representation of enumeration types (PR36168)

2018-02-01 Thread Momchil Velikov via Phabricator via cfe-commits

chill updated this revision to Diff 132414.
chill added a comment.

Minor update, the parameter to `createEnumerator` is `IsUnsigned` now (instead 
of `IsSigned`), changed caller.


https://reviews.llvm.org/D42736

Files:
  lib/CodeGen/CGDebugInfo.cpp
  test/CodeGen/debug-info-enum.cpp
  test/CodeGenCXX/debug-info-enum-class.cpp
  test/CodeGenCXX/debug-info-enum.cpp
  test/Modules/ModuleDebugInfo.cpp

Index: test/Modules/ModuleDebugInfo.cpp
===
--- test/Modules/ModuleDebugInfo.cpp
+++ test/Modules/ModuleDebugInfo.cpp
@@ -48,7 +48,7 @@
 // CHECK: !DICompositeType(tag: DW_TAG_enumeration_type,
 // CHECK-NOT:  name:
 // CHECK-SAME: )
-// CHECK: !DIEnumerator(name: "e5", value: 5)
+// CHECK: !DIEnumerator(name: "e5", value: 5, isUnsigned: true)
 
 // CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "B",
 // no mangled name here yet.
Index: test/CodeGenCXX/debug-info-enum.cpp
===
--- test/CodeGenCXX/debug-info-enum.cpp
+++ test/CodeGenCXX/debug-info-enum.cpp
@@ -11,7 +11,7 @@
 // CHECK-SAME:  identifier: "_ZTSN5test11eE"
 // CHECK: [[TEST1]] = !DINamespace(name: "test1"
 // CHECK: [[TEST1_ENUMS]] = !{[[TEST1_E:![0-9]*]]}
-// CHECK: [[TEST1_E]] = !DIEnumerator(name: "E", value: 0)
+// CHECK: [[TEST1_E]] = !DIEnumerator(name: "E", value: 0, isUnsigned: true)
 enum e { E };
 void foo() {
   int v = E;
Index: test/CodeGenCXX/debug-info-enum-class.cpp
===
--- test/CodeGenCXX/debug-info-enum-class.cpp
+++ test/CodeGenCXX/debug-info-enum-class.cpp
@@ -15,20 +15,20 @@
 // CHECK-SAME: baseType: ![[INT:[0-9]+]]
 // CHECK-SAME: size: 32
 // CHECK-NOT:  offset:
-// CHECK-NOT:  flags:
+// CHECK-SAME: flags: DIFlagFixedEnum
 // CHECK-SAME: ){{$}}
 // CHECK: ![[INT]] = !DIBasicType(name: "int"
 // CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "B"
 // CHECK-SAME: line: 4
 // CHECK-SAME: baseType: ![[ULONG:[0-9]+]]
 // CHECK-SAME: size: 64
 // CHECK-NOT:  offset:
-// CHECK-NOT:  flags:
+// CHECK-SAME: flags: DIFlagFixedEnum
 // CHECK-SAME: ){{$}}
 // CHECK: ![[ULONG]] = !DIBasicType(name: "long unsigned int"
 // CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "C"
 // CHECK-SAME: line: 5
-// CHECK-NOT:  baseType:
+// CHECK-SAME: baseType: ![[ULONG:[0-9]+]]
 // CHECK-SAME: size: 32
 // CHECK-NOT:  offset:
 // CHECK-NOT:  flags:
Index: test/CodeGen/debug-info-enum.cpp
===
--- /dev/null
+++ test/CodeGen/debug-info-enum.cpp
@@ -0,0 +1,68 @@
+// RUN: %clang -target x86_64-linux -g -S -emit-llvm -o - %s | FileCheck %s
+enum class E0 : signed char {
+  A0 = -128,
+  B0 = 127,
+} x0;
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E0"
+// CHECK-SAME: flags: DIFlagFixedEnum
+// CHECK: !DIBasicType(name: "signed char", size: 8, encoding: DW_ATE_signed_char)
+// CHECK: !DIEnumerator(name: "A0", value: -128)
+// CHECK: !DIEnumerator(name: "B0", value: 127)
+
+enum class E1 : unsigned char { A1 = 255 } x1;
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E1"
+// CHECK-SAME: flags: DIFlagFixedEnum
+// CHECK: !DIBasicType(name: "unsigned char", size: 8, encoding: DW_ATE_unsigned_char)
+// CHECK: !DIEnumerator(name: "A1", value: 255, isUnsigned: true)
+
+enum class E2 : signed short {
+  A2 = -32768,
+  B2 = 32767,
+} x2;
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E2"
+// CHECK-SAME: flags: DIFlagFixedEnum
+// CHECK: !DIBasicType(name: "short", size: 16, encoding: DW_ATE_signed)
+// CHECK: !DIEnumerator(name: "A2", value: -32768)
+// CHECK: !DIEnumerator(name: "B2", value: 32767)
+
+enum class E3 : unsigned short { A3 = 65535 } x3;
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E3"
+// CHECK-SAME: DIFlagFixedEnum
+// CHECK: !DIBasicType(name: "unsigned short", size: 16, encoding: DW_ATE_unsigned)
+// CHECK: !DIEnumerator(name: "A3", value: 65535, isUnsigned: true)
+
+enum class E4 : signed int { A4 = -2147483648, B4 = 2147483647 } x4;
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E4"
+// CHECK-SAME: flags: DIFlagFixedEnum
+// CHECK: ![[INT:[0-9]+]] = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+// CHECK: !DIEnumerator(name: "A4", value: -2147483648)
+// CHECK: !DIEnumerator(name: "B4", value: 2147483647)
+
+enum class E5 : unsigned int { A5 = 4294967295 } x5;
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E5"
+// CHECK-SAME: flags: DIFlagFixedEnum
+// CHECK: !DIBasicType(name: "unsigned int", size: 32, encoding: DW_ATE_unsigned)
+// CHECK: !DIEnumerator(name: "A5", value: 4294

[PATCH] D42736: [DebugInfo] Improvements to representation of enumeration types (PR36168)

2018-01-31 Thread David Blaikie via Phabricator via cfe-commits

dblaikie added inline comments.



Comment at: lib/CodeGen/CGDebugInfo.cpp:2490-2491
   for (const auto *Enum : ED->enumerators()) {
-Enumerators.push_back(DBuilder.createEnumerator(
-Enum->getName(), Enum->getInitVal().getSExtValue()));
+const auto &InitVal = Enum->getInitVal();
+bool IsSigned = ED->getIntegerType()->isSignedIntegerType();
+auto Value = IsSigned ? InitVal.getSExtValue() : InitVal.getZExtValue();

Move this "IsSigned" outside the loop, since it's loop invariant?



Repository:
  rC Clang

https://reviews.llvm.org/D42736



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

[PATCH] D42736: [DebugInfo] Improvements to representation of enumeration types (PR36168)

2018-01-31 Thread Momchil Velikov via Phabricator via cfe-commits

chill created this revision.
chill added reviewers: dblaikie, keith.walker.arm.
Herald added subscribers: JDevlieghere, aprantl.

This is the `clang` counterpart to https://reviews.llvm.org/D42734

This patch:

- fixed an incorrect sign-extension of unsigned  values, when emitting debug 
info metadata for enumerators
- the enumerators metadata is created with a flag, which determines 
interpretation of the value bits (signed or unsigned)
- the enumerations metadata contains the underlying integer type and a flag, 
indicating whether this is a C++ "fixed enum"


Repository:
  rC Clang

https://reviews.llvm.org/D42736

Files:
  lib/CodeGen/CGDebugInfo.cpp
  test/CodeGen/debug-info-enum.cpp
  test/CodeGenCXX/debug-info-enum-class.cpp
  test/CodeGenCXX/debug-info-enum.cpp
  test/Modules/ModuleDebugInfo.cpp

Index: test/Modules/ModuleDebugInfo.cpp
===
--- test/Modules/ModuleDebugInfo.cpp
+++ test/Modules/ModuleDebugInfo.cpp
@@ -48,7 +48,7 @@
 // CHECK: !DICompositeType(tag: DW_TAG_enumeration_type,
 // CHECK-NOT:  name:
 // CHECK-SAME: )
-// CHECK: !DIEnumerator(name: "e5", value: 5)
+// CHECK: !DIEnumerator(name: "e5", value: 5, isSigned: false)
 
 // CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "B",
 // no mangled name here yet.
Index: test/CodeGenCXX/debug-info-enum.cpp
===
--- test/CodeGenCXX/debug-info-enum.cpp
+++ test/CodeGenCXX/debug-info-enum.cpp
@@ -11,7 +11,7 @@
 // CHECK-SAME:  identifier: "_ZTSN5test11eE"
 // CHECK: [[TEST1]] = !DINamespace(name: "test1"
 // CHECK: [[TEST1_ENUMS]] = !{[[TEST1_E:![0-9]*]]}
-// CHECK: [[TEST1_E]] = !DIEnumerator(name: "E", value: 0)
+// CHECK: [[TEST1_E]] = !DIEnumerator(name: "E", value: 0, isSigned: false)
 enum e { E };
 void foo() {
   int v = E;
Index: test/CodeGenCXX/debug-info-enum-class.cpp
===
--- test/CodeGenCXX/debug-info-enum-class.cpp
+++ test/CodeGenCXX/debug-info-enum-class.cpp
@@ -15,20 +15,20 @@
 // CHECK-SAME: baseType: ![[INT:[0-9]+]]
 // CHECK-SAME: size: 32
 // CHECK-NOT:  offset:
-// CHECK-NOT:  flags:
+// CHECK-SAME: flags: DIFlagFixedEnum
 // CHECK-SAME: ){{$}}
 // CHECK: ![[INT]] = !DIBasicType(name: "int"
 // CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "B"
 // CHECK-SAME: line: 4
 // CHECK-SAME: baseType: ![[ULONG:[0-9]+]]
 // CHECK-SAME: size: 64
 // CHECK-NOT:  offset:
-// CHECK-NOT:  flags:
+// CHECK-SAME: flags: DIFlagFixedEnum
 // CHECK-SAME: ){{$}}
 // CHECK: ![[ULONG]] = !DIBasicType(name: "long unsigned int"
 // CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "C"
 // CHECK-SAME: line: 5
-// CHECK-NOT:  baseType:
+// CHECK-SAME: baseType: ![[ULONG:[0-9]+]]
 // CHECK-SAME: size: 32
 // CHECK-NOT:  offset:
 // CHECK-NOT:  flags:
Index: test/CodeGen/debug-info-enum.cpp
===
--- /dev/null
+++ test/CodeGen/debug-info-enum.cpp
@@ -0,0 +1,68 @@
+// RUN: %clang -target x86_64-linux -g -S -emit-llvm -o - %s | FileCheck %s
+enum class E0 : signed char {
+  A0 = -128,
+  B0 = 127,
+} x0;
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E0"
+// CHECK-SAME: flags: DIFlagFixedEnum
+// CHECK: !DIBasicType(name: "signed char", size: 8, encoding: DW_ATE_signed_char)
+// CHECK: !DIEnumerator(name: "A0", value: -128)
+// CHECK: !DIEnumerator(name: "B0", value: 127)
+
+enum class E1 : unsigned char { A1 = 255 } x1;
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E1"
+// CHECK-SAME: flags: DIFlagFixedEnum
+// CHECK: !DIBasicType(name: "unsigned char", size: 8, encoding: DW_ATE_unsigned_char)
+// CHECK: !DIEnumerator(name: "A1", value: 255, isSigned: false)
+
+enum class E2 : signed short {
+  A2 = -32768,
+  B2 = 32767,
+} x2;
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E2"
+// CHECK-SAME: flags: DIFlagFixedEnum
+// CHECK: !DIBasicType(name: "short", size: 16, encoding: DW_ATE_signed)
+// CHECK: !DIEnumerator(name: "A2", value: -32768)
+// CHECK: !DIEnumerator(name: "B2", value: 32767)
+
+enum class E3 : unsigned short { A3 = 65535 } x3;
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E3"
+// CHECK-SAME: DIFlagFixedEnum
+// CHECK: !DIBasicType(name: "unsigned short", size: 16, encoding: DW_ATE_unsigned)
+// CHECK: !DIEnumerator(name: "A3", value: 65535, isSigned: false)
+
+enum class E4 : signed int { A4 = -2147483648, B4 = 2147483647 } x4;
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E4"
+// CHECK-SAME: flags: DIFlagFixedEnum
+// CHECK: ![[INT:[0-9]+]] = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+/

[PATCH] D42736: [DebugInfo] Improvements to representation of enumeration types (PR36168)

[PATCH] D42736: [DebugInfo] Improvements to representation of enumeration types (PR36168)

[PATCH] D42736: [DebugInfo] Improvements to representation of enumeration types (PR36168)

[PATCH] D42736: [DebugInfo] Improvements to representation of enumeration types (PR36168)

[PATCH] D42736: [DebugInfo] Improvements to representation of enumeration types (PR36168)

[PATCH] D42736: [DebugInfo] Improvements to representation of enumeration types (PR36168)

[PATCH] D42736: [DebugInfo] Improvements to representation of enumeration types (PR36168)

[PATCH] D42736: [DebugInfo] Improvements to representation of enumeration types (PR36168)

[PATCH] D42736: [DebugInfo] Improvements to representation of enumeration types (PR36168)

[PATCH] D42736: [DebugInfo] Improvements to representation of enumeration types (PR36168)

10 matches

Site Navigation

Mail list logo

Footer information