On 10/08/18 13:14, Rainer Orth wrote:
> Hi Bernd,
>
>> On 10/05/18 20:15, Andreas Schwab wrote:
>>> On Sep 14 2018, Bernd Edlinger <bernd.edlin...@hotmail.de> wrote:
>>>
>>>> diff -Npur gcc/testsuite/gnat.dg/string_merge1.adb
>>>> gcc/testsuite/gnat.dg/string_merge1.adb
>>>> --- gcc/testsuite/gnat.dg/string_merge1.adb 1970-01-01
>>>> 01:00:00.000000000 +0100
>>>> +++ gcc/testsuite/gnat.dg/string_merge1.adb 2018-08-26
>>>> 16:31:12.650271931 +0200
>>>> @@ -0,0 +1,19 @@
>>>> +-- { dg-do compile }
>>>> +-- { dg-options "-O1 -fmerge-all-constants" }
>>>> +
>>>> +procedure String_Merge1 is
>>>> + procedure Process (X : String);
>>>> + pragma Import (Ada, Process);
>>>> +begin
>>>> + Process ("ABCD");
>>>> +end;
>>>> +
>>>> +-- We expect something like:
>>>> +
>>>> +-- .section .rodata.str1.1,"aMS",@progbits,1
>>>> +-- .LC1:
>>>> +-- .string "ABCD"
>>>> +
>>>> +-- { dg-final { scan-assembler-times "\\.rodata\\.str" 1 } }
>>>> +-- { dg-final { scan-assembler-times "\\.string" 1 } }
>>>> +-- { dg-final { scan-assembler-times "\"ABCD\"" 1 } }
>>>
>>> FAIL: gnat.dg/string_merge1.adb scan-assembler-times \\.string 1
>>>
>>> $ grep ABCD string_merge1.s
>>> stringz "ABCD"
>>>
>>
>> Ah, thanks.
>>
>> Turns out there are too much variations, like mentioned stringz, and asciz,
>> and
>> probably lots more here.
>>
>> But for the purpose of testing the optimization it should be sufficient to
>> look for
>> ".rodata.str" in the assembler.
>>
>> So I committed the following as obvious:
>
> unfortunately that patch is not enough and still shows a fundamental
> problem:
>
> * The tests still FAIL on Solaris with /bin/as and Darwin:
>
> FAIL: gcc.dg/merge-all-constants-2.c scan-assembler-not \\\\.rodata[\\n\\r]
>
> FAIL: gnat.dg/string_merge1.adb scan-assembler-times \\\\.rodata\\\\.str 1
> FAIL: gnat.dg/string_merge2.adb scan-assembler-times \\\\.rodata\\\\.str 1
>
> Both targets lack string merging support, so the tests should check
> for that.
>
> * The merge-all-constants-2.c test doesn't FAIL on Solaris/SPARC with
> /bin/as, although it lacks string merging support, too. The assembler
> output contains
>
> .section ".rodata"
>
> so the pattern currently used to check for .rodata is too
> restrictive. There is assembler syntax beyond gas on x86 ;-)
>
For the test that failed with the quotes around .rodata. I think
instead of looking for a end of line immediately after .rodata, it
would be sufficient to make sure it does not continue with .str, so
could you please try to add something like the following to your patch?
Index: gcc/testsuite/gcc.dg/merge-all-constants-2.c
===================================================================
--- gcc/testsuite/gcc.dg/merge-all-constants-2.c (revision 264888)
+++ gcc/testsuite/gcc.dg/merge-all-constants-2.c (working copy)
@@ -5,4 +5,4 @@
const char str2[37] = "0123456789abcdefghijklmnopqrstuvwxyz";
const char str3[10] = "0123456789abcdefghijklmnopqrstuvwxyz";
-/* { dg-final { scan-assembler-not "\\.rodata\[\n\r\]" } } */
+/* { dg-final { scan-assembler-not "\\.rodata\[^.]" } } */
> The following patch fixes the former issue; tested on
> sparc-sun-solaris2.11 with as (no string merging) resp. gas (with string
> merging). Installed on mainline.
>
> Besides, the patch seems to have produced more fallout on Solaris: I see
> many new Go testsuite failures on Solaris 10 which probably are related,
> and Solaris bootstrap with Ada included is broken due to the extended
> usage of string merging. I'm currently investigating what's going on
> there.
>
> Rainer
>
