Re: [PATCH] Emit DW_AT_inline for C++17 inline variables (take 2)

2016-11-01 Thread Mark Wielaard 
On Wed, 2016-11-02 at 00:08 +0100, Mark Wielaard wrote:
> That seemed to have been the last usage of origin_die in gen_variable_die.
> So removing that caused:
> 
> /home/mark/src/gcc/gcc/dwarf2out.c: In function ‘void gen_variable_die(tree, 
> tree, dw_die_ref)’:
> /home/mark/src/gcc/gcc/dwarf2out.c:22454:14: error: variable ‘origin_die’ set 
> but not used [-Werror=unused-but-set-variable]
>dw_die_ref origin_die = NULL;
>   ^~
> cc1plus: all warnings being treated as errors
> make[3]: *** [dwarf2out.o] Error 1
> 
> The following seems to work around that and makes things build again for me:

Which is precisely what Jakub checked in 5 minutes before I sent that
email.

Sorry for the noise. Everything builds again.

Thanks,

Mark

Re: [PATCH] Emit DW_AT_inline for C++17 inline variables (take 2)

2016-11-01 Thread Mark Wielaard 
On Tue, Nov 01, 2016 at 01:20:23PM -0400, Jason Merrill wrote:
> On Tue, Nov 1, 2016@12:10 PM, Jakub Jelinek  wrote:
> > + && !get_AT (var_die, DW_AT_inline)
> > + && (origin_die == NULL || get_AT (origin_die, DW_AT_inline) == 
> > NULL)
> 
> Can we drop this repetition (here and for DW_AT_const_expr)?  get_AT
> should look through DW_AT_abstract_origin, and we should have added
> that earlier in gen_variable_die.
> 
> OK with that change.

That seemed to have been the last usage of origin_die in gen_variable_die.
So removing that caused:

/home/mark/src/gcc/gcc/dwarf2out.c: In function ‘void gen_variable_die(tree, 
tree, dw_die_ref)’:
/home/mark/src/gcc/gcc/dwarf2out.c:22454:14: error: variable ‘origin_die’ set 
but not used [-Werror=unused-but-set-variable]
   dw_die_ref origin_die = NULL;
  ^~
cc1plus: all warnings being treated as errors
make[3]: *** [dwarf2out.o] Error 1

The following seems to work around that and makes things build again for me:

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 5ff6f97..24b85c1 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -22451,7 +22451,6 @@ gen_variable_die (tree decl, tree origin, dw_die_ref 
context_die)
   tree ultimate_origin;
   dw_die_ref var_die;
   dw_die_ref old_die = decl ? lookup_decl_die (decl) : NULL;
-  dw_die_ref origin_die = NULL;
   bool declaration = (DECL_EXTERNAL (decl_or_origin)
  || class_or_namespace_scope_p (context_die));
   bool specialization_p = false;
@@ -22627,7 +22626,7 @@ gen_variable_die (tree decl, tree origin, dw_die_ref 
context_die)
 var_die = new_die (DW_TAG_variable, context_die, decl);
 
   if (origin != NULL)
