diff mbox

debug-early branch merged into mainline

Message ID 5574EE9C.4070908@redhat.com
State New
Headers show

Commit Message

Aldy Hernandez June 8, 2015, 1:23 a.m. UTC
On 06/07/2015 02:33 PM, Richard Biener wrote:
> On June 7, 2015 6:00:05 PM GMT+02:00, Aldy Hernandez <aldyh@redhat.com> wrote:
>> On 06/07/2015 11:25 AM, Richard Biener wrote:
>>> On June 7, 2015 5:03:30 PM GMT+02:00, Aldy Hernandez
>> <aldyh@redhat.com> wrote:
>>>> On 06/06/2015 05:49 AM, Andreas Schwab wrote:
>>>>> Bootstrap fails on aarch64:
>>>>>
>>>>> Comparing stages 2 and 3
>>>>> warning: gcc/cc1objplus-checksum.o differs
>>>>> warning: gcc/cc1obj-checksum.o differs
>>>>> warning: gcc/cc1plus-checksum.o differs
>>>>> warning: gcc/cc1-checksum.o differs
>>>>> Bootstrap comparison failure!
>>>>> gcc/ira-costs.o differs
>>>>> gcc/tree-sra.o differs
>>>>> gcc/tree-parloops.o differs
>>>>> gcc/tree-vect-data-refs.o differs
>>>>> gcc/java/jcf-io.o differs
>>>>> gcc/ipa-inline-analysis.o differs
>>>>
>>>> The bootstrap comparison failure on ppc64le, aarch64, and possibly
>>>> others is due to the order of some sections being in a different
>> order
>>>> with and without debugging.
>>>>
>>>> Stage2 is being compiled with no debugging due to -gtoggle, and
>> stage3
>>>> is being compiled with debugging.
>>>>
>>>> For ira-costs.o on ppc64le we have:
>>>>
>>>> -Disassembly of section
>>>>
>> .rodata._ZN10hash_tableI19cost_classes_hasher11xcallocatorE6expandEv.str1.8:
>>>> +Disassembly of section
>>>>
>> .rodata._ZN10hash_tableI19cost_classes_hasher11xcallocatorE26find_empty_slot_for_expandEj.str1.8:
>>>>
>>>> ...
>>>>
>>>> -Disassembly of section
>>>>
>> .rodata._ZN10hash_tableI19cost_classes_hasher11xcallocatorE26find_empty_slot_for_expandEj.str1.8:
>>>> +Disassembly of section
>>>>
>> .rodata._ZN10hash_tableI19cost_classes_hasher11xcallocatorE6expandEv.str1.8:
>>>>
>>>> There is no semantic difference between the objects, just the
>> ordering.
>>>>
>>>> I assume it's the same problem for the rest of the objects and
>>>> architectures.
>>>>
>>>> I will look into this, unless someone beats me to it, or has an idea
>>>> right off the bat.
>>>
>>> Check whether the symbol table walkers are walking hash tables.  I
>> assume the above are emitted via the symbol removal handling for debug
>> stuff?
>>
>> Ughh, indeed.  These sections are being outputted from
>> output_object_blocks which traverses a hash table:
>>
>> void
>> output_object_blocks (void)
>> {
>>   object_block_htab->traverse<void *, output_object_block_htab> (NULL);
>> }
>>
>> Perhaps we should sort them by some deterministic field and then call
>> output_object_block() on each member of the resulting list?
>
> Yes, that would be the usual fix. Maybe sth has an UID already, is the 'object' a decl by chance?

The attached patch fixes the bootstrap failure on ppc64le, and 
theoretically the aarch64 problem as well, but I haven't checked.

Tested on ppc64le linux by bootstrapping, and regtesting C/C++ against 
pre debug-early merge sources.  Also tested by a full bootstrap and 
regtest on x86-64 Linux.

OK for mainline?

Aldy
* varasm.c (output_object_block_htab): Push each object_block into
	a vector instead of calling output_object_block.
	(output_object_block_compare): New.
	(output_object_blocks): Sort object_blocks and then output them.

Comments

