[PATCH] D71451: Support to emit extern variables debuginfo with "-fstandalone-debug"

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

If this feature treats a declaration in a header different from one in a 
primary source file - I think that's problematic from a debug info/debugging 
perspective (& perhaps a quirk of how this functionality was originally 
implemented for the kernel feature thingy (the name I forget right now)... one 
I didn't notice & should probably go check up on to see if it makes sense 
there/how to rationalize it). I think it'd be surprising to users if moving a 
declaration into/out of a source file/header changed what they could do in a 
debugger and/or significantly increased debug info size, etc.


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

https://reviews.llvm.org/D71451



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

[PATCH] D71451: Support to emit extern variables debuginfo with "-fstandalone-debug"

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

If I'm reading the data correctly this means a < 1% increase in .debug_info 
size? If this is correct, I'm fine with the change.


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

https://reviews.llvm.org/D71451



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

[PATCH] D71451: Support to emit extern variables debuginfo with "-fstandalone-debug"

[Jaydeep Chauhan via Phabricator via cfe-commits]

Jac1494 added a comment.

ping


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

https://reviews.llvm.org/D71451



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

[PATCH] D71451: Support to emit extern variables debuginfo with "-fstandalone-debug"

[Jaydeep Chauhan via Phabricator via cfe-commits]

Jac1494 added a comment.

In D71451#1817334 , @aprantl wrote:

> In D71451#1816269 , @Jac1494 wrote:
>
> > ping
>
>
> I'd be curious to the answer to David's questions. If the size increase is 
> because of unused extern variables coming in from libc or something then it 
> doesn't seem worth the cost.


@aprantl now size of clang is same with this change (D71451 
 and D71599 ).


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

https://reviews.llvm.org/D71451



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

[PATCH] D71451: Support to emit extern variables debuginfo with "-fstandalone-debug"

[Jaydeep Chauhan via Phabricator via cfe-commits]

Jac1494 marked an inline comment as done.
Jac1494 added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4598
+  if (!(DebugKind == clang::codegenoptions::FullDebugInfo))
+return;
 

aprantl wrote:
> nit: 
> 
> ```
> if (DebugKind < clang::codegenoptions::FullDebugInfo)
>   return
> 
> ```
@aprantl thanks for suggestion , change is updated.


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

https://reviews.llvm.org/D71451



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

[PATCH] D71451: Support to emit extern variables debuginfo with "-fstandalone-debug"

[Jaydeep Chauhan via Phabricator via cfe-commits]

Jac1494 updated this revision to Diff 239115.

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

https://reviews.llvm.org/D71451

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGen/debug-info-extern-variable-basic.c
  clang/test/CodeGen/debug-info-extern-variable-unused.c


Index: clang/test/CodeGen/debug-info-extern-variable-unused.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-extern-variable-unused.c
@@ -0,0 +1,10 @@
+// RUN: %clang -emit-llvm -S -g -fstandalone-debug %s -o - | FileCheck %s
+
+extern int i;
+int foo() {
+  extern int j;
+  return 0;
+}
+
+// CHECK-NOT: distinct !DIGlobalVariable(name: "i"
+// CHECK-NOT: distinct !DIGlobalVariable(name: "j"
Index: clang/test/CodeGen/debug-info-extern-variable-basic.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-extern-variable-basic.c
@@ -0,0 +1,10 @@
+// RUN: %clang -emit-llvm -S -g -fstandalone-debug %s -o - | FileCheck %s
+
+extern int i;
+int foo() {
+  extern int j;
+  return i+j;
+}
+
+// CHECK: distinct !DIGlobalVariable(name: "i"
+// CHECK: distinct !DIGlobalVariable(name: "j"
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -4619,6 +4619,8 @@
   assert(CGM.getCodeGenOpts().hasReducedDebugInfo());
   if (D->hasAttr())
 return;
+  if (DebugKind < clang::codegenoptions::FullDebugInfo)
+return;
 
   auto Align = getDeclAlignIfRequired(D, CGM.getContext());
   llvm::DIFile *Unit = getOrCreateFile(D->getLocation());
