[PATCH] D124006: [DebugInfo] Use the 'getTypeAlignIfRequired' function to get DW_AT_alignment correct when attribute((__aligned__)) is present.

2022-04-22 Thread Ying Yi via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb09ba4262076: Bug 51277: [DWARF] DW_AT_alignment incorrect 
when (authored by MaggieYi).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124006

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/debug-info-struct-align.cpp


Index: clang/test/CodeGenCXX/debug-info-struct-align.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-struct-align.cpp
@@ -0,0 +1,27 @@
+//  Test for debug info related to DW_AT_alignment attribute in the struct 
type.
+// RUN: %clang_cc1 -dwarf-version=5 -debug-info-kind=standalone -S -emit-llvm 
%s -o - | FileCheck %s
+
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType", 
{{.*}}, align: 32
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType1", 
{{.*}}, align: 8
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType2", 
{{.*}}, align: 8
+
+struct MyType {
+  int m;
+} __attribute__((aligned(1)));
+MyType mt;
+
+static_assert(alignof(MyType) == 4, "alignof MyType is wrong");
+
+struct MyType1 {
+  int m;
+} __attribute__((packed, aligned(1)));
+MyType1 mt1;
+
+static_assert(alignof(MyType1) == 1, "alignof MyType1 is wrong");
+
+struct MyType2 {
+  __attribute__((packed)) int m;
+} __attribute__((aligned(1)));
+MyType2 mt2;
+
+static_assert(alignof(MyType2) == 1, "alignof MyType2 is wrong");
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3561,7 +3561,11 @@
 return getOrCreateRecordFwdDecl(Ty, RDContext);
 
   uint64_t Size = CGM.getContext().getTypeSize(Ty);
-  auto Align = getDeclAlignIfRequired(D, CGM.getContext());
+  // __attribute__((aligned)) can increase or decrease alignment *except* on a
+  // struct or struct member, where it only increases  alignment unless 
'packed'
+  // is also specified. To handle this case, the `getTypeAlignIfRequired` needs
+  // to be used.
+  auto Align = getTypeAlignIfRequired(Ty, CGM.getContext());
 
   SmallString<256> Identifier = getTypeIdentifier(Ty, CGM, TheCU);
 


Index: clang/test/CodeGenCXX/debug-info-struct-align.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-struct-align.cpp
@@ -0,0 +1,27 @@
+//  Test for debug info related to DW_AT_alignment attribute in the struct type.
+// RUN: %clang_cc1 -dwarf-version=5 -debug-info-kind=standalone -S -emit-llvm %s -o - | FileCheck %s
+
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType", {{.*}}, align: 32
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType1", {{.*}}, align: 8
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType2", {{.*}}, align: 8
+
+struct MyType {
+  int m;
+} __attribute__((aligned(1)));
+MyType mt;
+
+static_assert(alignof(MyType) == 4, "alignof MyType is wrong");
+
+struct MyType1 {
+  int m;
+} __attribute__((packed, aligned(1)));
+MyType1 mt1;
+
+static_assert(alignof(MyType1) == 1, "alignof MyType1 is wrong");
+
+struct MyType2 {
+  __attribute__((packed)) int m;
+} __attribute__((aligned(1)));
+MyType2 mt2;
+
+static_assert(alignof(MyType2) == 1, "alignof MyType2 is wrong");
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3561,7 +3561,11 @@
 return getOrCreateRecordFwdDecl(Ty, RDContext);
 
   uint64_t Size = CGM.getContext().getTypeSize(Ty);
-  auto Align = getDeclAlignIfRequired(D, CGM.getContext());
+  // __attribute__((aligned)) can increase or decrease alignment *except* on a
+  // struct or struct member, where it only increases  alignment unless 'packed'
+  // is also specified. To handle this case, the `getTypeAlignIfRequired` needs
+  // to be used.
+  auto Align = getTypeAlignIfRequired(Ty, CGM.getContext());
 
   SmallString<256> Identifier = getTypeIdentifier(Ty, CGM, TheCU);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124006: [DebugInfo] Use the 'getTypeAlignIfRequired' function to get DW_AT_alignment correct when attribute((__aligned__)) is present.

2022-04-22 Thread Ying Yi via Phabricator via cfe-commits 
MaggieYi added inline comments.



Comment at: clang/test/CodeGenCXX/debug-info-struct-align.cpp:11
+} __attribute__((aligned(1)));
+struct MyType mt;
+

