Re: [PATCH] convert braced initializers to strings (PR 71625)

[Martin Sebor <msebor@gmail.com>]

On 08/14/2018 09:24 AM, Martin Sebor wrote:
> On 08/14/2018 09:08 AM, Martin Sebor wrote:
>> On 08/14/2018 07:27 AM, James Greenhalgh wrote:
>>> On Wed, Aug 08, 2018 at 07:17:07PM -0500, Martin Sebor wrote:
>>>> On 08/08/2018 05:08 AM, Jason Merrill wrote:
>>>>> On Wed, Aug 8, 2018 at 9:04 AM, Martin Sebor <msebor@gmail.com> wrote:
>>>>>> On 08/07/2018 02:57 AM, Jason Merrill wrote:
>>>>>>>
>>>>>>> On Wed, Aug 1, 2018 at 12:49 AM, Martin Sebor <msebor@gmail.com>
>>>>>>> wrote:
>>>>>>>>
>>>>>>>> On 07/31/2018 07:38 AM, Jason Merrill wrote:
>>>
>>> <snip>
>>>
>>>> Done in the attached patch.  I've also avoided dealing with
>>>> zero-length arrays and added tests to make sure their size
>>>> stays is regardless of the form of their initializer and
>>>> the appropriate warnings are issued.
>>>>
>>>> Using build_string() rather than build_string_literal() needed
>>>> a tweak in digest_init_r().  It didn't break anything but since
>>>> the array type may not have a domain yet, neither will the
>>>> string.  It looks like that may get adjusted later on but I've
>>>> temporarily guarded the code with #if 1.  If the change is
>>>> fine I'll remove the #if before committing.
>>>>
>>>> This initial patch only handles narrow character initializers
>>>> (i.e., those with TYPE_STRING_FLAG set).  Once this gets some
>>>> exposure I'd like to extend it to other character types,
>>>> including wchar_t.
>>>
>>> Hi Martin,
>>>
>>> This causes issues for the AArch64 tests (full list below).
>>>
>>> I see an error message on the following construct:
>>>
>>>   void foo (void)
>>>   {
>>>     __Poly8_t x[4] = { 1, 2, 3, 4 };
>>>   }
>>>
>>>   init.c:3:20: error: array of inappropriate type initialized from
>>> string constant
>>>   3 |   __Poly8_t x[4] = { 1, 2, 3, 4 };
>>>     |
>>>
>>> __Poly8_t is a type we define in our backend, through a convoluted
>>> set of
>>> functions, which operates a lot like an unsigned, QI mode type.
>>
>> I see the error with my aarch64 cross-compiler .  The new code
>> that does the conversion of array initializers to STRING_CSTs
>> looks for the TYPE_STRING_FLAG() to be set on the type of
>> the array elements.  Perhaps __Poly8_t should not have the flag
>> set?  (If it needs it then I think we'd have to only consider
>> named character types.)
>
> The change below gets rid of the compilation error.  I don't
> know if it's appropriate for the aarch64 back end:
>
> Index: gcc/config/aarch64/aarch64-builtins.c
> ===================================================================
> --- gcc/config/aarch64/aarch64-builtins.c    (revision 263537)
> +++ gcc/config/aarch64/aarch64-builtins.c    (working copy)
> @@ -643,6 +643,7 @@ aarch64_init_simd_builtin_types (void)
>    /* Poly types are a world of their own.  */
>    aarch64_simd_types[Poly8_t].eltype = aarch64_simd_types[Poly8_t].itype =
>      build_distinct_type_copy (unsigned_intQI_type_node);
> +  TYPE_STRING_FLAG (aarch64_simd_types[Poly8_t].eltype) = false;
>    aarch64_simd_types[Poly16_t].eltype =
> aarch64_simd_types[Poly16_t].itype =
>      build_distinct_type_copy (unsigned_intHI_type_node);
>    aarch64_simd_types[Poly64_t].eltype =
> aarch64_simd_types[Poly64_t].itype =
>
>
>>> A second set of tests fail due to changed inlining behaviour for
>>> functions
>>> with char array initialization:
>>>
>>>   gcc.target/aarch64/vset_lane_1.c
>>>   gcc.target/aarch64/vneg_s.c
>>>   gcc.target/aarch64/vclz.c
>>
>> I'm not sure what's going on here.  The tests are very big and
>> take forever to compile with an aarch64 cross-compiler, and I'm
>> not sure what to look for.  Can you provide a smaller test case
>> that shows the issue?

I wonder if these changes might be due to the same problem:
the tests define and initialize arrays of the Int8x16_t type
which is initialized to intQI_type_node, i.e., the signed
form of Poly8_t.  Does the conversion to STRING_CST cause
a performance degradation or is it just that the tests end
up with equivalent but slightly different assembly?

The tests also use int8_t and uint8_t for the expected results.
Those are typedefs for signed and unsigned char, respectively.
Is the conversion to strings for those fine?

Martin