Index: clang/include/clang/Basic/TargetInfo.h
===
--- clang/include/clang/Basic/TargetInfo.h
+++ clang/include/clang/Basic/TargetInfo.h
@@ -1388,7 +1388,7 @@
   virtual void setAuxTarget(const TargetInfo *Aux) {}
 
   /// Whether target allows debuginfo types for decl only variables.
-  virtual bool allowDebugInfoForExternalVar() const { return false; }
+  virtual bool allowDebugInfoForExternalVar() const { return true; }
 
 protected:
   /// Copy type and layout related info.


Index: clang/test/CodeGen/debug-info-extern-variable-unused.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-extern-variable-unused.c
@@ -0,0 +1,10 @@
+// RUN: %clang -emit-llvm -S -g -fstandalone-debug %s -o - | FileCheck %s
+
+extern int i;
+int foo() {
+  extern int j;
+  return 0;
+}
+
+// CHECK-NOT: distinct !DIGlobalVariable(name: "i"
+// CHECK-NOT: distinct !DIGlobalVariable(name: "j"
Index: clang/test/CodeGen/debug-info-extern-variable-basic.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-extern-variable-basic.c
@@ -0,0 +1,10 @@
+// RUN: %clang -emit-llvm -S -g -fstandalone-debug %s -o - | FileCheck %s
+
+extern int i;
+int foo() {
+  extern int j;
+  return i+j;
+}
+
+// CHECK: distinct !DIGlobalVariable(name: "i"
+// CHECK: distinct !DIGlobalVariable(name: "j"
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -4619,6 +4619,8 @@
   assert(CGM.getCodeGenOpts().hasReducedDebugInfo());
   if (D->hasAttr())
 return;
+  if (DebugKind < clang::codegenoptions::FullDebugInfo)
+return;
 
   auto Align = getDeclAlignIfRequired(D, CGM.getContext());
   llvm::DIFile *Unit = getOrCreateFile(D->getLocation());
Index: clang/include/clang/Basic/TargetInfo.h
===
--- clang/include/clang/Basic/TargetInfo.h
+++ clang/include/clang/Basic/TargetInfo.h
@@ -1388,7 +1388,7 @@
   virtual void setAuxTarget(const TargetInfo *Aux) {}
 
   /// Whether target allows debuginfo types for decl only variables.
-  virtual bool allowDebugInfoForExternalVar() const { return false; }
+  virtual bool allowDebugInfoForExternalVar() const { return true; }
 
 protected:
   /// Copy type and layout related info.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71451: Support to emit extern variables debuginfo with "-fstandalone-debug"

[Jaydeep Chauhan via Phabricator via cfe-commits]

Jac1494 updated this revision to Diff 239089.

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

https://reviews.llvm.org/D71451

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/CodeGen/CGDebugInfo.cpp


Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -4619,6 +4619,8 @@
   assert(CGM.getCodeGenOpts().hasReducedDebugInfo());
   if (D->hasAttr())
 return;
+  if (DebugKind < clang::codegenoptions::FullDebugInfo)
+return;
 
   auto Align = getDeclAlignIfRequired(D, CGM.getContext());
   llvm::DIFile *Unit = getOrCreateFile(D->getLocation());
Index: clang/include/clang/Basic/TargetInfo.h
===
--- clang/include/clang/Basic/TargetInfo.h
+++ clang/include/clang/Basic/TargetInfo.h
@@ -1388,7 +1388,7 @@
   virtual void setAuxTarget(const TargetInfo *Aux) {}
 
   /// Whether target allows debuginfo types for decl only variables.
-  virtual bool allowDebugInfoForExternalVar() const { return false; }
+  virtual bool allowDebugInfoForExternalVar() const { return true; }
 
 protected:
   /// Copy type and layout related info.


Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -4619,6 +4619,8 @@
   assert(CGM.getCodeGenOpts().hasReducedDebugInfo());
   if (D->hasAttr())
 return;
+  if (DebugKind < clang::codegenoptions::FullDebugInfo)
+return;
 
   auto Align = getDeclAlignIfRequired(D, CGM.getContext());
   llvm::DIFile *Unit = getOrCreateFile(D->getLocation());