dblaikie wrote:
> You can drop the "struct" here and from other references to these types (in 
> mt1/mt2 and the alignof calls)
Thanks David. All unnecessary `struct`s have been removed. 


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

https://reviews.llvm.org/D124006

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

[PATCH] D124006: [DebugInfo] Use the 'getTypeAlignIfRequired' function to get DW_AT_alignment correct when attribute((__aligned__)) is present.

2022-04-22 Thread Ying Yi via Phabricator via cfe-commits 
MaggieYi updated this revision to Diff 424420.

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

https://reviews.llvm.org/D124006

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/debug-info-struct-align.cpp


Index: clang/test/CodeGenCXX/debug-info-struct-align.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-struct-align.cpp
@@ -0,0 +1,27 @@
+//  Test for debug info related to DW_AT_alignment attribute in the struct 
type.
+// RUN: %clang_cc1 -dwarf-version=5 -debug-info-kind=standalone -S -emit-llvm 
%s -o - | FileCheck %s
+
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType", 
{{.*}}, align: 32
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType1", 
{{.*}}, align: 8
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType2", 
{{.*}}, align: 8
+
+struct MyType {
+  int m;
+} __attribute__((aligned(1)));
+MyType mt;
+
+static_assert(alignof(MyType) == 4, "alignof MyType is wrong");
+
+struct MyType1 {
+  int m;
+} __attribute__((packed, aligned(1)));
+MyType1 mt1;
+
+static_assert(alignof(MyType1) == 1, "alignof MyType1 is wrong");
+
+struct MyType2 {
+  __attribute__((packed)) int m;
+} __attribute__((aligned(1)));
+MyType2 mt2;
+
+static_assert(alignof(MyType2) == 1, "alignof MyType2 is wrong");
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3561,7 +3561,11 @@
 return getOrCreateRecordFwdDecl(Ty, RDContext);
 
   uint64_t Size = CGM.getContext().getTypeSize(Ty);
-  auto Align = getDeclAlignIfRequired(D, CGM.getContext());
+  // __attribute__((aligned)) can increase or decrease alignment *except* on a
+  // struct or struct member, where it only increases  alignment unless 
'packed'
+  // is also specified. To handle this case, the `getTypeAlignIfRequired` needs
+  // to be used.
+  auto Align = getTypeAlignIfRequired(Ty, CGM.getContext());
 
   SmallString<256> Identifier = getTypeIdentifier(Ty, CGM, TheCU);
 


Index: clang/test/CodeGenCXX/debug-info-struct-align.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-struct-align.cpp
@@ -0,0 +1,27 @@
+//  Test for debug info related to DW_AT_alignment attribute in the struct type.
+// RUN: %clang_cc1 -dwarf-version=5 -debug-info-kind=standalone -S -emit-llvm %s -o - | FileCheck %s
+
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType", {{.*}}, align: 32
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType1", {{.*}}, align: 8
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType2", {{.*}}, align: 8
+
+struct MyType {
+  int m;
+} __attribute__((aligned(1)));
+MyType mt;
+
+static_assert(alignof(MyType) == 4, "alignof MyType is wrong");
+
+struct MyType1 {
+  int m;
+} __attribute__((packed, aligned(1)));
+MyType1 mt1;
+
+static_assert(alignof(MyType1) == 1, "alignof MyType1 is wrong");
+
+struct MyType2 {
+  __attribute__((packed)) int m;
+} __attribute__((aligned(1)));
+MyType2 mt2;
+
+static_assert(alignof(MyType2) == 1, "alignof MyType2 is wrong");
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3561,7 +3561,11 @@
 return getOrCreateRecordFwdDecl(Ty, RDContext);
 
   uint64_t Size = CGM.getContext().getTypeSize(Ty);
-  auto Align = getDeclAlignIfRequired(D, CGM.getContext());
+  // __attribute__((aligned)) can increase or decrease alignment *except* on a
+  // struct or struct member, where it only increases  alignment unless 'packed'
+  // is also specified. To handle this case, the `getTypeAlignIfRequired` needs
+  // to be used.
+  auto Align = getTypeAlignIfRequired(Ty, CGM.getContext());
 
   SmallString<256> Identifier = getTypeIdentifier(Ty, CGM, TheCU);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124006: [DebugInfo] Use the 'getTypeAlignIfRequired' function to get DW_AT_alignment correct when attribute((__aligned__)) is present.

2022-04-21 Thread David Blaikie via Phabricator via cfe-commits 
dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Looks good, thanks!




