[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Djordje Todorovic via Phabricator via cfe-commits]

djtodoro added inline comments.



Comment at: llvm/docs/ReleaseNotes.rst:169
 
+* C DebugInfo API ``LLVMDIBuilderCreateTypedef`` is updated to include an extra
+argument ``AlignInBits``, to facilitate / propagate specified Alignment 
information

SouraVX wrote:
> djtodoro wrote:
> > djtodoro wrote:
> > > It looks like this breaks the llvm-sphinx-docs build.
> > Fixed with the 5762648.
> > 
> > Please take a look into the http://lab.llvm.org:8011/console when 
> > committing changes. There you can find info about build bots failures, if 
> > any.
> Thank You @djtodoro , had a brief look at 5762648. Seems good. 
> BTW are sphinx bots still failing due to this ?
The builder is back to passing again.


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

https://reviews.llvm.org/D70111



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

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Sourabh Singh Tomar via Phabricator via cfe-commits]

SouraVX added inline comments.



Comment at: llvm/docs/ReleaseNotes.rst:169
 
+* C DebugInfo API ``LLVMDIBuilderCreateTypedef`` is updated to include an extra
+argument ``AlignInBits``, to facilitate / propagate specified Alignment 
information

djtodoro wrote:
> djtodoro wrote:
> > It looks like this breaks the llvm-sphinx-docs build.
> Fixed with the 5762648.
> 
> Please take a look into the http://lab.llvm.org:8011/console when committing 
> changes. There you can find info about build bots failures, if any.
Thank You @djtodoro , had a brief look at 5762648. Seems good. 
BTW are sphinx bots still failing due to this ?


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

https://reviews.llvm.org/D70111



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

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Djordje Todorovic via Phabricator via cfe-commits]

djtodoro added inline comments.



Comment at: llvm/docs/ReleaseNotes.rst:169
 
+* C DebugInfo API ``LLVMDIBuilderCreateTypedef`` is updated to include an extra
+argument ``AlignInBits``, to facilitate / propagate specified Alignment 
information

djtodoro wrote:
> It looks like this breaks the llvm-sphinx-docs build.
Fixed with the 5762648.

Please take a look into the http://lab.llvm.org:8011/console when committing 
changes. There you can find info about build bots failures, if any.


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

https://reviews.llvm.org/D70111



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

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Djordje Todorovic via Phabricator via cfe-commits]

djtodoro added inline comments.



Comment at: llvm/docs/ReleaseNotes.rst:169
 
+* C DebugInfo API ``LLVMDIBuilderCreateTypedef`` is updated to include an extra
+argument ``AlignInBits``, to facilitate / propagate specified Alignment 
information

It looks like this breaks the llvm-sphinx-docs build.


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

https://reviews.llvm.org/D70111



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

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Sourabh Singh Tomar via Phabricator via cfe-commits]

SouraVX added a comment.

In D70111#1765223 , @dblaikie wrote:

> Looks OK to me - please recommit when you're ready.


Thank you @awpandey . As requested, I've recommited this.


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

https://reviews.llvm.org/D70111



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

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Looks OK to me - please recommit when you're ready.


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

https://reviews.llvm.org/D70111



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

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Awanish Pandey via Phabricator via cfe-commits]

awpandey updated this revision to Diff 231004.
awpandey added a comment.

Thanks @dblaikie. I have updated go and C bindings. I have also added the 
release note for why these APIs are changing.


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

https://reviews.llvm.org/D70111

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/debug-info-template-align.cpp
  llvm/bindings/go/llvm/dibuilder.go
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm-c/DebugInfo.h
  llvm/include/llvm/IR/DIBuilder.h
  llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
  llvm/lib/IR/DIBuilder.cpp
  llvm/lib/IR/DebugInfo.cpp
  llvm/test/DebugInfo/X86/debug-info-template-align.ll
  llvm/tools/llvm-c-test/debuginfo.c

Index: llvm/tools/llvm-c-test/debuginfo.c
===
--- llvm/tools/llvm-c-test/debuginfo.c
+++ llvm/tools/llvm-c-test/debuginfo.c
@@ -69,7 +69,7 @@
   LLVMMetadataRef Int64Ty =
   LLVMDIBuilderCreateBasicType(DIB, "Int64", 5, 64, 0, LLVMDIFlagZero);
   LLVMMetadataRef Int64TypeDef =
-LLVMDIBuilderCreateTypedef(DIB, Int64Ty, "int64_t", 7, File, 42, File);
+  LLVMDIBuilderCreateTypedef(DIB, Int64Ty, "int64_t", 7, File, 42, File, 0);
 
   LLVMMetadataRef GlobalVarValueExpr =
   LLVMDIBuilderCreateConstantValueExpression(DIB, 0);