-origin_die = add_abstract_origin_attribute (var_die, origin);
+add_abstract_origin_attribute (var_die, origin);
 
   /* Loop unrolling can create multiple blocks that refer to the same
  static variable, so we must test for the DW_AT_declaration flag.

Cheers,

Mark

Re: [PATCH] Emit DW_AT_inline for C++17 inline variables (take 2)

2016-11-01 Thread Jason Merrill 
On Tue, Nov 1, 2016 at 12:10 PM, Jakub Jelinek  wrote:
> + && !get_AT (var_die, DW_AT_inline)
> + && (origin_die == NULL || get_AT (origin_die, DW_AT_inline) == NULL)

Can we drop this repetition (here and for DW_AT_const_expr)?  get_AT
should look through DW_AT_abstract_origin, and we should have added
that earlier in gen_variable_die.

OK with that change.

Jason

[PATCH] Emit DW_AT_inline for C++17 inline variables (take 2)

2016-11-01 Thread Jakub Jelinek 
On Mon, Oct 31, 2016 at 10:38:40AM -0400, Jason Merrill wrote:
> > The current DWARF 5 wording is:
> > "If the variable entry represents the defining declaration for a C++ static 
> > data
> > member of a structure, class or union, the entry has a DW_AT_specification
> > attribute, whose value is a reference to the debugging information entry
> > representing the declaration of this data member. The referenced entry has
> > the tag DW_TAG_member and will be a child of some class, structure or
> > union type entry." on page 98 in DWARF5_Public_Review.pdf.
> 
> Yes, this changed in DWARF 3; DWARF 2 didn't specify the tag.  I think
> this was a mistake.
> 
> > Incidentally, I've filed today a DWARF issue that Appendix A doesn't list
> > for DW_TAG_member lots of attributes that are allowed for DW_TAG_variable
> > and are useful for static data members.
> 
> Using DW_TAG_variable would address that, too.

After some IRC discussions, following updated patch ensures that even for
inline static data members we emit DW_TAG_member in class with
DW_AT_declaration and DW_TAG_variable outside of the class with
DW_AT_specification pointing to the DW_TAG_member DIE.  DW_AT_location and
DW_AT_linkage_name appear on the DW_TAG_variable DIE,
DW_AT_const_value/DW_AT_const_expr/DW_AT_inline etc. on DW_TAG_member
(except for DW_AT_inline like for non-inline static data members).

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2016-11-01  Jakub Jelinek  

* dwarf2out.c (add_name_and_src_coords_attributes): Add NO_LINKAGE_NAME
argument, don't call add_linkage_name if it is true.
(gen_variable_die): For C++ inline static data members, consider the
initial call when old_die is NULL to be declaration and call
add_name_and_src_coords_attributes in that case with true as
NO_LINKAGE_NAME.  Add DW_AT_inline attribute if needed.
(gen_member_die): For C++ inline static data members, emit a
definition DIE right away in DW_TAG_compile_unit context.
cp/
* cp-objcp-common.c (cp_decl_dwarf_attribute): Handle DW_AT_inline.
testsuite/
* g++.dg/debug/dwarf2/inline-var-1.C: New test.

--- gcc/dwarf2out.c.jj  2016-10-31 22:46:16.932801519 +0100
+++ gcc/dwarf2out.c 2016-11-01 11:51:57.839403234 +0100
@@ -3503,7 +3503,7 @@ static void add_prototyped_attribute (dw
 static dw_die_ref add_abstract_origin_attribute (dw_die_ref, tree);
 static void add_pure_or_virtual_attribute (dw_die_ref, tree);
 static void add_src_coords_attributes (dw_die_ref, tree);
-static void add_name_and_src_coords_attributes (dw_die_ref, tree);
+static void add_name_and_src_coords_attributes (dw_die_ref, tree, bool = 
false);
 static void add_discr_value (dw_die_ref, dw_discr_value *);
 static void add_discr_list (dw_die_ref, dw_discr_list_ref);
 static inline dw_discr_list_ref AT_discr_list (dw_attr_node *);
@@ -19820,7 +19820,8 @@ add_linkage_name (dw_die_ref die, tree d
given decl, but only if it actually has a name.  */
 
 static void
-add_name_and_src_coords_attributes (dw_die_ref die, tree decl)
+add_name_and_src_coords_attributes (dw_die_ref die, tree decl,
+   bool no_linkage_name)
 {
   tree decl_name;
 
@@ -19833,7 +19834,8 @@ add_name_and_src_coords_attributes (dw_d
   if (! DECL_ARTIFICIAL (decl))
add_src_coords_attributes (die, decl);
 
-  add_linkage_name (die, decl);
+  if (!no_linkage_name)
+   add_linkage_name (die, decl);
 }
 
 #ifdef VMS_DEBUGGING_INFO
@@ -22215,6 +22217,22 @@ gen_variable_die (tree decl, tree origin
   bool declaration = (DECL_EXTERNAL (decl_or_origin)
  || class_or_namespace_scope_p (context_die));
   bool specialization_p = false;
+  bool no_linkage_name = false;
+
+  /* While C++ inline static data members have definitions inside of the
+ class, force the first DIE to be a declaration, then let gen_member_die
+ reparent it to the class context and call gen_variable_die again
+ to create the outside of the class DIE for the definition.  */
+  if (!declaration
+  && old_die == NULL
+  && decl
+  && DECL_CONTEXT (decl)
+  && TYPE_P (DECL_CONTEXT (decl))
+  && lang_hooks.decls.decl_dwarf_attribute (decl, DW_AT_inline) != -1)
+{
+  declaration = true;
+  no_linkage_name = true;
+}
 
   ultimate_origin = decl_ultimate_origin (decl_or_origin);
   if (decl || ultimate_origin)
@@ -22402,7 +22420,7 @@ gen_variable_die (tree decl, tree origin
}
 }
   else
-add_name_and_src_coords_attributes (var_die, decl);
+add_name_and_src_coords_attributes (var_die, decl, no_linkage_name);
 
   if ((origin == NULL && !specialization_p)
   || (origin != NULL
@@ -22465,6 +22483,17 @@ gen_variable_die (tree decl, tree origin
   && (origin_die == NULL || get_AT (origin_die, DW_AT_const_expr) == NULL)
   && !specialization_p)
 add_AT_flag (var_die, DW_AT_const_expr, 1);
+
+  if

Re: [PATCH] Emit DW_AT_inline for C++17 inline variables

2016-10-31 Thread Jason Merrill 
On Mon, Oct 31, 2016 at 10:25 AM, Jakub Jelinek  wrote:
> On Mon, Oct 31, 2016 at 09:52:28AM -0400, Jason Merrill wrote:
>> On 10/14/2016 01:33 PM, Jakub Jelinek wrote:
>> >This also uses the infrastructure of the langhook patch I've sent earlier.
>> >It emits (if not strict dwarf) DW_AT_inline on explicit or implicit inline
>> >variables, and also tweaks dwarf2out so that for inline static data members
>> >we consider in-class declarations as definitions (emit DW_AT_linkage_name
>> >and no DW_AT_declaration) for them.
>>
>> Hmm, so there's no DW_TAG_variable for the inline static data member? That
>> seems problematic to me.  The DWARF 3 convention that static data members
>> use DW_TAG_member seems rather misguided, since in fact they are variables.
>> Why did that change?
>
> I had to bisect it, apparently it has been changed by myself in
> r145770, rationale in thread starting at
> https://gcc.gnu.org/ml/gcc-patches/2009-04/msg00039.html
>
> The current DWARF 5 wording is:
> "If the variable entry represents the defining declaration for a C++ static 
> data
> member of a structure, class or union, the entry has a DW_AT_specification
> attribute, whose value is a reference to the debugging information entry
> representing the declaration of this data member. The referenced entry has
> the tag DW_TAG_member and will be a child of some class, structure or
> union type entry." on page 98 in DWARF5_Public_Review.pdf.

Yes, this changed in DWARF 3; DWARF 2 didn't specify the tag.  I think
this was a mistake.

> Incidentally, I've filed today a DWARF issue that Appendix A doesn't list
> for DW_TAG_member lots of attributes that are allowed for DW_TAG_variable
> and are useful for static data members.

Using DW_TAG_variable would address that, too.

Jason

Re: [PATCH] Emit DW_AT_inline for C++17 inline variables

2016-10-31 Thread Jakub Jelinek 
On Mon, Oct 31, 2016 at 09:52:28AM -0400, Jason Merrill wrote:
> On 10/14/2016 01:33 PM, Jakub Jelinek wrote:
> >This also uses the infrastructure of the langhook patch I've sent earlier.
> >It emits (if not strict dwarf) DW_AT_inline on explicit or implicit inline
> >variables, and also tweaks dwarf2out so that for inline static data members
> >we consider in-class declarations as definitions (emit DW_AT_linkage_name
> >and no DW_AT_declaration) for them.
> 
> Hmm, so there's no DW_TAG_variable for the inline static data member? That
> seems problematic to me.  The DWARF 3 convention that static data members
> use DW_TAG_member seems rather misguided, since in fact they are variables.
> Why did that change?

I had to bisect it, apparently it has been changed by myself in
r145770, rationale in thread starting at
https://gcc.gnu.org/ml/gcc-patches/2009-04/msg00039.html

The current DWARF 5 wording is:
"If the variable entry represents the defining declaration for a C++ static data
member of a structure, class or union, the entry has a DW_AT_specification
attribute, whose value is a reference to the debugging information entry
representing the declaration of this data member. The referenced entry has
the tag DW_TAG_member and will be a child of some class, structure or
union type entry." on page 98 in DWARF5_Public_Review.pdf.
And page 117 has in 5.7.6:
"A data member (as opposed to a member function) is represented by a
debugging information entry with the tag DW_TAG_member."

So, using DW_TAG_variable as DW_TAG_{structure,class}_type children
for the non-inline static data members would violate these two.
C++17 is outside of the scope of DWARF5 I guess, so the question is to find
out what do we want for those and agree on that with consumers and other
producers.

Incidentally, I've filed today a DWARF issue that Appendix A doesn't list
for DW_TAG_member lots of attributes that are allowed for DW_TAG_variable
and are useful for static data members.  Some of them we want anyway, at
least unless we agree on the non-inline static data members no longer be
DW_TAG_member in the structure/class type and change the standard.
In particular DW_AT_const_value and DW_AT_const_expr are desirable even
for the static data members in C++98..14 (or non-inline ones in C++17).
The rest, like DW_AT_linkage_name, DW_AT_location, etc. are generally more
useful mainly for the inline ones.

Jakub

Re: [PATCH] Emit DW_AT_inline for C++17 inline variables

2016-10-31 Thread Jason Merrill 

On 10/14/2016 01:33 PM, Jakub Jelinek wrote:

This also uses the infrastructure of the langhook patch I've sent earlier.
It emits (if not strict dwarf) DW_AT_inline on explicit or implicit inline
variables, and also tweaks dwarf2out so that for inline static data members
we consider in-class declarations as definitions (emit DW_AT_linkage_name
and no DW_AT_declaration) for them.


Hmm, so there's no DW_TAG_variable for the inline static data member? 
That seems problematic to me.  The DWARF 3 convention that static data 
members use DW_TAG_member seems rather misguided, since in fact they are 
variables.  Why did that change?


Jason

[PATCH] Emit DW_AT_inline for C++17 inline variables

2016-10-14 Thread Jakub Jelinek 
Hi!

This also uses the infrastructure of the langhook patch I've sent earlier.
It emits (if not strict dwarf) DW_AT_inline on explicit or implicit inline
variables, and also tweaks dwarf2out so that for inline static data members
we consider in-class declarations as definitions (emit DW_AT_linkage_name
and no DW_AT_declaration) for them.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2016-10-14  Jakub Jelinek  

* dwarf2out.c (add_linkage_name): Add linkage attribute even for
DW_TAG_member if it is inline static data member.
(gen_variable_die): Consider inline static data member's DW_TAG_member
to be definition rather than declaration.  Add DW_AT_inline attribute
if needed.
cp/
* cp-objcp-common.c (cp_decl_dwarf_attribute): Handle DW_AT_inline.
testsuite/
* g++.dg/debug/dwarf2/inline-var-1.C: New test.

--- gcc/dwarf2out.c.jj  2016-10-14 15:22:57.0 +0200
+++ gcc/dwarf2out.c 2016-10-14 16:39:12.226917016 +0200
@@ -18896,7 +18896,10 @@ add_linkage_name (dw_die_ref die, tree d
   && VAR_OR_FUNCTION_DECL_P (decl)
   && TREE_PUBLIC (decl)
   && !(VAR_P (decl) && DECL_REGISTER (decl))
-  && die->die_tag != DW_TAG_member)
+  && (die->die_tag != DW_TAG_member
+ || (VAR_P (decl)
+ && (lang_hooks.decls.decl_dwarf_attribute (decl, DW_AT_inline)
+ != -1
 add_linkage_name_raw (die, decl);
 }
 
@@ -21382,6 +21385,20 @@ gen_variable_die (tree decl, tree origin
   return;
 }
 
+  /* For static data members, the declaration (or definition for inline
+ variables) in the class is supposed to have DW_TAG_member tag;
+ the specification if any should still be DW_TAG_variable referencing
+ the DW_TAG_member DIE.  */
+  enum dwarf_tag tag = DW_TAG_variable;
+  if (declaration && class_scope_p (context_die))
+{
+  tag = DW_TAG_member;
+  /* Inline static data members are defined inside of the class.  */
+  if (lang_hooks.decls.decl_dwarf_attribute (decl_or_origin, DW_AT_inline)
+ != -1)
+   declaration = false;
+}
+
   if (old_die)
 {
   if (declaration)
@@ -21414,14 +21431,7 @@ gen_variable_die (tree decl, tree origin
  goto gen_variable_die_location;
}
 }
-
-  /* For static data members, the declaration in the class is supposed
- to have DW_TAG_member tag; the specification should still be
- DW_TAG_variable referencing the DW_TAG_member DIE.  */
-  if (declaration && class_scope_p (context_die))
-var_die = new_die (DW_TAG_member, context_die, decl);
-  else
-var_die = new_die (DW_TAG_variable, context_die, decl);
+  var_die = new_die (tag, context_die, decl);
 
   if (origin != NULL)
 origin_die = add_abstract_origin_attribute (var_die, origin);
@@ -21521,6 +21531,17 @@ gen_variable_die (tree decl, tree origin
   && (origin_die == NULL || get_AT (origin_die, DW_AT_const_expr) == NULL)
   && !specialization_p)
 add_AT_flag (var_die, DW_AT_const_expr, 1);
+
+  if (!dwarf_strict)
+{
+  int inl = lang_hooks.decls.decl_dwarf_attribute (decl_or_origin,
+  DW_AT_inline);
+  if (inl != -1
+ && !get_AT (var_die, DW_AT_inline)
+ && (origin_die == NULL || get_AT (origin_die, DW_AT_inline) == NULL)
+ && !specialization_p)
+   add_AT_unsigned (var_die, DW_AT_inline, inl);
+}
 }
 
 /* Generate a DIE to represent a named constant.  */
--- gcc/cp/cp-objcp-common.c.jj 2016-10-14 15:01:27.0 +0200
+++ gcc/cp/cp-objcp-common.c2016-10-14 15:46:14.650303379 +0200
@@ -173,6 +173,16 @@ cp_decl_dwarf_attribute (const_tree decl
return 1;
   break;
 
+case DW_AT_inline:
+  if (VAR_P (decl) && DECL_INLINE_VAR_P (decl))
+   {
+ if (DECL_VAR_DECLARED_INLINE_P (decl))
+   return DW_INL_declared_inlined;
+ else
+   return DW_INL_inlined;
+   }
+  break;
+
 default:
   break;
 }
--- gcc/testsuite/g++.dg/debug/dwarf2/inline-var-1.C.jj 2016-10-14 
16:55:30.345512927 +0200
+++ gcc/testsuite/g++.dg/debug/dwarf2/inline-var-1.C2016-10-14 
16:56:45.704558635 +0200
@@ -0,0 +1,26 @@
+// { dg-do compile }
+// { dg-options "-O -std=c++1z -g -dA -gno-strict-dwarf" }
+// { dg-require-weak "" }
+// { dg-final { scan-assembler-times "0x3\[^\n\r]* DW_AT_inline" 6 } }
+// { dg-final { scan-assembler-times "0x1\[^\n\r]* DW_AT_inline" 2 } }
+// { dg-final { scan-assembler-not " DW_AT_declaration" } }
+// { dg-final { scan-assembler-times " DW_AT_\[^\n\r]*linkage_name" 7 } }
+
+inline int a;
+struct S
+{
+  static inline double b = 4.0;
+  static constexpr int c = 2;
+  static constexpr inline char d = 3;
+} s;
+template 
+inline int e = N;
+int  = e<2>;
+template 
+struct T
+{
+  static inline double g = 4.0;
+  static constexpr int h = 2;
+  static inline constexpr char i = 3;
+};
+T<5> t;

Jakub