Comment at: clang/test/CodeGenCXX/debug-info-struct-align.cpp:11
+} __attribute__((aligned(1)));
+struct MyType mt;
+

You can drop the "struct" here and from other references to these types (in 
mt1/mt2 and the alignof calls)


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

https://reviews.llvm.org/D124006

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

[PATCH] D124006: [DebugInfo] Use the 'getTypeAlignIfRequired' function to get DW_AT_alignment correct when attribute((__aligned__)) is present.

2022-04-21 Thread Ying Yi via Phabricator via cfe-commits 
MaggieYi updated this revision to Diff 424257.
MaggieYi added a comment.

Thanks David, I have modified the test following your advice.


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

https://reviews.llvm.org/D124006

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/debug-info-struct-align.cpp


Index: clang/test/CodeGenCXX/debug-info-struct-align.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-struct-align.cpp
@@ -0,0 +1,27 @@
+//  Test for debug info related to DW_AT_alignment attribute in the struct 
type.
+// RUN: %clang_cc1 -dwarf-version=5 -debug-info-kind=standalone -S -emit-llvm 
%s -o - | FileCheck %s
+
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType", 
{{.*}}, align: 32
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType1", 
{{.*}}, align: 8
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType2", 
{{.*}}, align: 8
+
+struct MyType {
+  int m;
+} __attribute__((aligned(1)));
+struct MyType mt;
+
+static_assert(alignof(struct MyType) == 4, "alignof MyType is wrong");
+
+struct MyType1 {
+  int m;
+} __attribute__((packed, aligned(1)));
+struct MyType1 mt1;
+
+static_assert(alignof(struct MyType1) == 1, "alignof MyType1 is wrong");
+
+struct MyType2 {
+  __attribute__((packed)) int m;
+} __attribute__((aligned(1)));
+struct MyType2 mt2;
+
+static_assert(alignof(struct MyType2) == 1, "alignof MyType2 is wrong");
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3553,7 +3553,11 @@
 return getOrCreateRecordFwdDecl(Ty, RDContext);
 
   uint64_t Size = CGM.getContext().getTypeSize(Ty);
-  auto Align = getDeclAlignIfRequired(D, CGM.getContext());
+  // __attribute__((aligned)) can increase or decrease alignment *except* on a
+  // struct or struct member, where it only increases  alignment unless 
'packed'
+  // is also specified. To handle this case, the `getTypeAlignIfRequired` needs
+  // to be used.
+  auto Align = getTypeAlignIfRequired(Ty, CGM.getContext());
 
   SmallString<256> Identifier = getTypeIdentifier(Ty, CGM, TheCU);
 


Index: clang/test/CodeGenCXX/debug-info-struct-align.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-struct-align.cpp
@@ -0,0 +1,27 @@
+//  Test for debug info related to DW_AT_alignment attribute in the struct type.
+// RUN: %clang_cc1 -dwarf-version=5 -debug-info-kind=standalone -S -emit-llvm %s -o - | FileCheck %s
+
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType", {{.*}}, align: 32
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType1", {{.*}}, align: 8
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType2", {{.*}}, align: 8
+
+struct MyType {
+  int m;
+} __attribute__((aligned(1)));
+struct MyType mt;
+
+static_assert(alignof(struct MyType) == 4, "alignof MyType is wrong");
+
+struct MyType1 {
+  int m;
+} __attribute__((packed, aligned(1)));
+struct MyType1 mt1;
+
+static_assert(alignof(struct MyType1) == 1, "alignof MyType1 is wrong");
+
+struct MyType2 {
+  __attribute__((packed)) int m;
+} __attribute__((aligned(1)));
+struct MyType2 mt2;
+
+static_assert(alignof(struct MyType2) == 1, "alignof MyType2 is wrong");
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3553,7 +3553,11 @@
 return getOrCreateRecordFwdDecl(Ty, RDContext);
 
   uint64_t Size = CGM.getContext().getTypeSize(Ty);
-  auto Align = getDeclAlignIfRequired(D, CGM.getContext());
+  // __attribute__((aligned)) can increase or decrease alignment *except* on a
+  // struct or struct member, where it only increases  alignment unless 'packed'
+  // is also specified. To handle this case, the `getTypeAlignIfRequired` needs
+  // to be used.
+  auto Align = getTypeAlignIfRequired(Ty, CGM.getContext());
 
   SmallString<256> Identifier = getTypeIdentifier(Ty, CGM, TheCU);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124006: [DebugInfo] Use the 'getTypeAlignIfRequired' function to get DW_AT_alignment correct when attribute((__aligned__)) is present.

