This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Support bitfields in Wodr machinery (PR lto/85405).
On 04/18/2018 10:51 PM, Christophe Lyon wrote:
> Hi,
>
>
> On 17 April 2018 at 10:19, Jan Hubicka <hubicka@ucw.cz> 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 00000000000..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 &e;
>>>>> + 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 00000000000..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 <mliska@suse.cz>
>>> 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 <mliska@suse.cz>
>>>
>>> PR lto/85405
>>> * ipa-devirt.c (odr_types_equivalent_p):
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 2018-04-17 Martin Liska <mliska@suse.cz>
>>>
>>> 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 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 00000000000..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 &e;
>>> + 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 00000000000..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
>>>
>>