Richard Biener June 8, 2015, 8:26 a.m. UTC | #1
On Mon, Jun 8, 2015 at 3:23 AM, Aldy Hernandez <aldyh@redhat.com> wrote:
> On 06/07/2015 02:33 PM, Richard Biener wrote:
>>
>> On June 7, 2015 6:00:05 PM GMT+02:00, Aldy Hernandez <aldyh@redhat.com>
>> wrote:
>>>
>>> On 06/07/2015 11:25 AM, Richard Biener wrote:
>>>>
>>>> On June 7, 2015 5:03:30 PM GMT+02:00, Aldy Hernandez
>>>
>>> <aldyh@redhat.com> wrote:
>>>>>
>>>>> On 06/06/2015 05:49 AM, Andreas Schwab wrote:
>>>>>>
>>>>>> Bootstrap fails on aarch64:
>>>>>>
>>>>>> Comparing stages 2 and 3
>>>>>> warning: gcc/cc1objplus-checksum.o differs
>>>>>> warning: gcc/cc1obj-checksum.o differs
>>>>>> warning: gcc/cc1plus-checksum.o differs
>>>>>> warning: gcc/cc1-checksum.o differs
>>>>>> Bootstrap comparison failure!
>>>>>> gcc/ira-costs.o differs
>>>>>> gcc/tree-sra.o differs
>>>>>> gcc/tree-parloops.o differs
>>>>>> gcc/tree-vect-data-refs.o differs
>>>>>> gcc/java/jcf-io.o differs
>>>>>> gcc/ipa-inline-analysis.o differs
>>>>>
>>>>>
>>>>> The bootstrap comparison failure on ppc64le, aarch64, and possibly
>>>>> others is due to the order of some sections being in a different
>>>
>>> order
>>>>>
>>>>> with and without debugging.
>>>>>
>>>>> Stage2 is being compiled with no debugging due to -gtoggle, and
>>>
>>> stage3
>>>>>
>>>>> is being compiled with debugging.
>>>>>
>>>>> For ira-costs.o on ppc64le we have:
>>>>>
>>>>> -Disassembly of section
>>>>>
>>>
>>> .rodata._ZN10hash_tableI19cost_classes_hasher11xcallocatorE6expandEv.str1.8:
>>>>>
>>>>> +Disassembly of section
>>>>>
>>>
>>> .rodata._ZN10hash_tableI19cost_classes_hasher11xcallocatorE26find_empty_slot_for_expandEj.str1.8:
>>>>>
>>>>>
>>>>> ...
>>>>>
>>>>> -Disassembly of section
>>>>>
>>>
>>> .rodata._ZN10hash_tableI19cost_classes_hasher11xcallocatorE26find_empty_slot_for_expandEj.str1.8:
>>>>>
>>>>> +Disassembly of section
>>>>>
>>>
>>> .rodata._ZN10hash_tableI19cost_classes_hasher11xcallocatorE6expandEv.str1.8:
>>>>>
>>>>>
>>>>> There is no semantic difference between the objects, just the
>>>
>>> ordering.
>>>>>
>>>>>
>>>>> I assume it's the same problem for the rest of the objects and
>>>>> architectures.
>>>>>
>>>>> I will look into this, unless someone beats me to it, or has an idea
>>>>> right off the bat.
>>>>
>>>>
>>>> Check whether the symbol table walkers are walking hash tables.  I
>>>
>>> assume the above are emitted via the symbol removal handling for debug
>>> stuff?
>>>
>>> Ughh, indeed.  These sections are being outputted from
>>> output_object_blocks which traverses a hash table:
>>>
>>> void
>>> output_object_blocks (void)
>>> {
>>>   object_block_htab->traverse<void *, output_object_block_htab> (NULL);
>>> }
>>>
>>> Perhaps we should sort them by some deterministic field and then call
>>> output_object_block() on each member of the resulting list?
>>
>>
>> Yes, that would be the usual fix. Maybe sth has an UID already, is the
>> 'object' a decl by chance?
>
>
> The attached patch fixes the bootstrap failure on ppc64le, and theoretically
> the aarch64 problem as well, but I haven't checked.
>
> Tested on ppc64le linux by bootstrapping, and regtesting C/C++ against pre
> debug-early merge sources.  Also tested by a full bootstrap and regtest on
> x86-64 Linux.
>
> OK for mainline?

Please use FOR_EACH_HASH_TABLE_ELEMENT to put elements on the
vector instead of the htab traversal.

The compare function looks like we will end up having many equal elements
(and thus random ordering on hosts where qsort doesn't behave "sane"
here, like Solaris IIRC).  Unless all sections are named (which it looks like)
and we have only one object block per section name (which it looks like).
Thus can you re-write the compare function to just

  return strcmp (p1->sect->named.name, p2->sect->named.name);

?  (maybe with an assert that SECTION_NAMED is set on both)

Ok with those changes.  Btw, for portability the compare function should
be a total ordering, thus return 0 only iff p1 == p2, otherwise it won't
fix the bug on hosts where qsort may change the order of equal comparing
elements non-deterministically (IIRC Solaris).

Thanks,
Richard.

> Aldy
diff mbox

Patch

diff --git a/gcc/varasm.c b/gcc/varasm.c
index 18f3eac..008360e 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -7420,22 +7420,57 @@  output_object_block (struct object_block *block)
     }
 }
 
-/* A htab_traverse callback used to call output_object_block for
-   each member of object_block_htab.  */
+/* An htab_traverse callback used to copy object_blocks into a vector.  */
 
 int
-output_object_block_htab (object_block **slot, void *)
+output_object_block_htab (object_block **slot, void *data)
 {
-  output_object_block (*slot);
+  vec<object_block *, va_heap> *v = (vec<object_block *, va_heap> *) data;
+  v->safe_push (*slot);
   return 1;
 }
 
+/* A callback for qsort to compare object_blocks.  We only care about
+   named sections.  */
+
+static int
+output_object_block_compare (const void *x, const void *y)
+{
+  object_block *p1 = *(object_block * const*)x;
+  object_block *p2 = *(object_block * const*)y;
+
+  if (p1->sect->common.flags & SECTION_NAMED
+      && !(p2->sect->common.flags & SECTION_NAMED))
+    return 1;
+
+  if (!(p1->sect->common.flags & SECTION_NAMED)
+      && p2->sect->common.flags & SECTION_NAMED)
+    return -1;
+
+  if (p1->sect->common.flags & SECTION_NAMED
+      && p2->sect->common.flags & SECTION_NAMED)
+    return strcmp (p1->sect->named.name,
+		   p2->sect->named.name);
+
+  return 0;
+}
+
 /* Output the definitions of all object_blocks.  */
 
 void
 output_object_blocks (void)
 {
-  object_block_htab->traverse<void *, output_object_block_htab> (NULL);
+  vec<object_block *, va_heap> v = vNULL;
+  object_block_htab->traverse<void *, output_object_block_htab> (&v);
+
+  /* Sort them in order to output them in a deterministic manner,
+     otherwise we may get .rodata sections in different orders with
+     and without -g.  */
+  v.qsort (output_object_block_compare);
+  unsigned i;
+  object_block *obj;
+  FOR_EACH_VEC_ELT (v, i, obj)
+    output_object_block (obj);
 }
 
 /* This function provides a possible implementation of the