On November 20, 2017 4:51:04 PM GMT+01:00, Marek Polacek <pola...@redhat.com>
wrote:
>On Thu, Nov 16, 2017 at 02:20:59PM -0500, Jason Merrill wrote:
>> On Thu, Nov 16, 2017 at 12:41 PM, Marek Polacek <pola...@redhat.com>
>wrote:
>> > On Tue, Nov 14, 2017 at 07:34:54AM +0100, Richard Biener wrote:
>> >> On November 14, 2017 6:21:41 AM GMT+01:00, Jason Merrill
><ja...@redhat.com> wrote:
>> >> >On Mon, Nov 13, 2017 at 1:02 PM, Marek Polacek
><pola...@redhat.com>
>> >> >> In the end I did two bootstraps with the patch, but modifed one
>of
>> >> >them
>> >> >> to always return false for ix86_is_empty_record. Then I
>compared all
>> >> >the
>> >> >> *.o in both dirs. The result is attached. Then I looked at
>> >> >DW_AT_producer
>> >> >> for all these .o that differ; all of them are C++. Is this
>enough to
>> >> >> clear our concerns?
>> >> >
>> >> >Hmm, a bunch of these are right at the beginning, bytes 41 and
>65, in
>> >> >the header.
>> >> >
>> >> >Did you build them in the different trunk/trunk2 directories? I
>think
>> >> >Jakub was suggesting building them in the same directory.
>> >> >> And I also ran a bootstrap with --enable-cxx-flags=-Wabi=11,
>and
>> >> >didn't
>> >> >> see any warnings.
>> >> >
>> >> >If there's a codegen change, there ought to be a warning to go
>along
>> >> >with it.
>> >>
>> >> The question was of course also for unintended changes but yes (I
>was mainly concerned by libstdc++ ABI changes).
>> >
>> > Ok, I did two bootstraps in the same dir, one with
>ix86_is_empty_record always
>> > returning false. There were a few object files that differ in
>their assembly
>> > between those two bootstraps. Previously I didn't see any warnings
>because
>> > I hadn't thought of -Wsystem-headers. Also, we intentionally don't
>warn if
>> > the empty parameter is the last one:
>> >
>> > + bool seen_empty_type = false;
>> > + FOREACH_FUNCTION_ARGS (fntype, argtype, iter)
>> > + {
>> > + if (VOID_TYPE_P (argtype))
>> > + break;
>> > + if (TYPE_EMPTY_P (argtype))
>> > + seen_empty_type = true;
>> > + else if (seen_empty_type)
>> > + {
>> > + cum->warn_empty = true;
>> > + break;
>> > + }
>> > + }
>> >
>> > After enabling -Wsystem-headers and tweaking the code above so that
>we warn
>> > even if the empty parameter is trailing I can see the warnings that
>correspond
>> > to the assembly changes. Below is a summary of what I found.
>TL;DR: I don't
>> > see any unintended changes.
>>
>> Looks good to me.
>
>Thanks!
>
>Richi, are you ok with the patch now?
Yes.
Richard.
>Honza/Uros, are the config/i386/* changes ok?
>
>The last version of the patch is
>https://gcc.gnu.org/ml/gcc-patches/2017-11/msg00969.html
>
> Marek
