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.
Jason