Index: llvm/test/DebugInfo/X86/debug-info-template-align.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/X86/debug-info-template-align.ll
@@ -0,0 +1,63 @@
+; RUN: llc %s -filetype=obj -o - | llvm-dwarfdump -v - | FileCheck %s
+
+; C++ source to regenerate:
+
+;typedef char  __attribute__((__aligned__(64))) alchar;
+
+;int main(){
+;alchar newChar;
+;}
+; $ clang++ -O0 -g -gdwarf-5 debug-info-template-align.cpp -c
+
+; CHECK: .debug_abbrev contents:
+
+; CHECK: [5] DW_TAG_typedef  DW_CHILDREN_no
+; CHECK:  DW_AT_alignment DW_FORM_udata
+
+; CHECK: .debug_info contents:
+
+;CHECK: DW_TAG_typedef [5]
+;CHECK: DW_AT_name {{.*}} "alchar"
+;CHECK-NEXT: DW_AT_alignment [DW_FORM_udata] (64)
+
+
+; ModuleID = '/dir/test.cpp'
+source_filename = "/dir/test.cpp"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: noinline norecurse nounwind optnone uwtable
+define dso_local i32 @main() #0 !dbg !7 {
+entry:
+  %newChar = alloca i8, align 64
+  call void @llvm.dbg.declare(metadata i8* %newChar, metadata !12, metadata !DIExpression()), !dbg !15
+  ret i32 0, !dbg !16
+}
+
+; Function Attrs: nounwind readnone speculatable willreturn
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
+
+attributes #0 = { noinline norecurse nounwind optnone uwtable }
+attributes #1 = { nounwind readnone speculatable willreturn }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 10.0.0 ", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
+!1 = !DIFile(filename: "/dir/test.cpp", directory: "/dir/", checksumkind: CSK_MD5, checksum: "872e252efdfcb9480b4bfaf8437f58ab")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 5}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{!"clang version 10.0.0 "}
+!7 = distinct !DISubprogram(name: "main", scope: !8, file: !8, line: 12, type: !9, scopeLine: 12, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
+!8 = !DIFile(filename: "test.cpp", directory: "/dir", checksumkind: CSK_MD5, checksum: "872e252efdfcb9480b4bfaf8437f58ab")
+!9 = !DISubroutineType(types: !10)
+!10 = !{!11}
+!11 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!12 = !DILocalVariable(name: "newChar", scope: !7, file: !8, line: 13, type: !13)
+!13 = !DIDerivedType(tag: DW_TAG_typedef, name: "alchar", file: !8, line: 10, baseType: !14, align: 512)
+!14 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
+!15 = !DILocation(line: 13, column: 10, scope: !7)
+!16 = !DILocation(line: 14, column: 1, scope: !7)
Index: llvm/lib/IR/DebugInfo.cpp
===
--- llvm/lib/IR/DebugInfo.cpp
+++ llvm/lib/IR/DebugInfo.cpp
@@ -1108,11 +1108,10 @@
 LLVMDIBuilderCreateTypedef(LLVMDIBuilderRef Builder, LLVMMetadataRef Type,
const char *Name, size_t NameLen,
LLVMMetadataRef File, unsigned LineNo,
-   LLVMMetadataRef Scope) {
+   LLVMMetadataRef Scope, uint32_t AlignInBits) {
   return wrap(unwrap(Builder)->createTypedef(
-  unwrapDI(Type), {Name, NameLen},
-  unwrapDI(File), LineNo,
-  unwrapDI(Scope)));
+  unwrapDI(Type), {Name, NameLen}, unwrapDI(File), LineNo,
+  unwrapDI(Scope), 

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D70111#1754702 , @awpandey wrote:

> I am considering this
>
> - Will commit this without C bindings
> - will give separate patch for C bindings and release notes (if necessary).
> - will give another patch for Go bindings and release notes (if necessary).


I'm not sure I follow - are you asking a question? Proposing a path forward 
you're planning to take (or this patch already represents)?

I'd reiterate my previous request:

Please update the go bindings and include a release note that this API is 
changing & why. (include that in this patch review & then it can be reviewed & 
all committed together to keep the whole source tree in a working state, 
because committing the change while either removing the C API or updating it 
without updating the Go bindings would break the Go bindings, which isn't good)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70111



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

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Robert Widmann via Phabricator via cfe-commits]

CodaFi added a comment.

C binding changes LGTM.  Don’t worry about changing them, they are marked as 
experimental and subject to change at any time.




Comment at: llvm/include/llvm-c/DebugInfo.h:878
LLVMMetadataRef File, unsigned LineNo,
-   LLVMMetadataRef Scope);
+   LLVMMetadataRef Scope, uint32_t AlignInBits);
 

sammccall wrote:
> These functions in llvm-c are ABI-stable AFAIK.
> (This broke the go bindings which are in-tree, but could equally break 
> out-of-tree bindings)
The debug info bindings are not subject to the ABI stability guarantees of the 
rest of the C bindings to LLVM’s core.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70111



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

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

Sounds good to me!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70111



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

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Awanish Pandey via Phabricator via cfe-commits]

awpandey added a comment.

I am considering this

- Will commit this without C bindings
- will give separate patch for C bindings and release notes (if necessary).
- will give another patch for Go bindings and release notes (if necessary).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70111



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

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Please update the go bindings and include a release note that this API is 
changing & why.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70111



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

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

My apologies if the revert was incorrect. I asked for advice after seeing 
breakages and heard that llvm-c should be kept ABI compatible.
But it looks like that's too strong: 
https://llvm.org/docs/DeveloperPolicy.html#c-api-changes says we make a best 
effort to maintain compatibility.

So if we don't think it's worth staying compatible here feel free to roll 
forward, but please do update the go bindings since they're in-tree.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70111



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

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Sourabh Singh Tomar via Phabricator via cfe-commits]

SouraVX added a comment.

In D70111#1751981 , @aprantl wrote:

> There are two options here:
>
> - leave the C bindings as is (fine with me)
> - add an overloaded function to the C bindings that has the extra argument 
> (also fine with me).


In my opinon, we should be doing both of these. Off-course Step 1 first and 
subsequently Step 2.
Otherwise consumers using / utilizing C bindings will again try to revert this, 
If we don't do Step 2.
As without an updated binding and backward compaitibilty -- this will break 
things for C-binding users ??


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70111



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

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Sourabh Singh Tomar via Phabricator via cfe-commits]

SouraVX added a comment.

Some how, this{eric response} doesn't got it's way to phabricator, anyways FYI

> The C bindings, in general, don't have stability guarantees outside of a few 
> cases.



> Whitequark owns all of this now though :)



> -eric


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70111



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

Re: [PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Eric Christopher via cfe-commits]

The C bindings, in general, don't have stability guarantees outside of
a few cases.

Whitequark owns all of this now though :)

-eric

On Tue, Nov 19, 2019 at 1:38 PM Adrian Prantl via Phabricator
 wrote:
>
> aprantl added a subscriber: echristo.
> aprantl added a comment.
>
> In D70111#1752215 , @dblaikie wrote:
>
> > I have some vague recollection that the DWARF C bindings didn't have 
> > stability guarantees? Does that sound familiar to anyone? What sort of 
> > changes have been made to them in the past?
>
>
> That does sound familiar. I'm not sure if this was formalized anywhere or if 
> we just all agreed on it verbally. @echristo might know?
>
> In that case there is a 3rd option which is to
>
> - update the C bindings and the go bindings in one commit.
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D70111/new/
>
> https://reviews.llvm.org/D70111
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a subscriber: echristo.
aprantl added a comment.

In D70111#1752215 , @dblaikie wrote:

> I have some vague recollection that the DWARF C bindings didn't have 
> stability guarantees? Does that sound familiar to anyone? What sort of 
> changes have been made to them in the past?


That does sound familiar. I'm not sure if this was formalized anywhere or if we 
just all agreed on it verbally. @echristo might know?

In that case there is a 3rd option which is to

- update the C bindings and the go bindings in one commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70111



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

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

I have some vague recollection that the DWARF C bindings didn't have stability 
guarantees? Does that sound familiar to anyone? What sort of changes have been 
made to them in the past?




Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp:804-810
+  // attribute in DW_TAG_typedef DIE.
+  if (Tag == dwarf::DW_TAG_typedef && DD->getDwarfVersion() >= 5) {
+uint32_t AlignInBytes = DTy->getAlignInBytes();
+if (AlignInBytes > 0)
+  addUInt(Buffer, dwarf::DW_AT_alignment, dwarf::DW_FORM_udata,
+  AlignInBytes);
+  }

Should there be some general utility/appropriate point to add alignment to any 
type? (generalized over the other places we're already adding alignment on 
types (such as in DwarfUnit::constructTypeDIE(DIE , const 
DICompositeType *CTy), maybe other places) - probably doesn't generalize over 
all places that add DW_AT_alignment, like global/static variables, etc, which 
is fine)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70111



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

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

There are two options here:

- leave the C bindings as is (fine with me)
- add an overloaded function to the C bindings that has the extra argument 
(also fine with me).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70111



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

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Awanish Pandey via Phabricator via cfe-commits]

awpandey added a comment.

Hi @aprantl. Can we drop C binding for this feature ? What will be the impact 
of this.
As noted by @sammccall this does not conform / break LLVM-C ABI?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70111



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

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

I'm reverting because this breaks ABI for llvm-c (and the go bindings).
Feel free to reapply with that change removed, though. (If this option is 
required for the C API, I'm not sure what the best approach is)




Comment at: llvm/include/llvm-c/DebugInfo.h:878
LLVMMetadataRef File, unsigned LineNo,
-   LLVMMetadataRef Scope);
+   LLVMMetadataRef Scope, uint32_t AlignInBits);
 

These functions in llvm-c are ABI-stable AFAIK.
(This broke the go bindings which are in-tree, but could equally break 
out-of-tree bindings)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70111



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

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Sourabh Singh Tomar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG423f541c1a32: [DWARF5]Addition of alignment atrribute in 
typedef DIE. (authored by SouraVX).

Changed prior to commit:
  https://reviews.llvm.org/D70111?vs=229544=229697#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70111

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/debug-info-template-align.cpp
  llvm/include/llvm-c/DebugInfo.h
  llvm/include/llvm/IR/DIBuilder.h
  llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
  llvm/lib/IR/DIBuilder.cpp
  llvm/lib/IR/DebugInfo.cpp
  llvm/test/DebugInfo/X86/debug-info-template-align.ll
  llvm/tools/llvm-c-test/debuginfo.c

Index: llvm/tools/llvm-c-test/debuginfo.c
===
--- llvm/tools/llvm-c-test/debuginfo.c
+++ llvm/tools/llvm-c-test/debuginfo.c
@@ -69,7 +69,7 @@
   LLVMMetadataRef Int64Ty =
   LLVMDIBuilderCreateBasicType(DIB, "Int64", 5, 64, 0, LLVMDIFlagZero);
   LLVMMetadataRef Int64TypeDef =
-LLVMDIBuilderCreateTypedef(DIB, Int64Ty, "int64_t", 7, File, 42, File);
+  LLVMDIBuilderCreateTypedef(DIB, Int64Ty, "int64_t", 7, File, 42, File, 0);
 
   LLVMMetadataRef GlobalVarValueExpr =
   LLVMDIBuilderCreateConstantValueExpression(DIB, 0);
Index: llvm/test/DebugInfo/X86/debug-info-template-align.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/X86/debug-info-template-align.ll
@@ -0,0 +1,63 @@
+; RUN: llc %s -filetype=obj -o - | llvm-dwarfdump -v - | FileCheck %s
+
+; C++ source to regenerate:
+
+;typedef char  __attribute__((__aligned__(64))) alchar;
+
+;int main(){
+;alchar newChar;
+;}
+; $ clang++ -O0 -g -gdwarf-5 debug-info-template-align.cpp -c
+
+; CHECK: .debug_abbrev contents:
+
+; CHECK: [5] DW_TAG_typedef  DW_CHILDREN_no
+; CHECK:  DW_AT_alignment DW_FORM_udata
+
+; CHECK: .debug_info contents:
+
+;CHECK: DW_TAG_typedef [5]
+;CHECK: DW_AT_name {{.*}} "alchar"
+;CHECK-NEXT: DW_AT_alignment [DW_FORM_udata] (64)
+
+
+; ModuleID = '/dir/test.cpp'
+source_filename = "/dir/test.cpp"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: noinline norecurse nounwind optnone uwtable
+define dso_local i32 @main() #0 !dbg !7 {
+entry:
+  %newChar = alloca i8, align 64
+  call void @llvm.dbg.declare(metadata i8* %newChar, metadata !12, metadata !DIExpression()), !dbg !15
+  ret i32 0, !dbg !16
+}
+
+; Function Attrs: nounwind readnone speculatable willreturn
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
+
+attributes #0 = { noinline norecurse nounwind optnone uwtable }
+attributes #1 = { nounwind readnone speculatable willreturn }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 10.0.0 ", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
+!1 = !DIFile(filename: "/dir/test.cpp", directory: "/home/awpandey/tools/llvm/test/DebugInfo", checksumkind: CSK_MD5, checksum: "872e252efdfcb9480b4bfaf8437f58ab")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 5}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{!"clang version 10.0.0 "}
+!7 = distinct !DISubprogram(name: "main", scope: !8, file: !8, line: 12, type: !9, scopeLine: 12, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
+!8 = !DIFile(filename: "test.cpp", directory: "/dir", checksumkind: CSK_MD5, checksum: "872e252efdfcb9480b4bfaf8437f58ab")
+!9 = !DISubroutineType(types: !10)
+!10 = !{!11}
+!11 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!12 = !DILocalVariable(name: "newChar", scope: !7, file: !8, line: 13, type: !13)
+!13 = !DIDerivedType(tag: DW_TAG_typedef, name: "alchar", file: !8, line: 10, baseType: !14, align: 512)
+!14 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
+!15 = !DILocation(line: 13, column: 10, scope: !7)
+!16 = !DILocation(line: 14, column: 1, scope: !7)
Index: llvm/lib/IR/DebugInfo.cpp
===
--- llvm/lib/IR/DebugInfo.cpp
+++ llvm/lib/IR/DebugInfo.cpp
@@ -1108,11 +1108,10 @@
 LLVMDIBuilderCreateTypedef(LLVMDIBuilderRef Builder, LLVMMetadataRef Type,
const char *Name, size_t NameLen,
LLVMMetadataRef File, unsigned LineNo,
-   LLVMMetadataRef Scope) {
+   LLVMMetadataRef Scope, uint32_t AlignInBits) {
   return wrap(unwrap(Builder)->createTypedef(
-  unwrapDI(Type), {Name, NameLen},
-  unwrapDI(File), LineNo,
-  unwrapDI(Scope)));
+  

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Sourabh Singh Tomar via Phabricator via cfe-commits]

SouraVX added a comment.

In D70111#1748765 , @awpandey wrote:

> Hi @aprantl. I had made the changes and my team member will commit this.


Thanks  @awpandey for working on this! as requested, I've committed / closed 
this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70111



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

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Awanish Pandey via Phabricator via cfe-commits]

awpandey added a comment.

Hi @aprantl. I had made the changes and my team member will commit this.


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

https://reviews.llvm.org/D70111



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

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Adrian Prantl via Phabricator via cfe-commits]

aprantl accepted this revision.
aprantl added a comment.
This revision is now accepted and ready to land.

LGTM with one change to DIBuilder inline.




Comment at: llvm/include/llvm/IR/DIBuilder.h:243
+ unsigned LineNo, DIScope *Context,
+ Optional AlignInBits);
 

This can still be `Optional AlignInBits = {}` to avoid churn.


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

https://reviews.llvm.org/D70111



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

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Awanish Pandey via Phabricator via cfe-commits]

awpandey updated this revision to Diff 229544.
awpandey added a comment.

Thanks for your suggestion @aprantl. I have revised the data type based on your 
suggestion. Please inform me if any other changes are required.


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

https://reviews.llvm.org/D70111

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/debug-info-template-align.cpp
  llvm/include/llvm-c/DebugInfo.h
  llvm/include/llvm/IR/DIBuilder.h
  llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
  llvm/lib/IR/DIBuilder.cpp
  llvm/lib/IR/DebugInfo.cpp
  llvm/test/DebugInfo/X86/debug-info-template-align.ll
  llvm/tools/llvm-c-test/debuginfo.c

Index: llvm/tools/llvm-c-test/debuginfo.c
===
--- llvm/tools/llvm-c-test/debuginfo.c
+++ llvm/tools/llvm-c-test/debuginfo.c
@@ -69,7 +69,7 @@
   LLVMMetadataRef Int64Ty =
   LLVMDIBuilderCreateBasicType(DIB, "Int64", 5, 64, 0, LLVMDIFlagZero);
   LLVMMetadataRef Int64TypeDef =
-LLVMDIBuilderCreateTypedef(DIB, Int64Ty, "int64_t", 7, File, 42, File);
+  LLVMDIBuilderCreateTypedef(DIB, Int64Ty, "int64_t", 7, File, 42, File, 0);
 
   LLVMMetadataRef GlobalVarValueExpr =
   LLVMDIBuilderCreateConstantValueExpression(DIB, 0);
Index: llvm/test/DebugInfo/X86/debug-info-template-align.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/X86/debug-info-template-align.ll
@@ -0,0 +1,63 @@
+; RUN: llc %s -filetype=obj -o - | llvm-dwarfdump -v - | FileCheck %s
+
+; C++ source to regenerate:
+
+;typedef char  __attribute__((__aligned__(64))) alchar;
+
+;int main(){
+;alchar newChar;
+;}
+; $ clang++ -O0 -g -gdwarf-5 debug-info-template-align.cpp -c
+
+; CHECK: .debug_abbrev contents:
+
+; CHECK: [5] DW_TAG_typedef  DW_CHILDREN_no
+; CHECK:  DW_AT_alignment DW_FORM_udata
+
+; CHECK: .debug_info contents:
+
+;CHECK: DW_TAG_typedef [5]
+;CHECK: DW_AT_name {{.*}} "alchar"
+;CHECK-NEXT: DW_AT_alignment [DW_FORM_udata] (64)
+
+
+; ModuleID = '/dir/test.cpp'
+source_filename = "/dir/test.cpp"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: noinline norecurse nounwind optnone uwtable
+define dso_local i32 @main() #0 !dbg !7 {
+entry:
+  %newChar = alloca i8, align 64
+  call void @llvm.dbg.declare(metadata i8* %newChar, metadata !12, metadata !DIExpression()), !dbg !15
+  ret i32 0, !dbg !16
+}
+
+; Function Attrs: nounwind readnone speculatable willreturn
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
+
+attributes #0 = { noinline norecurse nounwind optnone uwtable }
+attributes #1 = { nounwind readnone speculatable willreturn }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 10.0.0 ", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
+!1 = !DIFile(filename: "/dir/test.cpp", directory: "/home/awpandey/tools/llvm/test/DebugInfo", checksumkind: CSK_MD5, checksum: "872e252efdfcb9480b4bfaf8437f58ab")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 5}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{!"clang version 10.0.0 "}
+!7 = distinct !DISubprogram(name: "main", scope: !8, file: !8, line: 12, type: !9, scopeLine: 12, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
+!8 = !DIFile(filename: "test.cpp", directory: "/dir", checksumkind: CSK_MD5, checksum: "872e252efdfcb9480b4bfaf8437f58ab")
+!9 = !DISubroutineType(types: !10)
+!10 = !{!11}
+!11 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!12 = !DILocalVariable(name: "newChar", scope: !7, file: !8, line: 13, type: !13)
+!13 = !DIDerivedType(tag: DW_TAG_typedef, name: "alchar", file: !8, line: 10, baseType: !14, align: 512)
+!14 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
+!15 = !DILocation(line: 13, column: 10, scope: !7)
+!16 = !DILocation(line: 14, column: 1, scope: !7)
Index: llvm/lib/IR/DebugInfo.cpp
===
--- llvm/lib/IR/DebugInfo.cpp
+++ llvm/lib/IR/DebugInfo.cpp
@@ -1108,11 +1108,10 @@
 LLVMDIBuilderCreateTypedef(LLVMDIBuilderRef Builder, LLVMMetadataRef Type,
const char *Name, size_t NameLen,
LLVMMetadataRef File, unsigned LineNo,
-   LLVMMetadataRef Scope) {
+   LLVMMetadataRef Scope, uint32_t AlignInBits) {
   return wrap(unwrap(Builder)->createTypedef(
-  unwrapDI(Type), {Name, NameLen},
-  unwrapDI(File), LineNo,
-  unwrapDI(Scope)));
+  unwrapDI(Type), {Name, NameLen}, unwrapDI(File), LineNo,
+  unwrapDI(Scope), AlignInBits));
 }

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: llvm/include/llvm/IR/DIBuilder.h:243
+ unsigned LineNo, DIScope *Context,
+ uint32_t AlignInBits = 0);
 

awpandey wrote:
> aprantl wrote:
> > This should be `Optional` AlignInBits. Even better perhaps 
> > `llvm::Align` but perhaps that's too restrictive.
> Yes @aprantl  this should be of `Optional`  type but changing this 
> will require change in the `DIDerivedType::get` macro and this macro is used 
> by many other functions. Please correct me if I am wrong. ??
> 
> Current functionality of the `createTypeDef` is like the default value of the 
> `alignInBits`  will be `0`. Consider below comment in `DIBuilder.cpp`.
Updating `DIDerivedType::get` to use Optional would also be a good change, but 
for this patch I'd be happy if at least the DIBuilder interface is using 
optional and then treats None as `0` in the implemenation. We can optionally do 
another patch to change the interface of `DIDerivedType::get`


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

https://reviews.llvm.org/D70111



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

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Awanish Pandey via Phabricator via cfe-commits]

awpandey added inline comments.



Comment at: clang/test/CodeGenCXX/debug-info-template-align.cpp:8
+
+// CHECK: DIDerivedType(tag: DW_TAG_typedef, {{.*}}, align:
+

aprantl wrote:
> Do we need to hardcode the target here? Could we check for the specific align 
> value?
Sorry, I could not get your first comment

> Do we need to hardcode the target here?

I have incorporated your rest of the suggestions.





Comment at: llvm/include/llvm/IR/DIBuilder.h:243
+ unsigned LineNo, DIScope *Context,
+ uint32_t AlignInBits = 0);
 

aprantl wrote:
> This should be `Optional` AlignInBits. Even better perhaps 
> `llvm::Align` but perhaps that's too restrictive.
Yes @aprantl  this should be of `Optional`  type but changing this 
will require change in the `DIDerivedType::get` macro and this macro is used by 
many other functions. Please correct me if I am wrong. ??

Current functionality of the `createTypeDef` is like the default value of the 
`alignInBits`  will be `0`. Consider below comment in `DIBuilder.cpp`.



Comment at: llvm/lib/IR/DIBuilder.cpp:309
-DIScope *Context) {
+DIScope *Context,
+uint32_t AlignInBits) {
   return DIDerivedType::get(VMContext, dwarf::DW_TAG_typedef, Name, File,
-LineNo, getNonCompileUnitScope(Context), Ty, 0, 0,

Consider the 9th argument here 
```
return DIDerivedType::get(VMContext, dwarf::DW_TAG_typedef, Name, File,LineNo, 
getNonCompileUnitScope(Context), Ty, 0, 0, --Hard coded alignment for the 
typedef field in existing API.
0, None, DINode::FlagZero);
}
```
 That is why rather than changing the entire API by using `Option` I 
used `uint_32`


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

https://reviews.llvm.org/D70111



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

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Awanish Pandey via Phabricator via cfe-commits]

awpandey updated this revision to Diff 229013.
awpandey marked 3 inline comments as done.
awpandey added a comment.
Herald added a reviewer: deadalnix.

@aprantl  Thanks for the suggestion. Based on your suggestion I have done the 
following changes

1. I have added alignment value as a check in the test case
2. I have added C-Binding changes.


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

https://reviews.llvm.org/D70111

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/debug-info-template-align.cpp
  llvm/include/llvm-c/DebugInfo.h
  llvm/include/llvm/IR/DIBuilder.h
  llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
  llvm/lib/IR/DIBuilder.cpp
  llvm/lib/IR/DebugInfo.cpp
  llvm/test/DebugInfo/X86/debug-info-template-align.ll
  llvm/tools/llvm-c-test/debuginfo.c

Index: llvm/tools/llvm-c-test/debuginfo.c
===
--- llvm/tools/llvm-c-test/debuginfo.c
+++ llvm/tools/llvm-c-test/debuginfo.c
@@ -69,7 +69,7 @@
   LLVMMetadataRef Int64Ty =
   LLVMDIBuilderCreateBasicType(DIB, "Int64", 5, 64, 0, LLVMDIFlagZero);
   LLVMMetadataRef Int64TypeDef =
-LLVMDIBuilderCreateTypedef(DIB, Int64Ty, "int64_t", 7, File, 42, File);
+  LLVMDIBuilderCreateTypedef(DIB, Int64Ty, "int64_t", 7, File, 42, File, 0);
 
   LLVMMetadataRef GlobalVarValueExpr =
   LLVMDIBuilderCreateConstantValueExpression(DIB, 0);
Index: llvm/test/DebugInfo/X86/debug-info-template-align.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/X86/debug-info-template-align.ll
@@ -0,0 +1,63 @@
+; RUN: llc %s -filetype=obj -o - | llvm-dwarfdump -v - | FileCheck %s
+
+; C++ source to regenerate:
+
+;typedef char  __attribute__((__aligned__(64))) alchar;
+
+;int main(){
+;alchar newChar;
+;}
+; $ clang++ -O0 -g -gdwarf-5 debug-info-template-align.cpp -c
+
+; CHECK: .debug_abbrev contents:
+
+; CHECK: [5] DW_TAG_typedef  DW_CHILDREN_no
+; CHECK:  DW_AT_alignment DW_FORM_udata
+
+; CHECK: .debug_info contents:
+
+;CHECK: DW_TAG_typedef [5]
+;CHECK: DW_AT_name {{.*}} "alchar"
+;CHECK-NEXT: DW_AT_alignment [DW_FORM_udata] (64)
+
+
+; ModuleID = '/dir/test.cpp'
+source_filename = "/dir/test.cpp"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: noinline norecurse nounwind optnone uwtable
+define dso_local i32 @main() #0 !dbg !7 {
+entry:
+  %newChar = alloca i8, align 64
+  call void @llvm.dbg.declare(metadata i8* %newChar, metadata !12, metadata !DIExpression()), !dbg !15
+  ret i32 0, !dbg !16
+}
+
+; Function Attrs: nounwind readnone speculatable willreturn
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
+
+attributes #0 = { noinline norecurse nounwind optnone uwtable }
+attributes #1 = { nounwind readnone speculatable willreturn }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 10.0.0 ", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
+!1 = !DIFile(filename: "/dir/test.cpp", directory: "/home/awpandey/tools/llvm/test/DebugInfo", checksumkind: CSK_MD5, checksum: "872e252efdfcb9480b4bfaf8437f58ab")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 5}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{!"clang version 10.0.0 "}
+!7 = distinct !DISubprogram(name: "main", scope: !8, file: !8, line: 12, type: !9, scopeLine: 12, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
+!8 = !DIFile(filename: "test.cpp", directory: "/dir", checksumkind: CSK_MD5, checksum: "872e252efdfcb9480b4bfaf8437f58ab")
+!9 = !DISubroutineType(types: !10)
+!10 = !{!11}
+!11 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!12 = !DILocalVariable(name: "newChar", scope: !7, file: !8, line: 13, type: !13)
+!13 = !DIDerivedType(tag: DW_TAG_typedef, name: "alchar", file: !8, line: 10, baseType: !14, align: 512)
+!14 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
+!15 = !DILocation(line: 13, column: 10, scope: !7)
+!16 = !DILocation(line: 14, column: 1, scope: !7)
Index: llvm/lib/IR/DebugInfo.cpp
===
--- llvm/lib/IR/DebugInfo.cpp
+++ llvm/lib/IR/DebugInfo.cpp
@@ -1108,11 +1108,10 @@
 LLVMDIBuilderCreateTypedef(LLVMDIBuilderRef Builder, LLVMMetadataRef Type,
const char *Name, size_t NameLen,
LLVMMetadataRef File, unsigned LineNo,
-   LLVMMetadataRef Scope) {
+   LLVMMetadataRef Scope, uint32_t AlignInBits) {
   return wrap(unwrap(Builder)->createTypedef(
-  unwrapDI(Type), {Name, NameLen},
-  unwrapDI(File), LineNo,
-

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: clang/test/CodeGenCXX/debug-info-template-align.cpp:8
+
+// CHECK: DIDerivedType(tag: DW_TAG_typedef, {{.*}}, align:
+

Do we need to hardcode the target here? Could we check for the specific align 
value?



Comment at: llvm/include/llvm/IR/DIBuilder.h:243
+ unsigned LineNo, DIScope *Context,
+ uint32_t AlignInBits = 0);
 

This should be `Optional` AlignInBits. Even better perhaps 
`llvm::Align` but perhaps that's too restrictive.



Comment at: llvm/include/llvm/IR/DIBuilder.h:244
+ uint32_t AlignInBits = 0);
 
 /// Create debugging information entry for a 'friend'.

Do you need to update the C bindings as well?



Comment at: llvm/test/DebugInfo/X86/debug-info-template-align.ll:1
+; RUN: llc < %s -filetype=obj -o %t
+; RUN: llvm-dwarfdump -v %t | FileCheck %s

`RUN: llc %s -filetype=obj -o - | llvm-dwarfdump -v - | FileCheck %s`


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

https://reviews.llvm.org/D70111



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

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Awanish Pandey via Phabricator via cfe-commits]

awpandey marked 8 inline comments as done.
awpandey added a comment.

Thank you @djtodoro  for reviewing this.


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

https://reviews.llvm.org/D70111



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

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Awanish Pandey via Phabricator via cfe-commits]

awpandey updated this revision to Diff 228852.
awpandey added a comment.

Revised and address comments of @djtodoro


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

https://reviews.llvm.org/D70111

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/debug-info-template-align.cpp
  llvm/include/llvm/IR/DIBuilder.h
  llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
  llvm/lib/IR/DIBuilder.cpp
  llvm/test/DebugInfo/X86/debug-info-template-align.ll

Index: llvm/test/DebugInfo/X86/debug-info-template-align.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/X86/debug-info-template-align.ll
@@ -0,0 +1,64 @@
+; RUN: llc < %s -filetype=obj -o %t
+; RUN: llvm-dwarfdump -v %t | FileCheck %s
+
+; C++ source to regenerate:
+
+;typedef char  __attribute__((__aligned__(64))) alchar;
+
+;int main(){
+;alchar newChar;
+;}
+; $ clang++ -O0 -g -gdwarf-5 debug-info-template-align.cpp -c
+
+; CHECK: .debug_abbrev contents:
+
+; CHECK: [5] DW_TAG_typedef  DW_CHILDREN_no
+; CHECK:  DW_AT_alignment DW_FORM_udata
+
+; CHECK: .debug_info contents:
+
+;CHECK: DW_TAG_typedef [5]
+;CHECK: DW_AT_name {{.*}} "alchar"
+;CHECK-NEXT: DW_AT_alignment [DW_FORM_udata] (64)
+
+
+; ModuleID = '/dir/test.cpp'
+source_filename = "/dir/test.cpp"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: noinline norecurse nounwind optnone uwtable
+define dso_local i32 @main() #0 !dbg !7 {
+entry:
+  %newChar = alloca i8, align 64
+  call void @llvm.dbg.declare(metadata i8* %newChar, metadata !12, metadata !DIExpression()), !dbg !15
+  ret i32 0, !dbg !16
+}
+
+; Function Attrs: nounwind readnone speculatable willreturn
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
+
+attributes #0 = { noinline norecurse nounwind optnone uwtable }
+attributes #1 = { nounwind readnone speculatable willreturn }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 10.0.0 ", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
+!1 = !DIFile(filename: "/dir/test.cpp", directory: "/home/awpandey/tools/llvm/test/DebugInfo", checksumkind: CSK_MD5, checksum: "872e252efdfcb9480b4bfaf8437f58ab")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 5}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{!"clang version 10.0.0 "}
+!7 = distinct !DISubprogram(name: "main", scope: !8, file: !8, line: 12, type: !9, scopeLine: 12, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
+!8 = !DIFile(filename: "test.cpp", directory: "/dir", checksumkind: CSK_MD5, checksum: "872e252efdfcb9480b4bfaf8437f58ab")
+!9 = !DISubroutineType(types: !10)
+!10 = !{!11}
+!11 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!12 = !DILocalVariable(name: "newChar", scope: !7, file: !8, line: 13, type: !13)
+!13 = !DIDerivedType(tag: DW_TAG_typedef, name: "alchar", file: !8, line: 10, baseType: !14, align: 512)
+!14 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
+!15 = !DILocation(line: 13, column: 10, scope: !7)
+!16 = !DILocation(line: 14, column: 1, scope: !7)
Index: llvm/lib/IR/DIBuilder.cpp
===
--- llvm/lib/IR/DIBuilder.cpp
+++ llvm/lib/IR/DIBuilder.cpp
@@ -305,10 +305,11 @@
 
 DIDerivedType *DIBuilder::createTypedef(DIType *Ty, StringRef Name,
 DIFile *File, unsigned LineNo,
-DIScope *Context) {
+DIScope *Context,
+uint32_t AlignInBits) {
   return DIDerivedType::get(VMContext, dwarf::DW_TAG_typedef, Name, File,
-LineNo, getNonCompileUnitScope(Context), Ty, 0, 0,
-0, None, DINode::FlagZero);
+LineNo, getNonCompileUnitScope(Context), Ty, 0,
+AlignInBits, 0, None, DINode::FlagZero);
 }
 
 DIDerivedType *DIBuilder::createFriend(DIType *Ty, DIType *FriendTy) {
Index: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
@@ -800,6 +800,15 @@
   if (!Name.empty())
 addString(Buffer, dwarf::DW_AT_name, Name);
 
+  // If alignment is specified for a typedef , create and insert DW_AT_alignment
+  // attribute in DW_TAG_typedef DIE.
+  if (Tag == dwarf::DW_TAG_typedef && DD->getDwarfVersion() >= 5) {
+uint32_t AlignInBytes = DTy->getAlignInBytes();
+if (AlignInBytes > 0)
+  addUInt(Buffer, dwarf::DW_AT_alignment, dwarf::DW_FORM_udata,
+

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Djordje Todorovic via Phabricator via cfe-commits]

djtodoro added a comment.

@awpandey Thanks for the patch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70111



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

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Djordje Todorovic via Phabricator via cfe-commits]

djtodoro added inline comments.



Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp:803
 
+  if (Tag == dwarf::DW_TAG_typedef && DD->getDwarfVersion() >= 5) {
+uint32_t AlignInBytes = DTy->getAlignInBytes();

Please add a comment here.



Comment at: llvm/test/DebugInfo/X86/debug-info-template-align.ll:26
+; ModuleID = 
'/home/awpandey/tools/llvm/tools/clang/test/CodeGenCXX/debug-info-template-align.cpp'
+source_filename = 
"/home/awpandey/tools/llvm/tools/clang/test/CodeGenCXX/debug-info-template-align.cpp"
+target datalayout = 
"e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"

I guess you don't want to share your local path. It is enough to write 
'test.cpp' only.



Comment at: llvm/test/DebugInfo/X86/debug-info-template-align.ll:39
+; Function Attrs: nounwind readnone speculatable willreturn
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
+

So, we can get rid of the `#N`.



Comment at: llvm/test/DebugInfo/X86/debug-info-template-align.ll:41
+
+attributes #0 = { noinline norecurse nounwind optnone uwtable 
"correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" 
"frame-pointer"="all" "less-precise-fpmad"="false" "min-legal-vector-width"="0" 
"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-cpu"="x86-64" 
"target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" 
"use-soft-float"="false" }
+attributes #1 = { nounwind readnone speculatable willreturn }

Usually, you don't need the attributes.



Comment at: llvm/test/DebugInfo/X86/debug-info-template-align.ll:48
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, 
producer: "clang version 10.0.0 
(ssh://awpan...@idcvgits02.amd.com:29418/perfcompiler/er/llvm/cpu/clang 
8b3533f5560dd219a0e6d346be2190623822bada) (llvm/cpu/llvm 
61a342a7f3320d5517ba0bd012edc7cc908b36b7)", isOptimized: false, runtimeVersion: 
0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
+!1 = !DIFile(filename: 
"/home/awpandey/tools/llvm/tools/clang/test/CodeGenCXX/debug-info-template-align.cpp",
 directory: "/home/awpandey/tools/llvm/test/DebugInfo", checksumkind: CSK_MD5, 
checksum: "872e252efdfcb9480b4bfaf8437f58ab")

`producer: "clang version 10.0.0"` is enough.



Comment at: llvm/test/DebugInfo/X86/debug-info-template-align.ll:49
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, 
producer: "clang version 10.0.0 
(ssh://awpan...@idcvgits02.amd.com:29418/perfcompiler/er/llvm/cpu/clang 
8b3533f5560dd219a0e6d346be2190623822bada) (llvm/cpu/llvm 
61a342a7f3320d5517ba0bd012edc7cc908b36b7)", isOptimized: false, runtimeVersion: 
0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
+!1 = !DIFile(filename: 
"/home/awpandey/tools/llvm/tools/clang/test/CodeGenCXX/debug-info-template-align.cpp",
 directory: "/home/awpandey/tools/llvm/test/DebugInfo", checksumkind: CSK_MD5, 
checksum: "872e252efdfcb9480b4bfaf8437f58ab")
+!2 = !{}

Usually, we put an artificial name for the dir, e.g. `/dir`.



Comment at: llvm/test/DebugInfo/X86/debug-info-template-align.ll:54
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{!"clang version 10.0.0 
(ssh://awpan...@idcvgits02.amd.com:29418/perfcompiler/er/llvm/cpu/clang 
8b3533f5560dd219a0e6d346be2190623822bada) (llvm/cpu/llvm 
61a342a7f3320d5517ba0bd012edc7cc908b36b7)"}
+!7 = distinct !DISubprogram(name: "main", scope: !8, file: !8, line: 12, type: 
!9, scopeLine: 12, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: 
!0, retainedNodes: !2)

` "clang version 10.0.0"` is enough.



Comment at: llvm/test/DebugInfo/X86/debug-info-template-align.ll:56
+!7 = distinct !DISubprogram(name: "main", scope: !8, file: !8, line: 12, type: 
!9, scopeLine: 12, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: 
!0, retainedNodes: !2)
+!8 = !DIFile(filename: 
"tools/clang/test/CodeGenCXX/debug-info-template-align.cpp", directory: 
"/home/awpandey/tools/llvm", checksumkind: CSK_MD5, checksum: 
"872e252efdfcb9480b4bfaf8437f58ab")
+!9 = !DISubroutineType(types: !10)

`filename: test.c, directory: "/dir"`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70111



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

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Awanish Pandey via Phabricator via cfe-commits]

awpandey created this revision.
awpandey added reviewers: aprantl, dblaikie, jini.susan.george, SouraVX, alok.
awpandey added a project: debug-info.
Herald added subscribers: llvm-commits, cfe-commits, ormris, hiraditya.
Herald added projects: clang, LLVM.

This patch, adds support for DW_AT_alignment[DWARF5], to be emitted with 
typdef. When explicit alignment is specified.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70111

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/debug-info-template-align.cpp
  llvm/include/llvm/IR/DIBuilder.h
  llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
  llvm/lib/IR/DIBuilder.cpp
  llvm/test/DebugInfo/X86/debug-info-template-align.ll

Index: llvm/test/DebugInfo/X86/debug-info-template-align.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/X86/debug-info-template-align.ll
@@ -0,0 +1,64 @@
+; RUN: llc < %s -filetype=obj -o %t
+; RUN: llvm-dwarfdump -v %t | FileCheck %s
+
+; C++ source to regenerate:
+
+;typedef char  __attribute__((__aligned__(64))) alchar;
+
+;int main(){
+;alchar newChar;
+;}
+; $ clang++ -O0 -g -gdwarf-5 debug-info-template-align.cpp -c
+
+; CHECK: .debug_abbrev contents:
+
+; CHECK: [5] DW_TAG_typedef  DW_CHILDREN_no
+; CHECK:  DW_AT_alignment DW_FORM_udata
+
+; CHECK: .debug_info contents:
+
+;CHECK: DW_TAG_typedef [5]
+;CHECK: DW_AT_name {{.*}} "alchar"
+;CHECK-NEXT: DW_AT_alignment [DW_FORM_udata] (64)
+
+
+; ModuleID = '/home/awpandey/tools/llvm/tools/clang/test/CodeGenCXX/debug-info-template-align.cpp'
+source_filename = "/home/awpandey/tools/llvm/tools/clang/test/CodeGenCXX/debug-info-template-align.cpp"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: noinline norecurse nounwind optnone uwtable
+define dso_local i32 @main() #0 !dbg !7 {
+entry:
+  %newChar = alloca i8, align 64
+  call void @llvm.dbg.declare(metadata i8* %newChar, metadata !12, metadata !DIExpression()), !dbg !15
+  ret i32 0, !dbg !16
+}
+
+; Function Attrs: nounwind readnone speculatable willreturn
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
+
+attributes #0 = { noinline norecurse nounwind optnone uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "frame-pointer"="all" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "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-cpu"="x86-64" "target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { nounwind readnone speculatable willreturn }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 10.0.0 (ssh://awpan...@idcvgits02.amd.com:29418/perfcompiler/er/llvm/cpu/clang 8b3533f5560dd219a0e6d346be2190623822bada) (llvm/cpu/llvm 61a342a7f3320d5517ba0bd012edc7cc908b36b7)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
+!1 = !DIFile(filename: "/home/awpandey/tools/llvm/tools/clang/test/CodeGenCXX/debug-info-template-align.cpp", directory: "/home/awpandey/tools/llvm/test/DebugInfo", checksumkind: CSK_MD5, checksum: "872e252efdfcb9480b4bfaf8437f58ab")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 5}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{!"clang version 10.0.0 (ssh://awpan...@idcvgits02.amd.com:29418/perfcompiler/er/llvm/cpu/clang 8b3533f5560dd219a0e6d346be2190623822bada) (llvm/cpu/llvm 61a342a7f3320d5517ba0bd012edc7cc908b36b7)"}
+!7 = distinct !DISubprogram(name: "main", scope: !8, file: !8, line: 12, type: !9, scopeLine: 12, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
+!8 = !DIFile(filename: "tools/clang/test/CodeGenCXX/debug-info-template-align.cpp", directory: "/home/awpandey/tools/llvm", checksumkind: CSK_MD5, checksum: "872e252efdfcb9480b4bfaf8437f58ab")
+!9 = !DISubroutineType(types: !10)
+!10 = !{!11}
+!11 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!12 = !DILocalVariable(name: "newChar", scope: !7, file: !8, line: 13, type: !13)
+!13 = !DIDerivedType(tag: DW_TAG_typedef, name: "alchar", file: !8, line: 10, baseType: !14, align: 512)
+!14 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
+!15 = !DILocation(line: 13, column: 10, scope: !7)
+!16 = !DILocation(line: 14, column: 1, scope: !7)
Index: llvm/lib/IR/DIBuilder.cpp
===
--- llvm/lib/IR/DIBuilder.cpp
+++ llvm/lib/IR/DIBuilder.cpp
@@ -305,10 +305,11 @@
 
 DIDerivedType *DIBuilder::createTypedef(DIType *Ty, StringRef Name,