Re: [PATCH] Support bitfields in Wodr machinery (PR lto/85405).

2018-04-19 Thread Christophe Lyon 
On 19 April 2018 at 10:37, Martin Liška  wrote:
> On 04/18/2018 10:51 PM, Christophe Lyon wrote:
>> Hi,
>>
>>
>> On 17 April 2018 at 10:19, Jan Hubicka  wrote:
 On 04/17/2018 08:58 AM, Jakub Jelinek wrote:
> On Tue, Apr 17, 2018 at 07:39:20AM +0200, Martin Liška wrote:
>> +  if (DECL_BIT_FIELD (f1) != DECL_BIT_FIELD (f2))
>> +{
>> +  warn_odr (t1, t2, f1, f2, warn, warned,
>> +G_ ("one field is bitfield while other is not 
>> "));
>
> I think all the G_ uses don't put a space in between G_ and (
> Also, please avoid the trailing space in the message.

 Sure. I see other format violations, should I fix that in follow up patch:

 gcc/c-family/c-warn.c:   ? G_ ("floating point overflow in 
 expression %qE "
 gcc/c-family/c-warn.c:   : G_ ("floating point overflow in 
 expression of type %qT "
 gcc/gimple-ssa-sprintf.c: ? G_ ("%<%.*s%> 
 directive output between %wu and "
 gcc/gimple-ssa-sprintf.c: : G_ ("%<%.*s%> 
 directive output between %wu and "
 gcc/testsuite/gcc.dg/plugin/ggcplug.c:  warning (0, G_ ("option 
 '-fplugin-arg-%s-count-ggc-start=%s'"
 gcc/testsuite/gcc.dg/plugin/ggcplug.c:  warning (0, G_ ("option 
 '-fplugin-arg-%s-count-ggc-end=%s'"
 gcc/testsuite/gcc.dg/plugin/ggcplug.c:  warning (0, G_ ("option 
 '-fplugin-arg-%s-count-ggc-mark=%s'"
 gcc/testsuite/gcc.dg/plugin/ggcplug.c:  warning (0, G_ ("option 
 '-fplugin-arg-%s-test-extra-root=%s'"

 ?

>
> Do you diagnose if both are bit-fields, but with different bitcount, e.g. 
> if
> in your testcase you replace bool mbDisposed : 1; with int mbDisposed : 3;
> and bool mbDisposed; with int mbDisposed : 7; ?

 Good point, I add a new test-case, done in patch.
 Is it OK to install the patch?
>>> OK, thanks! (and sorry for the whitespace errors)
>>> Honza

 Martin

>
>> +  return false;
>> +}
>> +  else
>> +gcc_assert (DECL_NONADDRESSABLE_P (f1)
>> +== DECL_NONADDRESSABLE_P (f2));
>>  }
>>
>>/* If one aggregate has more fields than the other, they
>> diff --git a/gcc/testsuite/g++.dg/lto/pr85405_0.C 
>> b/gcc/testsuite/g++.dg/lto/pr85405_0.C
>> new file mode 100644
>> index 000..1a41d81099c
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/lto/pr85405_0.C
>> @@ -0,0 +1,18 @@
>> +// { dg-lto-do link }
>> +// { dg-lto-options {{-fPIC -shared -flto}} }
>> +
>> +class VclReferenceBase { // { dg-lto-warning "7: type 'struct 
>> VclReferenceBase' violates the C\\+\\+ One Definition Rule" }
>> +  int mnRefCnt;
>> +  bool mbDisposed : 1;
>> +  virtual ~VclReferenceBase();
>> +};
>> +class a;
>> +class b {
>> +  a 
>> +  bool c();
>> +};
>> +class B {
>> +  VclReferenceBase d;
>> +};
>> +class a : B {};
>> +bool b::c() { return false; }
>> diff --git a/gcc/testsuite/g++.dg/lto/pr85405_1.C 
>> b/gcc/testsuite/g++.dg/lto/pr85405_1.C
>> new file mode 100644
>> index 000..78606185624
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/lto/pr85405_1.C
>> @@ -0,0 +1,9 @@
>> +class VclReferenceBase {
>> +  int mnRefCnt;
>> +  bool mbDisposed;
>> +
>> +protected:
>> +  virtual ~VclReferenceBase();
>> +};
>> +class : VclReferenceBase {
>> +} a;
>>
>
> Jakub
>

>>>
 From 87380235f9b81bea4cf910e702192586c072ce93 Mon Sep 17 00:00:00 2001
 From: marxin 
 Date: Tue, 17 Apr 2018 10:11:07 +0200
 Subject: [PATCH] Fix coding style and add a new test-case (PR lto/85405).

 gcc/ChangeLog:

 2018-04-17  Martin Liska  

   PR lto/85405
   * ipa-devirt.c (odr_types_equivalent_p):

 gcc/testsuite/ChangeLog:

 2018-04-17  Martin Liska  

   PR lto/85405
   * g++.dg/lto/pr85405b_0.C: New test.
   * g++.dg/lto/pr85405b_1.C: New test.
>>
>> The new testcases require that -shared and -fpic work, which is not
>> the case on bare-metal targets
>> (eg arm-eabi, aarch64-elf).
>>
>> The attached small patch adds
>> +// { dg-require-effective-target shared }
>> +// { dg-require-effective-target fpic }
>> so the tests are skipped on such targets.
>>
>> OK for trunk?
>
> Thanks for the fix. It's quite obvious, please install it.
>

Thanks, done.

I also patched gcc/testsuite/g++.dg/lto/pr84805_0.C in the same way.

Christophe

> Martin
>
>>
>> Thanks,
>>
>> Christophe
>>
 ---
  gcc/ipa-devirt.c  |  2 +-
  gcc/testsuite/g++.dg/lto/pr85405b_0.C | 18 ++

Re: [PATCH] Support bitfields in Wodr machinery (PR lto/85405).

2018-04-19 Thread Martin Liška 
On 04/18/2018 10:51 PM, Christophe Lyon wrote:
> Hi,
> 
> 
> On 17 April 2018 at 10:19, Jan Hubicka  wrote:
>>> On 04/17/2018 08:58 AM, Jakub Jelinek wrote:
 On Tue, Apr 17, 2018 at 07:39:20AM +0200, Martin Liška wrote:
> +  if (DECL_BIT_FIELD (f1) != DECL_BIT_FIELD (f2))
> +{
> +  warn_odr (t1, t2, f1, f2, warn, warned,
> +G_ ("one field is bitfield while other is not 
> "));

 I think all the G_ uses don't put a space in between G_ and (
 Also, please avoid the trailing space in the message.
>>>
>>> Sure. I see other format violations, should I fix that in follow up patch:
>>>
>>> gcc/c-family/c-warn.c:   ? G_ ("floating point overflow in 
>>> expression %qE "
>>> gcc/c-family/c-warn.c:   : G_ ("floating point overflow in 
>>> expression of type %qT "
>>> gcc/gimple-ssa-sprintf.c: ? G_ ("%<%.*s%> directive 
>>> output between %wu and "
>>> gcc/gimple-ssa-sprintf.c: : G_ ("%<%.*s%> directive 
>>> output between %wu and "
>>> gcc/testsuite/gcc.dg/plugin/ggcplug.c:  warning (0, G_ ("option 
>>> '-fplugin-arg-%s-count-ggc-start=%s'"
>>> gcc/testsuite/gcc.dg/plugin/ggcplug.c:  warning (0, G_ ("option 
>>> '-fplugin-arg-%s-count-ggc-end=%s'"
>>> gcc/testsuite/gcc.dg/plugin/ggcplug.c:  warning (0, G_ ("option 
>>> '-fplugin-arg-%s-count-ggc-mark=%s'"
>>> gcc/testsuite/gcc.dg/plugin/ggcplug.c:  warning (0, G_ ("option 
>>> '-fplugin-arg-%s-test-extra-root=%s'"
>>>
>>> ?
>>>

 Do you diagnose if both are bit-fields, but with different bitcount, e.g. 
 if
 in your testcase you replace bool mbDisposed : 1; with int mbDisposed : 3;
 and bool mbDisposed; with int mbDisposed : 7; ?
>>>
>>> Good point, I add a new test-case, done in patch.
>>> Is it OK to install the patch?
>> OK, thanks! (and sorry for the whitespace errors)
>> Honza
>>>
>>> Martin
>>>

> +  return false;
> +}
> +  else
> +gcc_assert (DECL_NONADDRESSABLE_P (f1)
> +== DECL_NONADDRESSABLE_P (f2));
>  }
>
>/* If one aggregate has more fields than the other, they
> diff --git a/gcc/testsuite/g++.dg/lto/pr85405_0.C 
> b/gcc/testsuite/g++.dg/lto/pr85405_0.C
> new file mode 100644
> index 000..1a41d81099c
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/lto/pr85405_0.C
> @@ -0,0 +1,18 @@
> +// { dg-lto-do link }
> +// { dg-lto-options {{-fPIC -shared -flto}} }
> +
> +class VclReferenceBase { // { dg-lto-warning "7: type 'struct 
> VclReferenceBase' violates the C\\+\\+ One Definition Rule" }
> +  int mnRefCnt;
> +  bool mbDisposed : 1;
> +  virtual ~VclReferenceBase();
> +};
> +class a;
> +class b {
> +  a 
> +  bool c();
> +};
> +class B {
> +  VclReferenceBase d;
> +};
> +class a : B {};
> +bool b::c() { return false; }
> diff --git a/gcc/testsuite/g++.dg/lto/pr85405_1.C 
> b/gcc/testsuite/g++.dg/lto/pr85405_1.C
> new file mode 100644
> index 000..78606185624
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/lto/pr85405_1.C
> @@ -0,0 +1,9 @@
> +class VclReferenceBase {
> +  int mnRefCnt;
> +  bool mbDisposed;
> +
> +protected:
> +  virtual ~VclReferenceBase();
> +};
> +class : VclReferenceBase {
> +} a;
>

 Jakub

>>>
>>
>>> From 87380235f9b81bea4cf910e702192586c072ce93 Mon Sep 17 00:00:00 2001
>>> From: marxin 
>>> Date: Tue, 17 Apr 2018 10:11:07 +0200
>>> Subject: [PATCH] Fix coding style and add a new test-case (PR lto/85405).
>>>
>>> gcc/ChangeLog:
>>>
>>> 2018-04-17  Martin Liska  
>>>
>>>   PR lto/85405
>>>   * ipa-devirt.c (odr_types_equivalent_p):
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 2018-04-17  Martin Liska  
>>>
>>>   PR lto/85405
>>>   * g++.dg/lto/pr85405b_0.C: New test.
>>>   * g++.dg/lto/pr85405b_1.C: New test.
> 
> The new testcases require that -shared and -fpic work, which is not
> the case on bare-metal targets
> (eg arm-eabi, aarch64-elf).
> 
> The attached small patch adds
> +// { dg-require-effective-target shared }
> +// { dg-require-effective-target fpic }
> so the tests are skipped on such targets.
> 
> OK for trunk?

Thanks for the fix. It's quite obvious, please install it.

Martin

> 
> Thanks,
> 
> Christophe
> 
>>> ---
>>>  gcc/ipa-devirt.c  |  2 +-
>>>  gcc/testsuite/g++.dg/lto/pr85405b_0.C | 18 ++
>>>  gcc/testsuite/g++.dg/lto/pr85405b_1.C |  9 +
>>>  3 files changed, 28 insertions(+), 1 deletion(-)
>>>  create mode 100644 gcc/testsuite/g++.dg/lto/pr85405b_0.C
>>>  create mode 100644 gcc/testsuite/g++.dg/lto/pr85405b_1.C
>>>
>>> diff --git a/gcc/ipa-devirt.c b/gcc/ipa-devirt.c
>>> index

Re: [PATCH] Support bitfields in Wodr machinery (PR lto/85405).

2018-04-18 Thread Christophe Lyon 
Hi,


On 17 April 2018 at 10:19, Jan Hubicka  wrote:
>> On 04/17/2018 08:58 AM, Jakub Jelinek wrote:
>> > On Tue, Apr 17, 2018 at 07:39:20AM +0200, Martin Liška wrote:
>> >> +  if (DECL_BIT_FIELD (f1) != DECL_BIT_FIELD (f2))
>> >> +{
>> >> +  warn_odr (t1, t2, f1, f2, warn, warned,
>> >> +G_ ("one field is bitfield while other is not 
>> >> "));
>> >
>> > I think all the G_ uses don't put a space in between G_ and (
>> > Also, please avoid the trailing space in the message.
>>
>> Sure. I see other format violations, should I fix that in follow up patch:
>>
>> gcc/c-family/c-warn.c:   ? G_ ("floating point overflow in 
>> expression %qE "
>> gcc/c-family/c-warn.c:   : G_ ("floating point overflow in 
>> expression of type %qT "
>> gcc/gimple-ssa-sprintf.c: ? G_ ("%<%.*s%> directive 
>> output between %wu and "
>> gcc/gimple-ssa-sprintf.c: : G_ ("%<%.*s%> directive 
>> output between %wu and "
>> gcc/testsuite/gcc.dg/plugin/ggcplug.c:  warning (0, G_ ("option 
>> '-fplugin-arg-%s-count-ggc-start=%s'"
>> gcc/testsuite/gcc.dg/plugin/ggcplug.c:  warning (0, G_ ("option 
>> '-fplugin-arg-%s-count-ggc-end=%s'"
>> gcc/testsuite/gcc.dg/plugin/ggcplug.c:  warning (0, G_ ("option 
>> '-fplugin-arg-%s-count-ggc-mark=%s'"
>> gcc/testsuite/gcc.dg/plugin/ggcplug.c:  warning (0, G_ ("option 
>> '-fplugin-arg-%s-test-extra-root=%s'"
>>
>> ?
>>
>> >
>> > Do you diagnose if both are bit-fields, but with different bitcount, e.g. 
>> > if
>> > in your testcase you replace bool mbDisposed : 1; with int mbDisposed : 3;
>> > and bool mbDisposed; with int mbDisposed : 7; ?
>>
>> Good point, I add a new test-case, done in patch.
>> Is it OK to install the patch?
> OK, thanks! (and sorry for the whitespace errors)
> Honza
>>
>> Martin
>>
>> >
>> >> +  return false;
>> >> +}
>> >> +  else
>> >> +gcc_assert (DECL_NONADDRESSABLE_P (f1)
>> >> +== DECL_NONADDRESSABLE_P (f2));
>> >>  }
>> >>
>> >>/* If one aggregate has more fields than the other, they
>> >> diff --git a/gcc/testsuite/g++.dg/lto/pr85405_0.C 
>> >> b/gcc/testsuite/g++.dg/lto/pr85405_0.C
>> >> new file mode 100644
>> >> index 000..1a41d81099c
>> >> --- /dev/null
>> >> +++ b/gcc/testsuite/g++.dg/lto/pr85405_0.C
>> >> @@ -0,0 +1,18 @@
>> >> +// { dg-lto-do link }
>> >> +// { dg-lto-options {{-fPIC -shared -flto}} }
>> >> +
>> >> +class VclReferenceBase { // { dg-lto-warning "7: type 'struct 
>> >> VclReferenceBase' violates the C\\+\\+ One Definition Rule" }
>> >> +  int mnRefCnt;
>> >> +  bool mbDisposed : 1;
>> >> +  virtual ~VclReferenceBase();
>> >> +};
>> >> +class a;
>> >> +class b {
>> >> +  a 
>> >> +  bool c();
>> >> +};
>> >> +class B {
>> >> +  VclReferenceBase d;
>> >> +};
>> >> +class a : B {};
>> >> +bool b::c() { return false; }
>> >> diff --git a/gcc/testsuite/g++.dg/lto/pr85405_1.C 
>> >> b/gcc/testsuite/g++.dg/lto/pr85405_1.C
>> >> new file mode 100644
>> >> index 000..78606185624
>> >> --- /dev/null
>> >> +++ b/gcc/testsuite/g++.dg/lto/pr85405_1.C
>> >> @@ -0,0 +1,9 @@
>> >> +class VclReferenceBase {
>> >> +  int mnRefCnt;
>> >> +  bool mbDisposed;
>> >> +
>> >> +protected:
>> >> +  virtual ~VclReferenceBase();
>> >> +};
>> >> +class : VclReferenceBase {
>> >> +} a;
>> >>
>> >
>> > Jakub
>> >
>>
>
>> From 87380235f9b81bea4cf910e702192586c072ce93 Mon Sep 17 00:00:00 2001
>> From: marxin 
>> Date: Tue, 17 Apr 2018 10:11:07 +0200
>> Subject: [PATCH] Fix coding style and add a new test-case (PR lto/85405).
>>
>> gcc/ChangeLog:
>>
>> 2018-04-17  Martin Liska  
>>
>>   PR lto/85405
>>   * ipa-devirt.c (odr_types_equivalent_p):
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2018-04-17  Martin Liska  
>>
>>   PR lto/85405
>>   * g++.dg/lto/pr85405b_0.C: New test.
>>   * g++.dg/lto/pr85405b_1.C: New test.

The new testcases require that -shared and -fpic work, which is not
the case on bare-metal targets
(eg arm-eabi, aarch64-elf).

The attached small patch adds
+// { dg-require-effective-target shared }
+// { dg-require-effective-target fpic }
so the tests are skipped on such targets.

OK for trunk?

Thanks,

Christophe

>> ---
>>  gcc/ipa-devirt.c  |  2 +-
>>  gcc/testsuite/g++.dg/lto/pr85405b_0.C | 18 ++
>>  gcc/testsuite/g++.dg/lto/pr85405b_1.C |  9 +
>>  3 files changed, 28 insertions(+), 1 deletion(-)
>>  create mode 100644 gcc/testsuite/g++.dg/lto/pr85405b_0.C
>>  create mode 100644 gcc/testsuite/g++.dg/lto/pr85405b_1.C
>>
>> diff --git a/gcc/ipa-devirt.c b/gcc/ipa-devirt.c
>> index 85b8ef175f3..cc9b5e347e6 100644
>> --- a/gcc/ipa-devirt.c
>> +++ b/gcc/ipa-devirt.c
>> @@ -1599,7 +1599,7 @@ odr_types_equivalent_p (tree t1, tree t2, bool warn, 
>> bool *warned,
>>   if (DECL_BIT_FIELD (f1) !=

Re: [PATCH] Support bitfields in Wodr machinery (PR lto/85405).

2018-04-17 Thread Martin Liška 
On 04/17/2018 10:33 AM, Jakub Jelinek wrote:
> On Tue, Apr 17, 2018 at 10:29:35AM +0200, Martin Liška wrote:
>> On 04/17/2018 10:28 AM, Martin Liška wrote:
>>> I'm sending patch candidate.
>>
>> This one is the right one.
> 
> Ok for stage1 with appropriate ChangeLog entries.
> 
>   Jakub
> 

Good, thanks. There's version I'll install once we flip into stage1 again.

Martin
>From 93b53a0f55ebf4df7a96c620408af19546c7fc3a Mon Sep 17 00:00:00 2001
From: marxin 
Date: Tue, 17 Apr 2018 10:47:36 +0200
Subject: [PATCH] Fix GNU coding style for G_.

gcc/ChangeLog:

2018-04-17  Martin Liska  

	* gimple-ssa-sprintf.c (format_directive): Do not use
	space in between 'G_' and '('.

gcc/c-family/ChangeLog:

2018-04-17  Martin Liska  

	* c-warn.c (overflow_warning): Do not use
	space in between 'G_' and '('.

gcc/testsuite/ChangeLog:

2018-04-17  Martin Liska  

	* gcc.dg/plugin/ggcplug.c (plugin_init): Do not use
	space in between 'G_' and '('.
---
 gcc/c-family/c-warn.c |  8 
 gcc/gimple-ssa-sprintf.c  | 12 ++--
 gcc/testsuite/gcc.dg/plugin/ggcplug.c | 16 
 3 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c
index d0d9c7894a8..2614eb58f14 100644
--- a/gcc/c-family/c-warn.c
+++ b/gcc/c-family/c-warn.c
@@ -98,10 +98,10 @@ overflow_warning (location_t loc, tree value, tree expr)
 
 case REAL_CST:
   warnfmt = (expr
-		 ? G_ ("floating point overflow in expression %qE "
-		   "of type %qT results in %qE")
-		 : G_ ("floating point overflow in expression of type %qT "
-		   "results in %qE"));
+		 ? G_("floating point overflow in expression %qE "
+		  "of type %qT results in %qE")
+		 : G_("floating point overflow in expression of type %qT "
+		  "results in %qE"));
   break;
 
 case FIXED_CST:
diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index 4ec58605ce8..ec5e7046f6e 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -2933,12 +2933,12 @@ format_directive (const sprintf_dom_walker::call_info ,
   else
 	warned = fmtwarn (dirloc, argloc, NULL, info.warnopt (),
 			  fmtres.range.min > target_int_max ()
-			  ? G_ ("%<%.*s%> directive output between %wu and "
-"%wu bytes causes result to exceed "
-"%")
-			  : G_ ("%<%.*s%> directive output between %wu and "
-"%wu bytes may cause result to exceed "
-"%"), dirlen,
+			  ? G_("%<%.*s%> directive output between %wu and "
+			   "%wu bytes causes result to exceed "
+			   "%")
+			  : G_("%<%.*s%> directive output between %wu and "
+			   "%wu bytes may cause result to exceed "
+			   "%"), dirlen,
 			  target_to_host (hostdir, sizeof hostdir, dir.beg),
 			  fmtres.range.min, fmtres.range.max);
 }
diff --git a/gcc/testsuite/gcc.dg/plugin/ggcplug.c b/gcc/testsuite/gcc.dg/plugin/ggcplug.c
index c4bc334868b..c186d119371 100644
--- a/gcc/testsuite/gcc.dg/plugin/ggcplug.c
+++ b/gcc/testsuite/gcc.dg/plugin/ggcplug.c
@@ -64,8 +64,8 @@ plugin_init (struct plugin_name_args *plugin_info,
   if (!strcmp (argv[i].key, "count-ggc-start"))
 	{
 	  if (argv[i].value)
-	warning (0, G_ ("option '-fplugin-arg-%s-count-ggc-start=%s'"
-			" ignored (superfluous '=%s')"),
+	warning (0, G_("option '-fplugin-arg-%s-count-ggc-start=%s'"
+			   " ignored (superfluous '=%s')"),
 		 plugin_name, argv[i].value, argv[i].value);
 	  else
 	register_callback ("ggcplug",
@@ -76,8 +76,8 @@ plugin_init (struct plugin_name_args *plugin_info,
   else if (!strcmp (argv[i].key, "count-ggc-end"))
 	{
 	  if (argv[i].value)
-	warning (0, G_ ("option '-fplugin-arg-%s-count-ggc-end=%s'"
-			" ignored (superfluous '=%s')"),
+	warning (0, G_("option '-fplugin-arg-%s-count-ggc-end=%s'"
+			   " ignored (superfluous '=%s')"),
 		 plugin_name, argv[i].value, argv[i].value);
 	  else
 	register_callback ("ggcplug",
@@ -88,8 +88,8 @@ plugin_init (struct plugin_name_args *plugin_info,
   else if (!strcmp (argv[i].key, "count-ggc-mark"))
 	{
 	  if (argv[i].value)
-	warning (0, G_ ("option '-fplugin-arg-%s-count-ggc-mark=%s'"
-			" ignored (superfluous '=%s')"),
+	warning (0, G_("option '-fplugin-arg-%s-count-ggc-mark=%s'"
+			   " ignored (superfluous '=%s')"),
 		 plugin_name, argv[i].value, argv[i].value);
 	  else
 	register_callback ("ggcplug",
@@ -100,8 +100,8 @@ plugin_init (struct plugin_name_args *plugin_info,
   else if (!strcmp (argv[i].key, "test-extra-root"))
 	{
 	  if (argv[i].value)
-	warning (0, G_ ("option '-fplugin-arg-%s-test-extra-root=%s'"
-			" ignored (superfluous '=%s')"),
+	warning (0, G_("option '-fplugin-arg-%s-test-extra-root=%s'"
+			   " ignored (superfluous '=%s')"),
 		 plugin_name, argv[i].value, argv[i].value);
 	  else
 	register_callback ("ggcplug",
--

Re: [PATCH] Support bitfields in Wodr machinery (PR lto/85405).

2018-04-17 Thread Jakub Jelinek 
On Tue, Apr 17, 2018 at 10:17:15AM +0200, Martin Liška wrote:
> Sure. I see other format violations, should I fix that in follow up patch:
> 
> gcc/c-family/c-warn.c:   ? G_ ("floating point overflow in expression 
> %qE "
> gcc/c-family/c-warn.c:   : G_ ("floating point overflow in expression 
> of type %qT "
> gcc/gimple-ssa-sprintf.c: ? G_ ("%<%.*s%> directive 
> output between %wu and "
> gcc/gimple-ssa-sprintf.c: : G_ ("%<%.*s%> directive 
> output between %wu and "
> gcc/testsuite/gcc.dg/plugin/ggcplug.c:  warning (0, G_ ("option 
> '-fplugin-arg-%s-count-ggc-start=%s'"
> gcc/testsuite/gcc.dg/plugin/ggcplug.c:  warning (0, G_ ("option 
> '-fplugin-arg-%s-count-ggc-end=%s'"
> gcc/testsuite/gcc.dg/plugin/ggcplug.c:  warning (0, G_ ("option 
> '-fplugin-arg-%s-count-ggc-mark=%s'"
> gcc/testsuite/gcc.dg/plugin/ggcplug.c:  warning (0, G_ ("option 
> '-fplugin-arg-%s-test-extra-root=%s'"

If you mean the space between G_ and (, sure, if you mean trailing
whitespace, note likely none of the above have trailing whitespace, at least
if they are followed by "something on another line.

> 2018-04-17  Martin Liska  
> 
>   PR lto/85405
>   * ipa-devirt.c (odr_types_equivalent_p):

Please say what you've changed ;)

Jakub

Re: [PATCH] Support bitfields in Wodr machinery (PR lto/85405).

2018-04-17 Thread Jakub Jelinek 
On Tue, Apr 17, 2018 at 10:29:35AM +0200, Martin Liška wrote:
> On 04/17/2018 10:28 AM, Martin Liška wrote:
> > I'm sending patch candidate.
> 
> This one is the right one.

Ok for stage1 with appropriate ChangeLog entries.

Jakub

Re: [PATCH] Support bitfields in Wodr machinery (PR lto/85405).

2018-04-17 Thread Martin Liška 
On 04/17/2018 10:28 AM, Martin Liška wrote:
> I'm sending patch candidate.

This one is the right one.

Martin
diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c
index d0d9c7894a8..2614eb58f14 100644
--- a/gcc/c-family/c-warn.c
+++ b/gcc/c-family/c-warn.c
@@ -98,10 +98,10 @@ overflow_warning (location_t loc, tree value, tree expr)
 
 case REAL_CST:
   warnfmt = (expr
-		 ? G_ ("floating point overflow in expression %qE "
-		   "of type %qT results in %qE")
-		 : G_ ("floating point overflow in expression of type %qT "
-		   "results in %qE"));
+		 ? G_("floating point overflow in expression %qE "
+		  "of type %qT results in %qE")
+		 : G_("floating point overflow in expression of type %qT "
+		  "results in %qE"));
   break;
 
 case FIXED_CST:
diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index 4ec58605ce8..ec5e7046f6e 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -2933,12 +2933,12 @@ format_directive (const sprintf_dom_walker::call_info ,
   else
 	warned = fmtwarn (dirloc, argloc, NULL, info.warnopt (),
 			  fmtres.range.min > target_int_max ()
-			  ? G_ ("%<%.*s%> directive output between %wu and "
-"%wu bytes causes result to exceed "
-"%")
-			  : G_ ("%<%.*s%> directive output between %wu and "
-"%wu bytes may cause result to exceed "
-"%"), dirlen,
+			  ? G_("%<%.*s%> directive output between %wu and "
+			   "%wu bytes causes result to exceed "
+			   "%")
+			  : G_("%<%.*s%> directive output between %wu and "
+			   "%wu bytes may cause result to exceed "
+			   "%"), dirlen,
 			  target_to_host (hostdir, sizeof hostdir, dir.beg),
 			  fmtres.range.min, fmtres.range.max);
 }
diff --git a/gcc/testsuite/gcc.dg/plugin/ggcplug.c b/gcc/testsuite/gcc.dg/plugin/ggcplug.c
index c4bc334868b..c186d119371 100644
--- a/gcc/testsuite/gcc.dg/plugin/ggcplug.c
+++ b/gcc/testsuite/gcc.dg/plugin/ggcplug.c
@@ -64,8 +64,8 @@ plugin_init (struct plugin_name_args *plugin_info,
   if (!strcmp (argv[i].key, "count-ggc-start"))
 	{
 	  if (argv[i].value)
-	warning (0, G_ ("option '-fplugin-arg-%s-count-ggc-start=%s'"
-			" ignored (superfluous '=%s')"),
+	warning (0, G_("option '-fplugin-arg-%s-count-ggc-start=%s'"
+			   " ignored (superfluous '=%s')"),
 		 plugin_name, argv[i].value, argv[i].value);
 	  else
 	register_callback ("ggcplug",
@@ -76,8 +76,8 @@ plugin_init (struct plugin_name_args *plugin_info,
   else if (!strcmp (argv[i].key, "count-ggc-end"))
 	{
 	  if (argv[i].value)
-	warning (0, G_ ("option '-fplugin-arg-%s-count-ggc-end=%s'"
-			" ignored (superfluous '=%s')"),
+	warning (0, G_("option '-fplugin-arg-%s-count-ggc-end=%s'"
+			   " ignored (superfluous '=%s')"),
 		 plugin_name, argv[i].value, argv[i].value);
 	  else
 	register_callback ("ggcplug",
@@ -88,8 +88,8 @@ plugin_init (struct plugin_name_args *plugin_info,
   else if (!strcmp (argv[i].key, "count-ggc-mark"))
 	{
 	  if (argv[i].value)
-	warning (0, G_ ("option '-fplugin-arg-%s-count-ggc-mark=%s'"
-			" ignored (superfluous '=%s')"),
+	warning (0, G_("option '-fplugin-arg-%s-count-ggc-mark=%s'"
+			   " ignored (superfluous '=%s')"),
 		 plugin_name, argv[i].value, argv[i].value);
 	  else
 	register_callback ("ggcplug",
@@ -100,8 +100,8 @@ plugin_init (struct plugin_name_args *plugin_info,
   else if (!strcmp (argv[i].key, "test-extra-root"))
 	{
 	  if (argv[i].value)
-	warning (0, G_ ("option '-fplugin-arg-%s-test-extra-root=%s'"
-			" ignored (superfluous '=%s')"),
+	warning (0, G_("option '-fplugin-arg-%s-test-extra-root=%s'"
+			   " ignored (superfluous '=%s')"),
 		 plugin_name, argv[i].value, argv[i].value);
 	  else
 	register_callback ("ggcplug",

Re: [PATCH] Support bitfields in Wodr machinery (PR lto/85405).

2018-04-17 Thread Martin Liška 
On 04/17/2018 10:21 AM, Jakub Jelinek wrote:
> On Tue, Apr 17, 2018 at 10:17:15AM +0200, Martin Liška wrote:
>> Sure. I see other format violations, should I fix that in follow up patch:
>>
>> gcc/c-family/c-warn.c:   ? G_ ("floating point overflow in 
>> expression %qE "
>> gcc/c-family/c-warn.c:   : G_ ("floating point overflow in 
>> expression of type %qT "
>> gcc/gimple-ssa-sprintf.c: ? G_ ("%<%.*s%> directive 
>> output between %wu and "
>> gcc/gimple-ssa-sprintf.c: : G_ ("%<%.*s%> directive 
>> output between %wu and "
>> gcc/testsuite/gcc.dg/plugin/ggcplug.c:  warning (0, G_ ("option 
>> '-fplugin-arg-%s-count-ggc-start=%s'"
>> gcc/testsuite/gcc.dg/plugin/ggcplug.c:  warning (0, G_ ("option 
>> '-fplugin-arg-%s-count-ggc-end=%s'"
>> gcc/testsuite/gcc.dg/plugin/ggcplug.c:  warning (0, G_ ("option 
>> '-fplugin-arg-%s-count-ggc-mark=%s'"
>> gcc/testsuite/gcc.dg/plugin/ggcplug.c:  warning (0, G_ ("option 
>> '-fplugin-arg-%s-test-extra-root=%s'"
> 
> If you mean the space between G_ and (, sure, if you mean trailing
> whitespace, note likely none of the above have trailing whitespace, at least
> if they are followed by "something on another line.

I'm sending patch candidate.

> 
>> 2018-04-17  Martin Liska  
>>
>>  PR lto/85405
>>  * ipa-devirt.c (odr_types_equivalent_p):
> 
> Please say what you've changed ;)

Yes, done that in r259431.

M.

> 
>   Jakub
> 

>From 143424c754415a20487e0dc615ac46cd934c8f78 Mon Sep 17 00:00:00 2001
From: marxin 
Date: Tue, 17 Apr 2018 10:11:07 +0200
Subject: [PATCH] Fix coding style and add a new test-case (PR lto/85405).

gcc/ChangeLog:

2018-04-17  Martin Liska  

	PR lto/85405
	* ipa-devirt.c (odr_types_equivalent_p): Remove trailing
	in message, remote space in between '_G' and '('.

gcc/testsuite/ChangeLog:

2018-04-17  Martin Liska  

	PR lto/85405
	* g++.dg/lto/pr85405b_0.C: New test.
	* g++.dg/lto/pr85405b_1.C: New test.
---
 gcc/ipa-devirt.c  |  2 +-
 gcc/testsuite/g++.dg/lto/pr85405b_0.C | 18 ++
 gcc/testsuite/g++.dg/lto/pr85405b_1.C |  9 +
 3 files changed, 28 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/lto/pr85405b_0.C
 create mode 100644 gcc/testsuite/g++.dg/lto/pr85405b_1.C

diff --git a/gcc/ipa-devirt.c b/gcc/ipa-devirt.c
index 85b8ef175f3..cc9b5e347e6 100644
--- a/gcc/ipa-devirt.c
+++ b/gcc/ipa-devirt.c
@@ -1599,7 +1599,7 @@ odr_types_equivalent_p (tree t1, tree t2, bool warn, bool *warned,
 		if (DECL_BIT_FIELD (f1) != DECL_BIT_FIELD (f2))
 		  {
 		warn_odr (t1, t2, f1, f2, warn, warned,
-			  G_ ("one field is bitfield while other is not "));
+			  G_("one field is bitfield while other is not"));
 		return false;
 		  }
 		else
diff --git a/gcc/testsuite/g++.dg/lto/pr85405b_0.C b/gcc/testsuite/g++.dg/lto/pr85405b_0.C
new file mode 100644
index 000..a692abb7715
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lto/pr85405b_0.C
@@ -0,0 +1,18 @@
+// { dg-lto-do link }
+// { dg-lto-options {{-fPIC -shared -flto}} }
+
+class VclReferenceBase { // { dg-lto-warning "7: type 'struct VclReferenceBase' violates the C\\+\\+ One Definition Rule" }
+  int mnRefCnt;
+  int mbDisposed : 3;
+  virtual ~VclReferenceBase();
+};
+class a;
+class b {
+  a 
+  bool c();
+};
+class B {
+  VclReferenceBase d;
+};
+class a : B {};
+bool b::c() { return false; }
diff --git a/gcc/testsuite/g++.dg/lto/pr85405b_1.C b/gcc/testsuite/g++.dg/lto/pr85405b_1.C
new file mode 100644
index 000..fd98e631d56
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lto/pr85405b_1.C
@@ -0,0 +1,9 @@
+class VclReferenceBase {
+  int mnRefCnt;
+  int mbDisposed: 7; // { dg-lto-message "19: a field of same name but different type is defined in another translation unit" }
+
+protected:
+  virtual ~VclReferenceBase();
+};
+class : VclReferenceBase {
+} a;
-- 
2.16.3

Re: [PATCH] Support bitfields in Wodr machinery (PR lto/85405).

2018-04-17 Thread Jan Hubicka 
> On 04/17/2018 08:58 AM, Jakub Jelinek wrote:
> > On Tue, Apr 17, 2018 at 07:39:20AM +0200, Martin Liška wrote:
> >> +  if (DECL_BIT_FIELD (f1) != DECL_BIT_FIELD (f2))
> >> +{
> >> +  warn_odr (t1, t2, f1, f2, warn, warned,
> >> +G_ ("one field is bitfield while other is not "));
> > 
> > I think all the G_ uses don't put a space in between G_ and (
> > Also, please avoid the trailing space in the message.
> 
> Sure. I see other format violations, should I fix that in follow up patch:
> 
> gcc/c-family/c-warn.c:   ? G_ ("floating point overflow in expression 
> %qE "
> gcc/c-family/c-warn.c:   : G_ ("floating point overflow in expression 
> of type %qT "
> gcc/gimple-ssa-sprintf.c: ? G_ ("%<%.*s%> directive 
> output between %wu and "
> gcc/gimple-ssa-sprintf.c: : G_ ("%<%.*s%> directive 
> output between %wu and "
> gcc/testsuite/gcc.dg/plugin/ggcplug.c:  warning (0, G_ ("option 
> '-fplugin-arg-%s-count-ggc-start=%s'"
> gcc/testsuite/gcc.dg/plugin/ggcplug.c:  warning (0, G_ ("option 
> '-fplugin-arg-%s-count-ggc-end=%s'"
> gcc/testsuite/gcc.dg/plugin/ggcplug.c:  warning (0, G_ ("option 
> '-fplugin-arg-%s-count-ggc-mark=%s'"
> gcc/testsuite/gcc.dg/plugin/ggcplug.c:  warning (0, G_ ("option 
> '-fplugin-arg-%s-test-extra-root=%s'"
> 
> ?
> 
> > 
> > Do you diagnose if both are bit-fields, but with different bitcount, e.g. if
> > in your testcase you replace bool mbDisposed : 1; with int mbDisposed : 3;
> > and bool mbDisposed; with int mbDisposed : 7; ?
> 
> Good point, I add a new test-case, done in patch.
> Is it OK to install the patch?
OK, thanks! (and sorry for the whitespace errors)
Honza
> 
> Martin
> 
> > 
> >> +  return false;
> >> +}
> >> +  else
> >> +gcc_assert (DECL_NONADDRESSABLE_P (f1)
> >> +== DECL_NONADDRESSABLE_P (f2));
> >>  }
> >>  
> >>/* If one aggregate has more fields than the other, they
> >> diff --git a/gcc/testsuite/g++.dg/lto/pr85405_0.C 
> >> b/gcc/testsuite/g++.dg/lto/pr85405_0.C
> >> new file mode 100644
> >> index 000..1a41d81099c
> >> --- /dev/null
> >> +++ b/gcc/testsuite/g++.dg/lto/pr85405_0.C
> >> @@ -0,0 +1,18 @@
> >> +// { dg-lto-do link }
> >> +// { dg-lto-options {{-fPIC -shared -flto}} }
> >> +
> >> +class VclReferenceBase { // { dg-lto-warning "7: type 'struct 
> >> VclReferenceBase' violates the C\\+\\+ One Definition Rule" }
> >> +  int mnRefCnt;
> >> +  bool mbDisposed : 1;
> >> +  virtual ~VclReferenceBase();
> >> +};
> >> +class a;
> >> +class b {
> >> +  a 
> >> +  bool c();
> >> +};
> >> +class B {
> >> +  VclReferenceBase d;
> >> +};
> >> +class a : B {};
> >> +bool b::c() { return false; }
> >> diff --git a/gcc/testsuite/g++.dg/lto/pr85405_1.C 
> >> b/gcc/testsuite/g++.dg/lto/pr85405_1.C
> >> new file mode 100644
> >> index 000..78606185624
> >> --- /dev/null
> >> +++ b/gcc/testsuite/g++.dg/lto/pr85405_1.C
> >> @@ -0,0 +1,9 @@
> >> +class VclReferenceBase {
> >> +  int mnRefCnt;
> >> +  bool mbDisposed;
> >> +
> >> +protected:
> >> +  virtual ~VclReferenceBase();
> >> +};
> >> +class : VclReferenceBase {
> >> +} a;
> >>
> > 
> > Jakub
> > 
> 

> From 87380235f9b81bea4cf910e702192586c072ce93 Mon Sep 17 00:00:00 2001
> From: marxin 
> Date: Tue, 17 Apr 2018 10:11:07 +0200
> Subject: [PATCH] Fix coding style and add a new test-case (PR lto/85405).
> 
> gcc/ChangeLog:
> 
> 2018-04-17  Martin Liska  
> 
>   PR lto/85405
>   * ipa-devirt.c (odr_types_equivalent_p):
> 
> gcc/testsuite/ChangeLog:
> 
> 2018-04-17  Martin Liska  
> 
>   PR lto/85405
>   * g++.dg/lto/pr85405b_0.C: New test.
>   * g++.dg/lto/pr85405b_1.C: New test.
> ---
>  gcc/ipa-devirt.c  |  2 +-
>  gcc/testsuite/g++.dg/lto/pr85405b_0.C | 18 ++
>  gcc/testsuite/g++.dg/lto/pr85405b_1.C |  9 +
>  3 files changed, 28 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/g++.dg/lto/pr85405b_0.C
>  create mode 100644 gcc/testsuite/g++.dg/lto/pr85405b_1.C
> 
> diff --git a/gcc/ipa-devirt.c b/gcc/ipa-devirt.c
> index 85b8ef175f3..cc9b5e347e6 100644
> --- a/gcc/ipa-devirt.c
> +++ b/gcc/ipa-devirt.c
> @@ -1599,7 +1599,7 @@ odr_types_equivalent_p (tree t1, tree t2, bool warn, 
> bool *warned,
>   if (DECL_BIT_FIELD (f1) != DECL_BIT_FIELD (f2))
> {
>   warn_odr (t1, t2, f1, f2, warn, warned,
> -   G_ ("one field is bitfield while other is not "));
> +   G_("one field is bitfield while other is not"));
>   return false;
> }
>   else
> diff --git a/gcc/testsuite/g++.dg/lto/pr85405b_0.C 
> b/gcc/testsuite/g++.dg/lto/pr85405b_0.C
> new file mode 100644
> index 000..a692abb7715
> --- /dev/null
> +++

Re: [PATCH] Support bitfields in Wodr machinery (PR lto/85405).

2018-04-17 Thread Martin Liška 
On 04/17/2018 08:58 AM, Jakub Jelinek wrote:
> On Tue, Apr 17, 2018 at 07:39:20AM +0200, Martin Liška wrote:
>> +if (DECL_BIT_FIELD (f1) != DECL_BIT_FIELD (f2))
>> +  {
>> +warn_odr (t1, t2, f1, f2, warn, warned,
>> +  G_ ("one field is bitfield while other is not "));
> 
> I think all the G_ uses don't put a space in between G_ and (
> Also, please avoid the trailing space in the message.

Sure. I see other format violations, should I fix that in follow up patch:

gcc/c-family/c-warn.c:   ? G_ ("floating point overflow in expression 
%qE "
gcc/c-family/c-warn.c:   : G_ ("floating point overflow in expression 
of type %qT "
gcc/gimple-ssa-sprintf.c: ? G_ ("%<%.*s%> directive 
output between %wu and "
gcc/gimple-ssa-sprintf.c: : G_ ("%<%.*s%> directive 
output between %wu and "
gcc/testsuite/gcc.dg/plugin/ggcplug.c:  warning (0, G_ ("option 
'-fplugin-arg-%s-count-ggc-start=%s'"
gcc/testsuite/gcc.dg/plugin/ggcplug.c:  warning (0, G_ ("option 
'-fplugin-arg-%s-count-ggc-end=%s'"
gcc/testsuite/gcc.dg/plugin/ggcplug.c:  warning (0, G_ ("option 
'-fplugin-arg-%s-count-ggc-mark=%s'"
gcc/testsuite/gcc.dg/plugin/ggcplug.c:  warning (0, G_ ("option 
'-fplugin-arg-%s-test-extra-root=%s'"

?

> 
> Do you diagnose if both are bit-fields, but with different bitcount, e.g. if
> in your testcase you replace bool mbDisposed : 1; with int mbDisposed : 3;
> and bool mbDisposed; with int mbDisposed : 7; ?

Good point, I add a new test-case, done in patch.
Is it OK to install the patch?

Martin

> 
>> +return false;
>> +  }
>> +else
>> +  gcc_assert (DECL_NONADDRESSABLE_P (f1)
>> +  == DECL_NONADDRESSABLE_P (f2));
>>}
>>  
>>  /* If one aggregate has more fields than the other, they
>> diff --git a/gcc/testsuite/g++.dg/lto/pr85405_0.C 
>> b/gcc/testsuite/g++.dg/lto/pr85405_0.C
>> new file mode 100644
>> index 000..1a41d81099c
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/lto/pr85405_0.C
>> @@ -0,0 +1,18 @@
>> +// { dg-lto-do link }
>> +// { dg-lto-options {{-fPIC -shared -flto}} }
>> +
>> +class VclReferenceBase { // { dg-lto-warning "7: type 'struct 
>> VclReferenceBase' violates the C\\+\\+ One Definition Rule" }
>> +  int mnRefCnt;
>> +  bool mbDisposed : 1;
>> +  virtual ~VclReferenceBase();
>> +};
>> +class a;
>> +class b {
>> +  a 
>> +  bool c();
>> +};
>> +class B {
>> +  VclReferenceBase d;
>> +};
>> +class a : B {};
>> +bool b::c() { return false; }
>> diff --git a/gcc/testsuite/g++.dg/lto/pr85405_1.C 
>> b/gcc/testsuite/g++.dg/lto/pr85405_1.C
>> new file mode 100644
>> index 000..78606185624
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/lto/pr85405_1.C
>> @@ -0,0 +1,9 @@
>> +class VclReferenceBase {
>> +  int mnRefCnt;
>> +  bool mbDisposed;
>> +
>> +protected:
>> +  virtual ~VclReferenceBase();
>> +};
>> +class : VclReferenceBase {
>> +} a;
>>
> 
>   Jakub
> 

>From 87380235f9b81bea4cf910e702192586c072ce93 Mon Sep 17 00:00:00 2001
From: marxin 
Date: Tue, 17 Apr 2018 10:11:07 +0200
Subject: [PATCH] Fix coding style and add a new test-case (PR lto/85405).

gcc/ChangeLog:

2018-04-17  Martin Liska  

	PR lto/85405
	* ipa-devirt.c (odr_types_equivalent_p):

gcc/testsuite/ChangeLog:

2018-04-17  Martin Liska  

	PR lto/85405
	* g++.dg/lto/pr85405b_0.C: New test.
	* g++.dg/lto/pr85405b_1.C: New test.
---
 gcc/ipa-devirt.c  |  2 +-
 gcc/testsuite/g++.dg/lto/pr85405b_0.C | 18 ++
 gcc/testsuite/g++.dg/lto/pr85405b_1.C |  9 +
 3 files changed, 28 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/lto/pr85405b_0.C
 create mode 100644 gcc/testsuite/g++.dg/lto/pr85405b_1.C

diff --git a/gcc/ipa-devirt.c b/gcc/ipa-devirt.c
index 85b8ef175f3..cc9b5e347e6 100644
--- a/gcc/ipa-devirt.c
+++ b/gcc/ipa-devirt.c
@@ -1599,7 +1599,7 @@ odr_types_equivalent_p (tree t1, tree t2, bool warn, bool *warned,
 		if (DECL_BIT_FIELD (f1) != DECL_BIT_FIELD (f2))
 		  {
 		warn_odr (t1, t2, f1, f2, warn, warned,
-			  G_ ("one field is bitfield while other is not "));
+			  G_("one field is bitfield while other is not"));
 		return false;
 		  }
 		else
diff --git a/gcc/testsuite/g++.dg/lto/pr85405b_0.C b/gcc/testsuite/g++.dg/lto/pr85405b_0.C
new file mode 100644
index 000..a692abb7715
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lto/pr85405b_0.C
@@ -0,0 +1,18 @@
+// { dg-lto-do link }
+// { dg-lto-options {{-fPIC -shared -flto}} }
+
+class VclReferenceBase { // { dg-lto-warning "7: type 'struct VclReferenceBase' violates the C\\+\\+ One Definition Rule" }
+  int mnRefCnt;
+  int mbDisposed : 3;
+  virtual ~VclReferenceBase();
+};
+class a;
+class b {
+  a 
+  bool c();
+};
+class B {
+  VclReferenceBase d;
+};
+class a : B {};
+bool b::c() { return false; }

Re: [PATCH] Support bitfields in Wodr machinery (PR lto/85405).

2018-04-17 Thread Jakub Jelinek 
On Tue, Apr 17, 2018 at 07:39:20AM +0200, Martin Liška wrote:
> + if (DECL_BIT_FIELD (f1) != DECL_BIT_FIELD (f2))
> +   {
> + warn_odr (t1, t2, f1, f2, warn, warned,
> +   G_ ("one field is bitfield while other is not "));

I think all the G_ uses don't put a space in between G_ and (
Also, please avoid the trailing space in the message.

Do you diagnose if both are bit-fields, but with different bitcount, e.g. if
in your testcase you replace bool mbDisposed : 1; with int mbDisposed : 3;
and bool mbDisposed; with int mbDisposed : 7; ?

> + return false;
> +   }
> + else
> +   gcc_assert (DECL_NONADDRESSABLE_P (f1)
> +   == DECL_NONADDRESSABLE_P (f2));
> }
>  
>   /* If one aggregate has more fields than the other, they
> diff --git a/gcc/testsuite/g++.dg/lto/pr85405_0.C 
> b/gcc/testsuite/g++.dg/lto/pr85405_0.C
> new file mode 100644
> index 000..1a41d81099c
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/lto/pr85405_0.C
> @@ -0,0 +1,18 @@
> +// { dg-lto-do link }
> +// { dg-lto-options {{-fPIC -shared -flto}} }
> +
> +class VclReferenceBase { // { dg-lto-warning "7: type 'struct 
> VclReferenceBase' violates the C\\+\\+ One Definition Rule" }
> +  int mnRefCnt;
> +  bool mbDisposed : 1;
> +  virtual ~VclReferenceBase();
> +};
> +class a;
> +class b {
> +  a 
> +  bool c();
> +};
> +class B {
> +  VclReferenceBase d;
> +};
> +class a : B {};
> +bool b::c() { return false; }
> diff --git a/gcc/testsuite/g++.dg/lto/pr85405_1.C 
> b/gcc/testsuite/g++.dg/lto/pr85405_1.C
> new file mode 100644
> index 000..78606185624
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/lto/pr85405_1.C
> @@ -0,0 +1,9 @@
> +class VclReferenceBase {
> +  int mnRefCnt;
> +  bool mbDisposed;
> +
> +protected:
> +  virtual ~VclReferenceBase();
> +};
> +class : VclReferenceBase {
> +} a;
> 

Jakub

[PATCH] Support bitfields in Wodr machinery (PR lto/85405).

2018-04-16 Thread Martin Liška 
Hi.

This is Honza's ODR warning patch that I've just tested. He approved that.
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Martin

gcc/ChangeLog:

2018-04-16  Jan Hubicka  

PR lto/85405
* ipa-devirt.c (odr_types_equivalent_p): Handle bit fields.

gcc/testsuite/ChangeLog:

2018-04-16  Martin Liska  

PR lto/85405
* g++.dg/lto/pr85405_0.C: New test.
* g++.dg/lto/pr85405_1.C: New test.
---
 gcc/ipa-devirt.c | 11 +--
 gcc/testsuite/g++.dg/lto/pr85405_0.C | 18 ++
 gcc/testsuite/g++.dg/lto/pr85405_1.C |  9 +
 3 files changed, 36 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/lto/pr85405_0.C
 create mode 100644 gcc/testsuite/g++.dg/lto/pr85405_1.C


diff --git a/gcc/ipa-devirt.c b/gcc/ipa-devirt.c
index fa9380cce80..5da0f72d14f 100644
--- a/gcc/ipa-devirt.c
+++ b/gcc/ipa-devirt.c
@@ -1587,8 +1587,15 @@ odr_types_equivalent_p (tree t1, tree t2, bool warn, bool *warned,
  "in another translation unit"));
 		return false;
 		  }
-		gcc_assert (DECL_NONADDRESSABLE_P (f1)
-			== DECL_NONADDRESSABLE_P (f2));
+		if (DECL_BIT_FIELD (f1) != DECL_BIT_FIELD (f2))
+		  {
+		warn_odr (t1, t2, f1, f2, warn, warned,
+			  G_ ("one field is bitfield while other is not "));
+		return false;
+		  }
+		else
+		  gcc_assert (DECL_NONADDRESSABLE_P (f1)
+			  == DECL_NONADDRESSABLE_P (f2));
 	  }
 
 	/* If one aggregate has more fields than the other, they
diff --git a/gcc/testsuite/g++.dg/lto/pr85405_0.C b/gcc/testsuite/g++.dg/lto/pr85405_0.C
new file mode 100644
index 000..1a41d81099c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lto/pr85405_0.C
@@ -0,0 +1,18 @@
+// { dg-lto-do link }
+// { dg-lto-options {{-fPIC -shared -flto}} }
+
+class VclReferenceBase { // { dg-lto-warning "7: type 'struct VclReferenceBase' violates the C\\+\\+ One Definition Rule" }
+  int mnRefCnt;
+  bool mbDisposed : 1;
+  virtual ~VclReferenceBase();
+};
+class a;
+class b {
+  a 
+  bool c();
+};
+class B {
+  VclReferenceBase d;
+};
+class a : B {};
+bool b::c() { return false; }
diff --git a/gcc/testsuite/g++.dg/lto/pr85405_1.C b/gcc/testsuite/g++.dg/lto/pr85405_1.C
new file mode 100644
index 000..78606185624
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lto/pr85405_1.C
@@ -0,0 +1,9 @@
+class VclReferenceBase {
+  int mnRefCnt;
+  bool mbDisposed;
+
+protected:
+  virtual ~VclReferenceBase();
+};
+class : VclReferenceBase {
+} a;