Index: clang/include/clang/Basic/TargetInfo.h
===
--- clang/include/clang/Basic/TargetInfo.h
+++ clang/include/clang/Basic/TargetInfo.h
@@ -1388,7 +1388,7 @@
   virtual void setAuxTarget(const TargetInfo *Aux) {}
 
   /// Whether target allows debuginfo types for decl only variables.
-  virtual bool allowDebugInfoForExternalVar() const { return false; }
+  virtual bool allowDebugInfoForExternalVar() const { return true; }
 
 protected:
   /// Copy type and layout related info.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71451: Support to emit extern variables debuginfo with "-fstandalone-debug"

[Jaydeep Chauhan via Phabricator via cfe-commits]

Jac1494 added a comment.



> that still seems very surprising - this change is specifically intended to 
> add more debug info, is it not? So do you have any ideas/theories as to why 
> that's not showing up in the data?

Yes, In clang and llvm  extern variable are there inside header file ,So that 
it will not add debug info.And in test side only two test 
case(virtual-methods.ll, cl-options.c)  are there which is  using 
"-fstandalone-debug" option and but in this test cases there is no extern 
variable.


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

https://reviews.llvm.org/D71451



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

[PATCH] D71451: Support to emit extern variables debuginfo with "-fstandalone-debug"

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D71451#1828647 , @Jac1494 wrote:

> > Do you have any reason to believe these patches would reduce the size of 
> > the debug info? It seems like they only add more debug info, and don't 
> > remove anything - so we'd expect an increase in the size, certainly not a 
> > decrease. That means, to me, there's probably a mistake in the measurement 
> > somehow?
>
> Yes, In clang binary DW_AT_linkage_name attribute is missing for "None" and 
> "__default_lock_policy" two variable because of that size is reduced. For 
> that fixed files are updated.
>  Now size is same with and without D71451  
> and D71599 .


that still seems very surprising - this change is specifically intended to add 
more debug info, is it not? So do you have any ideas/theories as to why that's 
not showing up in the data?


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

https://reviews.llvm.org/D71451



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

[PATCH] D71451: Support to emit extern variables debuginfo with "-fstandalone-debug"

[Jaydeep Chauhan via Phabricator via cfe-commits]

Jac1494 added a comment.

> Do you have any reason to believe these patches would reduce the size of the 
> debug info? It seems like they only add more debug info, and don't remove 
> anything - so we'd expect an increase in the size, certainly not a decrease. 
> That means, to me, there's probably a mistake in the measurement somehow?

Yes, In clang binary DW_AT_linkage_name attribute is missing for "None" and 
"__default_lock_policy" two variable because of that size is reduced. For that 
fixed files are updated.
Now size is same with and without D71451  and 
D71599 .


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

https://reviews.llvm.org/D71451



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

[PATCH] D71451: Support to emit extern variables debuginfo with "-fstandalone-debug"

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D71451#1822497 , @Jac1494 wrote:

> >> I'd be curious to the answer to David's questions. If the size increase is 
> >> because of unused extern variables coming in from libc or something then 
> >> it doesn't seem worth the cost.
>
> For above case clang size is increase because ,it is difference between clang 
> build without "-fstandalone-debug" option and clang build with 
> "-fstandalone-debug"  option and both build contain change D71451 
>  and D71599 
>  . So for clang build with 
> "-fstandalone-debug"  option size will be more because it will add debuginfo.
>
> And to check impact of my change on clang i have build clang with and without 
> D71451  and D71599 
>  change(testcases are not included).
>
> Size of clang without D71451  and D71599 
>  change and with option 
> "-fstandalone-debug":-
>  ===
> …
>  .comment 159 0
>  .debug_str   3994952 0
>  .debug_loc   941 0
>  .debug_abbrev  12754 0
>  .debug_info  2223641 0
>  .debug_ranges  46592 0
>  .debug_line   153901 0
>  .note.gnu.gold-version28 0
>  Total6827932
>
> Size of clang with D71451  and D71599 
>  change and with option  
> "-fstandalone-debug":-
>  ==
> …
>  .comment 159 0
>  .debug_str   3994894 0
>  .debug_loc   941 0
>  .debug_abbrev  12746 0
>  .debug_info  2223617 0
>  .debug_ranges  46592 0
>  .debug_line   153865 0
>  .note.gnu.gold-version28 0
>  Total6827806
>
> Size of clang with D71451  and D71599 
>  is reduced.


Do you have any reason to believe these patches would reduce the size of the 
debug info? It seems like they only add more debug info, and don't remove 
anything - so we'd expect an increase in the size, certainly not a decrease. 
That means, to me, there's probably a mistake in the measurement somehow?

> This results are with latest source and with self-host build of clang. First 
> I have build clang with Release mode and using that clang I have build clang 
> with debug mode with below options
> 
> “cmake -DLLVM_ENABLE_PROJECTS=clang -DCMAKE_BUILD_TYPE=Debug 
> -DCMAKE_C_COMPILER=$(which clang) -DCMAKE_CXX_COMPILER=$(which clang++) 
> -DLLVM_TARGETS_TO_BUILD="X86" -DBUILD_SHARED_LIBS=On  
> -DCMAKE_CXX_FLAGS="-fstandalone-debug"   -DCMAKE_C_FLAGS="-fstandalone-debug" 
>  
> -DCMAKE_INSTALL_PREFIX=/home/bft/Jaydeep/latest_llvm/llvm-project/install_withhstandalone
>  ../llvm”




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

https://reviews.llvm.org/D71451



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

[PATCH] D71451: Support to emit extern variables debuginfo with "-fstandalone-debug"

[Jaydeep Chauhan via Phabricator via cfe-commits]

Jac1494 added a comment.

>> I'd be curious to the answer to David's questions. If the size increase is 
>> because of unused extern variables coming in from libc or something then it 
>> doesn't seem worth the cost.

For above case clang size is increase because ,it is difference between clang 
build without "-fstandalone-debug" option and clang build with 
"-fstandalone-debug"  option and both build contain change D71451 
 and D71599  
. So for clang build with "-fstandalone-debug"  option size will be more 
because it will add debuginfo.

And to check impact of my change on clang i have build clang with and without 
D71451  and D71599 
 change(testcases are not included).

Size of clang without D71451 and D71599 change and with option 
"-fstandalone-debug":-
=

…
.comment 159 0
.debug_str   3994952 0
.debug_loc   941 0
.debug_abbrev  12754 0
.debug_info  2223641 0
.debug_ranges  46592 0
.debug_line   153901 0
.note.gnu.gold-version28 0
Total6827932

Size of clang with D71451 and D71599 change and with option  
"-fstandalone-debug":-
===

…
.comment 159 0
.debug_str   3994894 0
.debug_loc   941 0
.debug_abbrev  12746 0
.debug_info  2223617 0
.debug_ranges  46592 0
.debug_line   153865 0
.note.gnu.gold-version28 0
Total6827806

Size of clang with D71451  and D71599 
 is reduced.

This results are with latest source and with self-host build of clang. First I 
have build clang with Release mode and using that clang I have build clang with 
debug mode with below options

“cmake -DLLVM_ENABLE_PROJECTS=clang -DCMAKE_BUILD_TYPE=Debug 
-DCMAKE_C_COMPILER=$(which clang) -DCMAKE_CXX_COMPILER=$(which clang++) 
-DLLVM_TARGETS_TO_BUILD="X86" -DBUILD_SHARED_LIBS=On  
-DCMAKE_CXX_FLAGS="-fstandalone-debug"   -DCMAKE_C_FLAGS="-fstandalone-debug"  
-DCMAKE_INSTALL_PREFIX=/home/bft/Jaydeep/latest_llvm/llvm-project/install_withhstandalone
 ../llvm”


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

https://reviews.llvm.org/D71451



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

[PATCH] D71451: Support to emit extern variables debuginfo with "-fstandalone-debug"

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

In D71451#1816269 , @Jac1494 wrote:

> ping


I'd be curious to the answer to David's questions. If the size increase is 
because of unused extern variables coming in from libc or something then it 
doesn't seem worth the cost.




Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4598
+  if (!(DebugKind == clang::codegenoptions::FullDebugInfo))
+return;
 

nit: 

```
if (DebugKind < clang::codegenoptions::FullDebugInfo)
  return

```


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

https://reviews.llvm.org/D71451



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

[PATCH] D71451: Support to emit extern variables debuginfo with "-fstandalone-debug"

[Jaydeep Chauhan via Phabricator via cfe-commits]