2022-04-19 Thread David Blaikie via Phabricator via cfe-commits 
dblaikie added a comment.

Could you include some static_asserts(+alignof) in the test case to demonstrate 
these are the alignments that the language is computing too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124006

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

[PATCH] D124006: [DebugInfo] Use the 'getTypeAlignIfRequired' function to get DW_AT_alignment correct when attribute((__aligned__)) is present.

2022-04-19 Thread Ying Yi via Phabricator via cfe-commits 
MaggieYi created this revision.
MaggieYi added reviewers: echristo, jmorse, wolfgangp, probinson, dblaikie, 
aprantl.
Herald added a project: All.
MaggieYi requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The detailed description of the issue could be found in 
https://bugs.llvm.org/show_bug.cgi?id=51277.

In the original code, the 'getDeclAlignIfRequired' function is used.
The 'getDeclAlignIfRequired' function will return the max alignment
of all aligned attributes if the type has aligned attributes. The
function doesn’t consider the type at all.

The 'getTypeAlignIfRequired' function uses the type’s alignment value,
which also used by the 'alignof' function. I think we should use the
function of 'getTypeAlignIfRequired'.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124006

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGen/debug-info-struct-align.c


Index: clang/test/CodeGen/debug-info-struct-align.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-struct-align.c
@@ -0,0 +1,21 @@
+//  Test for debug info related to DW_AT_alignment attribute in the struct 
type.
+// RUN: %clang_cc1 -dwarf-version=5 -debug-info-kind=standalone -S -emit-llvm 
%s -o - | FileCheck %s
+
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType", 
{{.*}}, align: 32
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType1", 
{{.*}}, align: 8
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType2", 
{{.*}}, align: 8
+
+struct MyType {
+  int m;
+} __attribute__((aligned(1)));
+struct MyType mt;
+
+struct MyType1 {
+  int m;
+} __attribute__((packed, aligned(1)));
+struct MyType1 mt1;
+
+struct MyType2 {
+  __attribute__((packed))int m;
+} __attribute__((aligned(1)));
+struct MyType2 mt2;
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3553,7 +3553,11 @@
 return getOrCreateRecordFwdDecl(Ty, RDContext);
 
   uint64_t Size = CGM.getContext().getTypeSize(Ty);
-  auto Align = getDeclAlignIfRequired(D, CGM.getContext());
+  // __attribute__((aligned)) can increase or decrease alignment *except* on a
+  // struct or struct member, where it only increases  alignment unless 
'packed'
+  // is also specified. To handle this case, the `getTypeAlignIfRequired` needs
+  // to be used.
+  auto Align = getTypeAlignIfRequired(Ty, CGM.getContext());
 
   SmallString<256> Identifier = getTypeIdentifier(Ty, CGM, TheCU);
 


Index: clang/test/CodeGen/debug-info-struct-align.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-struct-align.c
@@ -0,0 +1,21 @@
+//  Test for debug info related to DW_AT_alignment attribute in the struct type.
+// RUN: %clang_cc1 -dwarf-version=5 -debug-info-kind=standalone -S -emit-llvm %s -o - | FileCheck %s
+
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType", {{.*}}, align: 32
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType1", {{.*}}, align: 8
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType2", {{.*}}, align: 8
+
+struct MyType {
+  int m;
+} __attribute__((aligned(1)));
+struct MyType mt;
+
+struct MyType1 {
+  int m;
+} __attribute__((packed, aligned(1)));
+struct MyType1 mt1;
+
+struct MyType2 {
+  __attribute__((packed))int m;
+} __attribute__((aligned(1)));
+struct MyType2 mt2;
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3553,7 +3553,11 @@
 return getOrCreateRecordFwdDecl(Ty, RDContext);
 
   uint64_t Size = CGM.getContext().getTypeSize(Ty);
-  auto Align = getDeclAlignIfRequired(D, CGM.getContext());
+  // __attribute__((aligned)) can increase or decrease alignment *except* on a
+  // struct or struct member, where it only increases  alignment unless 'packed'
+  // is also specified. To handle this case, the `getTypeAlignIfRequired` needs
+  // to be used.
+  auto Align = getTypeAlignIfRequired(Ty, CGM.getContext());
 
   SmallString<256> Identifier = getTypeIdentifier(Ty, CGM, TheCU);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits