[patch] fix _OBJC_Module defined but not used warning

[patch] fix _OBJC_Module defined but not used warning

[Aldy Hernandez]

[-- Attachment #1: Type: text/plain, Size: 855 bytes --]

On 06/07/2015 06:19 AM, Andreas Schwab wrote:
> Another fallout:
>
> FAIL: obj-c++.dg/try-catch-5.mm -fgnu-runtime (test for excess errors)
> Excess errors:
> <built-in>: warning: '_OBJC_Module' defined but not used [-Wunused-variable]

check_global_declarations is called for more symbols now.  All the 
defined but not used errors I've seen in development have been 
legitimate.  For tests, the tests should be fixed.  For built-ins such 
as these, does the attached fix the problem?

It is up to the objc maintainers, we can either fix this with the 
attached patch, or setting DECL_IN_SYSTEM_HEADER.

/* Nonzero for a given ..._DECL node means that no warnings should be
    generated just because this node is unused.  */
#define DECL_IN_SYSTEM_HEADER(NODE) \
   (in_system_header_at (DECL_SOURCE_LOCATION (NODE)))

Let me know what you prefer.
Aldy

[-- Attachment #2: curr --]
[-- Type: text/plain, Size: 855 bytes --]

commit 25ce72372f7b1309004b87810140573b422e1355
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Sun Jun 7 07:32:12 2015 -0400

    	* objc-runtime-shared-support.c (build_module_descriptor): Set
    	TREE_USED on UOBJC_MODULES_decl.

diff --git a/gcc/objc/objc-runtime-shared-support.c b/gcc/objc/objc-runtime-shared-support.c
index d9b3c27..1bcb14a 100644
--- a/gcc/objc/objc-runtime-shared-support.c
+++ b/gcc/objc/objc-runtime-shared-support.c
@@ -519,6 +519,9 @@ build_module_descriptor (long vers, tree attr)
      is referenced by the runtime and, therefore, needed.  */
   DECL_PRESERVE_P (UOBJC_MODULES_decl) = 1;
 
+  /* Squash `defined but not used' warning.  */
+  TREE_USED (UOBJC_MODULES_decl) = 1;
+
   /* Allow the runtime to mark meta-data such that it can be assigned to target
      specific sections by the back-end.  */
   if (attr)

Re: [patch] fix _OBJC_Module defined but not used warning

[Andreas Schwab]

Aldy Hernandez <aldyh@redhat.com> writes:

> check_global_declarations is called for more symbols now.  All the defined
> but not used errors I've seen in development have been legitimate.  For
> tests, the tests should be fixed.  For built-ins such as these, does the
> attached fix the problem?

Yes, it does.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [patch] fix _OBJC_Module defined but not used warning

[Iain Sandoe]

Hi Aldy,

On 7 Jun 2015, at 12:37, Aldy Hernandez wrote:

> On 06/07/2015 06:19 AM, Andreas Schwab wrote:
>> Another fallout:
>> 
>> FAIL: obj-c++.dg/try-catch-5.mm -fgnu-runtime (test for excess errors)
>> Excess errors:
>> <built-in>: warning: '_OBJC_Module' defined but not used [-Wunused-variable]
> 
> check_global_declarations is called for more symbols now.  All the defined but not used errors I've seen in development have been legitimate.  For tests, the tests should be fixed.  For built-ins such as these, does the attached fix the problem?
> 
> It is up to the objc maintainers, we can either fix this with the attached patch,

The current patch is OK.

> or setting DECL_IN_SYSTEM_HEADER.

This seems a better long-term idea; however, I would prefer to go through all the cases where it would be applicable (including for the NeXT runtime) and apply that change as a coherent patch.  At the moment dealing with the NeXT stuff is a bit hampered by pr66448.

thanks,
Iain

Re: [patch] fix _OBJC_Module defined but not used warning

[Aldy Hernandez]

On 06/08/2015 04:03 AM, Iain Sandoe wrote:
> Hi Aldy,
>
> On 7 Jun 2015, at 12:37, Aldy Hernandez wrote:
>
>> On 06/07/2015 06:19 AM, Andreas Schwab wrote:
>>> Another fallout:
>>>
>>> FAIL: obj-c++.dg/try-catch-5.mm -fgnu-runtime (test for excess errors)
>>> Excess errors:
>>> <built-in>: warning: '_OBJC_Module' defined but not used [-Wunused-variable]
>>
>> check_global_declarations is called for more symbols now.  All the defined but not used errors I've seen in development have been legitimate.  For tests, the tests should be fixed.  For built-ins such as these, does the attached fix the problem?
>>
>> It is up to the objc maintainers, we can either fix this with the attached patch,
>
> The current patch is OK.

Committed.

>
>> or setting DECL_IN_SYSTEM_HEADER.
>
> This seems a better long-term idea; however, I would prefer to go through all the cases where it would be applicable (including for the NeXT runtime) and apply that change as a coherent patch.  At the moment dealing with the NeXT stuff is a bit hampered by pr66448.

On my list next.

Aldy

Re: debug-early branch merged into mainline

[Aldy Hernandez]

[-- Attachment #1: Type: text/plain, Size: 2932 bytes --]

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

[-- Attachment #2: curr --]
[-- Type: text/plain, Size: 2221 bytes --]

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

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

Re: debug-early branch merged into mainline

[Richard Biener]

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

Re: debug-early branch merged into mainline

[Aldy Hernandez]

[-- Attachment #1: Type: text/plain, Size: 3991 bytes --]

On 06/08/2015 04:26 AM, Richard Biener wrote:
> 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)

Some sections are not named.

How about we sort the named sections and output them, but call 
output_object_block() on the rest of the sections on whatever order they 
were in?  This solves the bootstrap problem as well.

Attached patch tested on x86-64 and ppc64le Linux.

OK?

Aldy

[-- Attachment #2: curr --]
[-- Type: text/plain, Size: 2291 bytes --]

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index e1bd305..f6d4bda 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,10 @@
+2015-06-07  Aldy Hernandez  <aldyh@redhat.com>
+
+	* varasm.c (output_object_block_htab): Remove.
+	(output_object_block_compare): New.
+	(output_object_blocks): Sort named object_blocks before outputting
+	them.
+
 2015-06-06  Jan Hubicka  <hubicka@ucw.cz>
 
 	* alias.c (get_alias_set): Be ready for TYPE_CANONICAL
diff --git a/gcc/varasm.c b/gcc/varasm.c
index 18f3eac..a765278 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -7420,14 +7420,18 @@ output_object_block (struct object_block *block)
     }
 }
 
-/* A htab_traverse callback used to call output_object_block for
-   each member of object_block_htab.  */
+/* A callback for qsort to compare object_blocks.  */
 
-int
-output_object_block_htab (object_block **slot, void *)
+static int
+output_object_block_compare (const void *x, const void *y)
 {
-  output_object_block (*slot);
-  return 1;
+  object_block *p1 = *(object_block * const*)x;
+  object_block *p2 = *(object_block * const*)y;
+
+  gcc_assert (p1->sect->common.flags & SECTION_NAMED
+	      && p2->sect->common.flags & SECTION_NAMED);
+
+  return strcmp (p1->sect->named.name, p2->sect->named.name);
 }
 
 /* Output the definitions of all object_blocks.  */
@@ -7435,7 +7439,25 @@ output_object_block_htab (object_block **slot, void *)
 void
 output_object_blocks (void)
 {
-  object_block_htab->traverse<void *, output_object_block_htab> (NULL);
+  vec<object_block *, va_heap> v = vNULL;
+  object_block *obj;
+  hash_table<object_block_hasher>::iterator hi;
+
+  FOR_EACH_HASH_TABLE_ELEMENT (*object_block_htab, obj, object_block *, hi)
+    if (obj->sect->common.flags & SECTION_NAMED)
+      v.safe_push (obj);
+
+  /* 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;
+  FOR_EACH_VEC_ELT (v, i, obj)
+    output_object_block (obj);
+
+  FOR_EACH_HASH_TABLE_ELEMENT (*object_block_htab, obj, object_block *, hi)
+    if (!(obj->sect->common.flags & SECTION_NAMED))
+      output_object_block (obj);
 }
 
 /* This function provides a possible implementation of the

Re: debug-early branch merged into mainline

[Richard Biener]

On Mon, Jun 8, 2015 at 2:05 PM, Aldy Hernandez <aldyh@redhat.com> wrote:
> On 06/08/2015 04:26 AM, Richard Biener wrote:
>>
>> 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)
>
>
> Some sections are not named.
>
> How about we sort the named sections and output them, but call
> output_object_block() on the rest of the sections on whatever order they
> were in?  This solves the bootstrap problem as well.
>
> Attached patch tested on x86-64 and ppc64le Linux.
>
> OK?

No, but hash_section suggests to sort after sect->common.flags if
the section is not named.  Conveniently flags is just an 'int' ...

Can you adjust again?

Thanks,
Richard.

> Aldy

Re: debug-early branch merged into mainline

[Aldy Hernandez]

On 06/08/2015 09:30 AM, Richard Biener wrote:
> On Mon, Jun 8, 2015 at 2:05 PM, Aldy Hernandez <aldyh@redhat.com> wrote:
>> On 06/08/2015 04:26 AM, Richard Biener wrote:
>>>
>>> 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)
>>
>>
>> Some sections are not named.
>>
>> How about we sort the named sections and output them, but call
>> output_object_block() on the rest of the sections on whatever order they
>> were in?  This solves the bootstrap problem as well.
>>
>> Attached patch tested on x86-64 and ppc64le Linux.
>>
>> OK?
>
> No, but hash_section suggests to sort after sect->common.flags if
> the section is not named.  Conveniently flags is just an 'int' ...

What about if the comparison routine gets a named section and an unnamed 
section?  How to compare?  That's why I was giving priority to one over 
the other originally, but I didn't know about problematic qsort 
implementations.

Aldy

Re: debug-early branch merged into mainline

[Richard Biener]

On June 8, 2015 7:14:19 PM GMT+02:00, Aldy Hernandez <aldyh@redhat.com> wrote:
>On 06/08/2015 09:30 AM, Richard Biener wrote:
>> On Mon, Jun 8, 2015 at 2:05 PM, Aldy Hernandez <aldyh@redhat.com>
>wrote:
>>> On 06/08/2015 04:26 AM, Richard Biener wrote:
>>>>
>>>> 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)
>>>
>>>
>>> Some sections are not named.
>>>
>>> How about we sort the named sections and output them, but call
>>> output_object_block() on the rest of the sections on whatever order
>they
>>> were in?  This solves the bootstrap problem as well.
>>>
>>> Attached patch tested on x86-64 and ppc64le Linux.
>>>
>>> OK?
>>
>> No, but hash_section suggests to sort after sect->common.flags if
>> the section is not named.  Conveniently flags is just an 'int' ...
>
>What about if the comparison routine gets a named section and an
>unnamed 
>section?  How to compare?  That's why I was giving priority to one over
>
>the other originally, but I didn't know about problematic qsort 
>implementations.

Obviously unnamed and a named section can be sorted like you did in the original patch.

Richard.

>Aldy

Re: debug-early branch merged into mainline

[Aldy Hernandez]

[-- Attachment #1: Type: text/plain, Size: 813 bytes --]

On 06/08/2015 02:59 PM, Richard Biener wrote:
> On June 8, 2015 7:14:19 PM GMT+02:00, Aldy Hernandez <aldyh@redhat.com> wrote:
>> On 06/08/2015 09:30 AM, Richard Biener wrote:
>>> On Mon, Jun 8, 2015 at 2:05 PM, Aldy Hernandez <aldyh@redhat.com>
>> wrote:
>>>> On 06/08/2015 04:26 AM, Richard Biener wrote:
>>>>>
>>>>> On Mon, Jun 8, 2015 at 3:23 AM, Aldy Hernandez <aldyh@redhat.com>

>> What about if the comparison routine gets a named section and an
>> unnamed
>> section?  How to compare?  That's why I was giving priority to one over
>>
>> the other originally, but I didn't know about problematic qsort
>> implementations.
>
> Obviously unnamed and a named section can be sorted like you did in the original patch.

Obviously I'm not understanding :).

How about this?

Tested on x86-64 and ppc64le.

Aldy

[-- Attachment #2: curr --]
[-- Type: text/plain, Size: 2424 bytes --]

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index e1bd305..f6d4bda 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,10 @@
+2015-06-07  Aldy Hernandez  <aldyh@redhat.com>
+
+	* varasm.c (output_object_block_htab): Remove.
+	(output_object_block_compare): New.
+	(output_object_blocks): Sort named object_blocks before outputting
+	them.
+
 2015-06-06  Jan Hubicka  <hubicka@ucw.cz>
 
 	* alias.c (get_alias_set): Be ready for TYPE_CANONICAL
diff --git a/gcc/varasm.c b/gcc/varasm.c
index 18f3eac..d69ba5a 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -7420,14 +7420,29 @@ output_object_block (struct object_block *block)
     }
 }
 
-/* A htab_traverse callback used to call output_object_block for
-   each member of object_block_htab.  */
+/* A callback for qsort to compare object_blocks.  */
 
-int
-output_object_block_htab (object_block **slot, void *)
+static int
+output_object_block_compare (const void *x, const void *y)
 {
-  output_object_block (*slot);
-  return 1;
+  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);
+
+  unsigned f1 = p1->sect->common.flags;
+  unsigned f2 = p2->sect->common.flags;
+  return f1 < f2 ? -1 : (f1 > f2 ? 1 : 0);
 }
 
 /* Output the definitions of all object_blocks.  */
@@ -7435,7 +7450,20 @@ output_object_block_htab (object_block **slot, void *)
 void
 output_object_blocks (void)
 {
-  object_block_htab->traverse<void *, output_object_block_htab> (NULL);
+  vec<object_block *, va_heap> v = vNULL;
+  object_block *obj;
+  hash_table<object_block_hasher>::iterator hi;
+
+  FOR_EACH_HASH_TABLE_ELEMENT (*object_block_htab, obj, object_block *, hi)
+    v.safe_push (obj);
+
+  /* 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;
+  FOR_EACH_VEC_ELT (v, i, obj)
+    output_object_block (obj);
 }
 
 /* This function provides a possible implementation of the

Re: debug-early branch merged into mainline

[Richard Biener]

On Mon, Jun 8, 2015 at 10:25 PM, Aldy Hernandez <aldyh@redhat.com> wrote:
> On 06/08/2015 02:59 PM, Richard Biener wrote:
>>
>> On June 8, 2015 7:14:19 PM GMT+02:00, Aldy Hernandez <aldyh@redhat.com>
>> wrote:
>>>
>>> On 06/08/2015 09:30 AM, Richard Biener wrote:
>>>>
>>>> On Mon, Jun 8, 2015 at 2:05 PM, Aldy Hernandez <aldyh@redhat.com>
>>>
>>> wrote:
>>>>>
>>>>> On 06/08/2015 04:26 AM, Richard Biener wrote:
>>>>>>
>>>>>>
>>>>>> On Mon, Jun 8, 2015 at 3:23 AM, Aldy Hernandez <aldyh@redhat.com>
>
>
>>> What about if the comparison routine gets a named section and an
>>> unnamed
>>> section?  How to compare?  That's why I was giving priority to one over
>>>
>>> the other originally, but I didn't know about problematic qsort
>>> implementations.
>>
>>
>> Obviously unnamed and a named section can be sorted like you did in the
>> original patch.
>
>
> Obviously I'm not understanding :).
>
> How about this?

Ok with adding

v.create (object_block_htab->elements ());

and using v.quick_push () (avoids re-allocations)

and with adding a

v.release ();

at the end of the function.  And re-writing

+  return f1 < f2 ? -1 : (f1 > f2 ? 1 : 0);

to

   if (f1 == f2)
     return 0;
   return f1 < f2 ? -1 : 1;

Thanks,
Richard.

> Tested on x86-64 and ppc64le.
>
> Aldy

Re: debug-early branch merged into mainline

[Aldy Hernandez]

[-- Attachment #1: Type: text/plain, Size: 1324 bytes --]

On 06/09/2015 04:00 AM, Richard Biener wrote:
> On Mon, Jun 8, 2015 at 10:25 PM, Aldy Hernandez <aldyh@redhat.com> wrote:
>> On 06/08/2015 02:59 PM, Richard Biener wrote:
>>>
>>> On June 8, 2015 7:14:19 PM GMT+02:00, Aldy Hernandez <aldyh@redhat.com>
>>> wrote:
>>>>
>>>> On 06/08/2015 09:30 AM, Richard Biener wrote:
>>>>>
>>>>> On Mon, Jun 8, 2015 at 2:05 PM, Aldy Hernandez <aldyh@redhat.com>
>>>>
>>>> wrote:
>>>>>>
>>>>>> On 06/08/2015 04:26 AM, Richard Biener wrote:
>>>>>>>
>>>>>>>
>>>>>>> On Mon, Jun 8, 2015 at 3:23 AM, Aldy Hernandez <aldyh@redhat.com>
>>
>>
>>>> What about if the comparison routine gets a named section and an
>>>> unnamed
>>>> section?  How to compare?  That's why I was giving priority to one over
>>>>
>>>> the other originally, but I didn't know about problematic qsort
>>>> implementations.
>>>
>>>
>>> Obviously unnamed and a named section can be sorted like you did in the
>>> original patch.
>>
>>
>> Obviously I'm not understanding :).
>>
>> How about this?
>
> Ok with adding

I've committed the attached patch.

>
> v.create (object_block_htab->elements ());
>
> and using v.quick_push () (avoids re-allocations)
>
> and with adding a
>
> v.release ();

For some reason I thought the new C++ vector world released stuff on its 
own if allocated on the heap.  Oh well...

Thanks.
Aldy

[-- Attachment #2: curr --]
[-- Type: text/plain, Size: 2356 bytes --]

commit d2f1a9cd6e91c400ab4fa569565eece1ae08b64d
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Tue Jun 9 05:39:34 2015 -0400

    	* varasm.c (output_object_block_htab): Remove.
    	(output_object_block_compare): New.
    	(output_object_blocks): Sort named object_blocks before outputting
    	them.

diff --git a/gcc/varasm.c b/gcc/varasm.c
index 18f3eac..86a70db 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -7420,14 +7420,31 @@ output_object_block (struct object_block *block)
     }
 }
 
-/* A htab_traverse callback used to call output_object_block for
-   each member of object_block_htab.  */
+/* A callback for qsort to compare object_blocks.  */
 
-int
-output_object_block_htab (object_block **slot, void *)
+static int
+output_object_block_compare (const void *x, const void *y)
 {
-  output_object_block (*slot);
-  return 1;
+  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);
+
+  unsigned f1 = p1->sect->common.flags;
+  unsigned f2 = p2->sect->common.flags;
+  if (f1 == f2)
+    return 0;
+  return f1 < f2 ? -1 : 1;
 }
 
 /* Output the definitions of all object_blocks.  */
@@ -7435,7 +7452,23 @@ output_object_block_htab (object_block **slot, void *)
 void
 output_object_blocks (void)
 {
-  object_block_htab->traverse<void *, output_object_block_htab> (NULL);
+  vec<object_block *, va_heap> v;
+  v.create (object_block_htab->elements ());
+  object_block *obj;
+  hash_table<object_block_hasher>::iterator hi;
+
+  FOR_EACH_HASH_TABLE_ELEMENT (*object_block_htab, obj, object_block *, hi)
+    v.quick_push (obj);
+
+  /* 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;
+  FOR_EACH_VEC_ELT (v, i, obj)
+    output_object_block (obj);
+
+  v.release ();
 }
 
 /* This function provides a possible implementation of the