Jac1494 added a comment.

ping


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

https://reviews.llvm.org/D71451



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

[PATCH] D71451: Support to emit extern variables debuginfo with "-fstandalone-debug"

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a subscriber: JDevlieghere.
dblaikie added a comment.

In D71451#1793828 , @Jac1494 wrote:

> > Am I reading this right that the data would suggest that enabling this 
> > feature /reduces/ the size of debug info sections? That doesn't sound right 
> > - can you explain why that would be the case? (perhaps the data is 
> > incorrect/it wasn't built with -fstandalone-debug?)
>
> Hi @dblaikie  sorry for incorrect data.
>
> Updated result are as per below:
>
> Without "-fstandalone-debug" option :-
>  $size -A -d build_debug_withoutstandalone/bin/clang-10
>  ...
>  ...
>  .comment 159 0
>  .debug_str   2655675 0
>  .debug_loc   941 0
>  .debug_abbrev  10761 0
>  .debug_info  1526674 0
>  .debug_ranges  46672 0
>  .debug_line   149807 0
>  .note.gnu.gold-version28 0
>  Total4786206
>
> With "-fstandalone-debug" option :-
>  $size -A -d build_debug_withstandalone/bin/clang-10
>  ...
>  ...
>  .comment 159 0
>  .debug_str   3997839 0
>  .debug_loc   941 0
>  .debug_abbrev  12746 0
>  .debug_info  2225392 0
>  .debug_ranges  46672 0
>  .debug_line   153779 0
>  .note.gnu.gold-version28 0
>  Total6833045


Ah, that looks more like what I'd expect - so that's a 42% increase in the 
final linked binary size? Yeah, that seems somewhat unlikely to be what anyone 
would want to enable, though I could be wrong (@aprantl @JDevlieghere - you 
folks use standalone-debug by default, have any opinions on this proposed 
change to standalone-debug behavior?)

Could you audit some files & see what sort of things are going in there? It 
might be that a more nuanced definition of "used" is needed.

For instance, does this global variable get emitted?

  extern int i;
  inline int f1() {
return i;
  }
  void f2() {
  }

'i' is used according to the AST, but it's not used even statically by this 
translation unit - f1 is never called here. So if this patch currently emits a 
declaration of 'i' then it shows one way there's some room for improvement - 
specifically I'd motivate the declaration emission by the IR usage itself - 
does the LLVM IR end up with a declaration of 'i' in it (for the purposes of 
loading/storing to it, etc) then add a declaration. This won't catch all cases 
(eg: "extern const int  i = 7; int f() { int x[i]; ... }" - there won't be any 
use of 'i' in the IR in that case), but would potentially be a workable 
tradeoff for now. Catching the "use of a global in a constant context" is part 
of a much broader challenge - to get that and some other existing cases like it 
right, we'd need some kind of reachability analysis of usage. That analysis 
could benefit other areas of debug info too, but is probably a lot more work.


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

https://reviews.llvm.org/D71451



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

[PATCH] D71451: Support to emit extern variables debuginfo with "-fstandalone-debug"

[Jaydeep Chauhan via Phabricator via cfe-commits]

Jac1494 added a comment.

> Am I reading this right that the data would suggest that enabling this 
> feature /reduces/ the size of debug info sections? That doesn't sound right - 
> can you explain why that would be the case? (perhaps the data is incorrect/it 
> wasn't built with -fstandalone-debug?)

Hi @dblaikie  sorry for incorrect data.

Updated result are as per below:

Without "-fstandalone-debug" option :-
...
...
.comment 159 0
.debug_str   2655675 0
.debug_loc   941 0
.debug_abbrev  10761 0
.debug_info  1526674 0
.debug_ranges  46672 0
.debug_line   149807 0
.note.gnu.gold-version28 0
Total4786206

With "-fstandalone-debug" option :-
...
...
.comment 159 0
.debug_str   3997839 0
.debug_loc   941 0
.debug_abbrev  12746 0
.debug_info  2225392 0
.debug_ranges  46672 0
.debug_line   153779 0
.note.gnu.gold-version28 0
Total6833045


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

https://reviews.llvm.org/D71451



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

[PATCH] D71451: Support to emit extern variables debuginfo with "-fstandalone-debug"

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D71451#1791314 , @Jac1494 wrote:

> >> @Jac1494 - have you made any measurements of the size increase of this 
> >> change? Perhaps a self-host build of clang?
>
> With change(default) and without change size diffrence given below ,in that 
> only debug section size is changed.
>
> Without change:-(default)
>  $size -A -d  build_debug_withoutfix/bin/clang-10 
>  
>  
>  .comment 159 0
>  .debug_str   3999832 0
>  .debug_loc   941 0
>  .debug_abbrev  12754 0
>  .debug_info  2225482 0
>  .debug_ranges  46672 0
>  .debug_line   153741 0
>  .note.gnu.gold-version28 0
>  Total6835098
>
> With change:-
>
> $size -A -d  build_debug_withfix/bin/clang-10 
>  .
>  .
>  .comment 159 0
>  .debug_str   3999775 0
>  .debug_loc   941 0
>  .debug_abbrev  12746 0
>  .debug_info  2225458 0
>  .debug_ranges  46672 0
>  .debug_line   153717 0
>  .note.gnu.gold-version28 0
>  Total6834985


Am I reading this right that the data would suggest that enabling this feature 
/reduces/ the size of debug info sections? That doesn't sound right - can you 
explain why that would be the case? (perhaps the data is incorrect/it wasn't 
built with -fstandalone-debug?)


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

https://reviews.llvm.org/D71451



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

[PATCH] D71451: Support to emit extern variables debuginfo with "-fstandalone-debug"

[Jaydeep Chauhan via Phabricator via cfe-commits]

Jac1494 added a comment.

>> @Jac1494 - have you made any measurements of the size increase of this 
>> change? Perhaps a self-host build of clang?

With change(default) and without change size diffrence given below ,in that 
only debug section size is changed.

Without change:-(default)
$size -A -d  build_debug_withoutfix/bin/clang-10 


.comment 159 0
.debug_str   3999832 0
.debug_loc   941 0
.debug_abbrev  12754 0
.debug_info  2225482 0
.debug_ranges  46672 0
.debug_line   153741 0
.note.gnu.gold-version28 0
Total6835098

With change:-

$size -A -d  build_debug_withfix/bin/clang-10 
.
.
.comment 159 0
.debug_str   3999775 0
.debug_loc   941 0
.debug_abbrev  12746 0
.debug_info  2225458 0
.debug_ranges  46672 0
.debug_line   153717 0
.note.gnu.gold-version28 0
Total6834985


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

https://reviews.llvm.org/D71451



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

[PATCH] D71451: Support to emit extern variables debuginfo with "-fstandalone-debug"

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D71451#1787729 , @Jac1494 wrote:

> In D71451#1786517 , @dblaikie wrote:
>
> > Do you have any particular users/use case for this?
>
>
> A case when shared library built without debug  info 
>  and executable with debug info. And while debugging we want to know the 
> types of extern.


OK - I was/am trying to clarify you have a particular user/need for this, since 
no one's had one for many years, so I'm just slightly surprised.

@aprantl - you folks might want to check what the growth of this is like since 
apple platforms defaults to standalone-debug.
@Jac1494 - have you made any measurements of the size increase of this change? 
Perhaps a self-host build of clang?


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

https://reviews.llvm.org/D71451



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

[PATCH] D71451: Support to emit extern variables debuginfo with "-fstandalone-debug"

[Jaydeep Chauhan via Phabricator via cfe-commits]

Jac1494 added a comment.

In D71451#1786517 , @dblaikie wrote:

> Do you have any particular users/use case for this?


A case when shared library built without debug  info 
and executable with debug info. And while debugging we want to know the types 
of extern.


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

https://reviews.llvm.org/D71451



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

[PATCH] D71451: Support to emit extern variables debuginfo with "-fstandalone-debug"

[Jaydeep Chauhan via Phabricator via cfe-commits]

Jac1494 updated this revision to Diff 234273.
Jac1494 added a comment.

Separate clang patch with test cases .


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

https://reviews.llvm.org/D71451

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGen/debug-info-extern-variable-basic.c
  clang/test/CodeGen/debug-info-extern-variable-unused.c


Index: clang/test/CodeGen/debug-info-extern-variable-unused.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-extern-variable-unused.c
@@ -0,0 +1,10 @@
+// RUN: %clang -emit-llvm -S -g -fstandalone-debug %s -o - | FileCheck %s
+
+extern int i;
+int foo() {
+  extern int j;
+  return 0;
+}
+
+// CHECK-NOT: distinct !DIGlobalVariable(name: "i"
+// CHECK-NOT: distinct !DIGlobalVariable(name: "j"
Index: clang/test/CodeGen/debug-info-extern-variable-basic.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-extern-variable-basic.c
@@ -0,0 +1,10 @@
+// RUN: %clang -emit-llvm -S -g -fstandalone-debug %s -o - | FileCheck %s
+
+extern int i;
+int foo() {
+  extern int j;
+  return i+j;
+}
+
+// CHECK: distinct !DIGlobalVariable(name: "i"
+// CHECK: distinct !DIGlobalVariable(name: "j"
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -4594,6 +4594,8 @@
   assert(DebugKind >= codegenoptions::LimitedDebugInfo);
   if (D->hasAttr())
 return;
+  if (!(DebugKind == clang::codegenoptions::FullDebugInfo))
+return;
 
   auto Align = getDeclAlignIfRequired(D, CGM.getContext());
   llvm::DIFile *Unit = getOrCreateFile(D->getLocation());
Index: clang/include/clang/Basic/TargetInfo.h
===
--- clang/include/clang/Basic/TargetInfo.h
+++ clang/include/clang/Basic/TargetInfo.h
@@ -1390,7 +1390,7 @@
   virtual void setAuxTarget(const TargetInfo *Aux) {}
 
   /// Whether target allows debuginfo types for decl only variables.
-  virtual bool allowDebugInfoForExternalVar() const { return false; }
+  virtual bool allowDebugInfoForExternalVar() const { return true; }
 
 protected:
   /// Copy type and layout related info.


Index: clang/test/CodeGen/debug-info-extern-variable-unused.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-extern-variable-unused.c
@@ -0,0 +1,10 @@
+// RUN: %clang -emit-llvm -S -g -fstandalone-debug %s -o - | FileCheck %s
+
+extern int i;
+int foo() {
+  extern int j;
+  return 0;
+}
+
+// CHECK-NOT: distinct !DIGlobalVariable(name: "i"
+// CHECK-NOT: distinct !DIGlobalVariable(name: "j"
Index: clang/test/CodeGen/debug-info-extern-variable-basic.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-extern-variable-basic.c
@@ -0,0 +1,10 @@
+// RUN: %clang -emit-llvm -S -g -fstandalone-debug %s -o - | FileCheck %s
+
+extern int i;
+int foo() {
+  extern int j;
+  return i+j;
+}
+
+// CHECK: distinct !DIGlobalVariable(name: "i"
+// CHECK: distinct !DIGlobalVariable(name: "j"
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -4594,6 +4594,8 @@
   assert(DebugKind >= codegenoptions::LimitedDebugInfo);
   if (D->hasAttr())
 return;
+  if (!(DebugKind == clang::codegenoptions::FullDebugInfo))
+return;
 
   auto Align = getDeclAlignIfRequired(D, CGM.getContext());
   llvm::DIFile *Unit = getOrCreateFile(D->getLocation());
Index: clang/include/clang/Basic/TargetInfo.h
===
--- clang/include/clang/Basic/TargetInfo.h
+++ clang/include/clang/Basic/TargetInfo.h
@@ -1390,7 +1390,7 @@
   virtual void setAuxTarget(const TargetInfo *Aux) {}
 
   /// Whether target allows debuginfo types for decl only variables.
-  virtual bool allowDebugInfoForExternalVar() const { return false; }
+  virtual bool allowDebugInfoForExternalVar() const { return true; }
 
 protected:
   /// Copy type and layout related info.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71451: Support to emit extern variables debuginfo with "-fstandalone-debug"

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Do you have any particular users/use case for this?

Might be best as two separate patches (one for the LLVM change, one for the 
Clang change) with tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71451



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

[PATCH] D71451: Support to emit extern variables debuginfo with "-fstandalone-debug"

[Jaydeep Chauhan via Phabricator via cfe-commits]

Jac1494 created this revision.
Jac1494 added reviewers: dblaikie, vsk, echristo.
Jac1494 added a project: debug-info.
Herald added subscribers: cfe-commits, hiraditya, aprantl.
Herald added projects: clang, LLVM.

Hi Devs,

Consider below testcases,

$cat shlib.c

int var;
int test()
{ return var++; }

$cat test.c

extern int test();
extern int var;
int main()
{ var++; printf("%d\n",test()); }

$clang  -fpic shlib.c -g -shared -o libshared.so
$clang  -L`pwd`   test.c -lshared -g
$export LD_LIBRARY_PATH=`pwd`

$ gdb a.out
GNU gdb (GDB) 8.2.50.20190204-git
Copyright (C) 2019 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-pc-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
http://www.gnu.org/software/gdb/bugs/.
Find the GDB manual and other documentation resources online at:

  .

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from a.out...
(gdb) b main
Breakpoint 1 at 0x400718: file test.c, line 6.
(gdb) pt var
type = 

As per above debugging ,for global extern variable "var" type info is missing 
because variable DebugInfo is not there in executable.
This is case where we don't run it, it means sharelib is not loaded. We can't 
get variable type.

LLVM is not adding debuginfo for externs with -g because this debug_info may 
increase final binary size.

So ,that i have given support for declaration of global extern variable with 
"-fstandalone-debug".

Thanks,
Jaydeep.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71451

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp


Index: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
@@ -171,7 +171,11 @@
   if (!GV->isDefinition())
 addFlag(*VariableDIE, dwarf::DW_AT_declaration);
   else
+  {
+// Add location.
+addLocationAttribute(VariableDIE, GV, GlobalExprs);
 addGlobalName(GV->getName(), *VariableDIE, DeclContext);
+  }
 
   if (uint32_t AlignInBytes = GV->getAlignInBytes())
 addUInt(*VariableDIE, dwarf::DW_AT_alignment, dwarf::DW_FORM_udata,
@@ -180,9 +184,6 @@
   if (MDTuple *TP = GV->getTemplateParams())
 addTemplateParams(*VariableDIE, DINodeArray(TP));
 
-  // Add location.
-  addLocationAttribute(VariableDIE, GV, GlobalExprs);
-
   return VariableDIE;
 }
 
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -4594,6 +4594,8 @@
   assert(DebugKind >= codegenoptions::LimitedDebugInfo);
   if (D->hasAttr())
 return;
+  if (!(DebugKind == clang::codegenoptions::FullDebugInfo))
+return;
 
   auto Align = getDeclAlignIfRequired(D, CGM.getContext());
   llvm::DIFile *Unit = getOrCreateFile(D->getLocation());
Index: clang/include/clang/Basic/TargetInfo.h
===
--- clang/include/clang/Basic/TargetInfo.h
+++ clang/include/clang/Basic/TargetInfo.h
@@ -1390,7 +1390,7 @@
   virtual void setAuxTarget(const TargetInfo *Aux) {}
 
   /// Whether target allows debuginfo types for decl only variables.
-  virtual bool allowDebugInfoForExternalVar() const { return false; }
+  virtual bool allowDebugInfoForExternalVar() const { return true; }
 
 protected:
   /// Copy type and layout related info.


Index: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
@@ -171,7 +171,11 @@
   if (!GV->isDefinition())
 addFlag(*VariableDIE, dwarf::DW_AT_declaration);
   else
+  {
+// Add location.
+addLocationAttribute(VariableDIE, GV, GlobalExprs);
 addGlobalName(GV->getName(), *VariableDIE, DeclContext);
+  }
 
   if (uint32_t AlignInBytes = GV->getAlignInBytes())
 addUInt(*VariableDIE, dwarf::DW_AT_alignment, dwarf::DW_FORM_udata,
@@ -180,9 +184,6 @@
   if (MDTuple *TP = GV->getTemplateParams())
 addTemplateParams(*VariableDIE, DINodeArray(TP));
 
-  // Add location.
-  addLocationAttribute(VariableDIE, GV, GlobalExprs);
-
   return VariableDIE;
 }
 
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -4594,6 +4594,8 @@