On Tue, Aug 14, 2018 at 09:34:08PM -0500, Martin Sebor wrote:
> 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 <mse...@gmail.com> wrote:
> >>>>>> On 08/07/2018 02:57 AM, Jason Merrill wrote:
> >>>>>>>
> >>>>>>> On Wed, Aug 1, 2018 at 12:49 AM, Martin Sebor <mse...@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 =
This fix seems correct to me, the poly types are not strings. Looking at
other uses of TYPE_STRING_FLAG this change doesn't seem like it would have
impact on parsing or code generation.
OK for trunk.
> >>> 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?
These tests aren't looking at performance, just expecting to see certain
instructions emitted. The only change is that now the int8x16_t forms are
inlined (so the scan-assembler-times fails with two matches, one expected,
one in the inlined function body copy).
The difference seems to be in the initialization cost of the input data set.
Before your patch:
int8_tD.3359 test_set0D.21541[8];
int8_tD.3359 answ_set0D.21542[8];
test_set0D.21541[0] = 0;
test_set0D.21541[1] = 1;
test_set0D.21541[2] = -1;
test_set0D.21541[3] = 10;
test_set0D.21541[4] = -10;
test_set0D.21541[5] = 0;
test_set0D.21541[6] = 127;
test_set0D.21541[7] = -128;
answ_set0D.21542[0] = 0;
answ_set0D.21542[1] = -1;
answ_set0D.21542[2] = 1;
answ_set0D.21542[3] = -10;
answ_set0D.21542[4] = 10;
answ_set0D.21542[5] = 0;
answ_set0D.21542[6] = -127;
answ_set0D.21542[7] = -128;
After your patch:
int8_tD.3357 test_set0D.21539[8];
int8_tD.3357 answ_set0D.21540[8];
test_set0D.21539 = "";
answ_set0D.21540 = "";
I think that is probably what you expected to happen; but the impact on
inlining might not have been. Probably, we want to just change these tests
to explicitly disable inlining. The tests appear to execute correctly.
The print in the dump file is a bit unusual; presumably the impact of having
non-printing characters in my initializer list - but less helpful output for
it.
Off topic; these tests are quick to copmpile on my cross and native
compilers. Do you have additional checking enabled?
Thanks,
James
>
> 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
