On Thu, Mar 9, 2017 at 4:07 PM, Olivier Hainque <hain...@adacore.com> wrote:
> Hello,
>
> The mainline compiler, configured for powerpc-wrs-vxworks, fails
> to handle the simple test below at -O2.
>
> The failure mode looks like:
>
> ./gnat1 -Iada/rts opt64_pkg.adb -O2
> ...
> opt64_pkg.adb: In function 'Opt64_Pkg.Encode':
> opt64_pkg.adb:14:1: error: alignment of array elements is greater than
> element size
>
> The error is spurious, caused by a misinteraction between tree-sra
> and tree-switch-conversion.
>
> SRA first transforms the case statement into:
>
> Created a replacement for result offset: 0, size: 8: result$1
> ...
>
> Opt64_Pkg.Encode (const integer x)
> {
> character result$1;
> character result[1:1];
>
> <bb 2> [0.00%]:
> switch (x_2(D)) <default: <L3> [0.00%], case 1: <L0> [0.00%], case 2: <L1>
> [0.00%], case 3: <L2> [0.00%]>
>
> <L0> [0.00%]:
> result$1_13 = MEM[(character[1:1] *)"1"];
> goto <bb 7>; [0.00%]
> ...
> <L3> [0.00%]:
> result$1_14 = MEM[(character[1:1] *)"?"];
>
> <bb 7> [0.00%]:
> # result$1_10 = PHI <result$1_13(3), result$1_12(4), result$1_11(5),
> result$1_14(6)>
> MEM[(character[1:1] *)&opt64_pkg__last_hash] = result$1_10;
> return;
>
> with a particularity on the MEM references for loads: their
> type are of QI mode with 32bit alignment, from
>
> build_ref_for_offset (...)
> { ...
> get_object_alignment_1 (base, &align, &misalign);
> ...
> if (align != TYPE_ALIGN (exp_type))
> exp_type = build_aligned_type (exp_type, align);
> ...
>
> Indeed, align is assigned through
>
> get_object_alignment_2 (...)
> ...
> else if (TREE_CODE (exp) == STRING_CST)
> {
> /* STRING_CST are the only constant objects we allow to be not
> wrapped inside a CONST_DECL. */
> align = TYPE_ALIGN (TREE_TYPE (exp));
> if (CONSTANT_CLASS_P (exp))
> align = (unsigned) CONSTANT_ALIGNMENT (exp, align);
>
> and rs6000.h has:
>
> /* Make strings word-aligned so strcpy from constants will be faster. */
> #define CONSTANT_ALIGNMENT(EXP, ALIGN) \
> ...
>
> Later on, the switch-conversion pass quicks in and decides to make an array
> of the possible values assigned to "result". The chosen array element type is
> one altered by SRA (QI mode, align 32) and stor-layout complains.
>
> The over-alignment is allowed by SRA on purpose, not to interfere with
> vectorisation - see PR65310 & PR50819.
>
> The attached patch is a suggestion to fix this by teaching switch-conversion
> about this possibility and adjust the alignment back to the first value lower
> than size for the type used for array elements.
>
> Bootstrapped and regtested on x86_64-linux. Verified that it cures the
> spurious error on powerpc and that the resolution to PR50819 isn't affected.
>
> The change triggering the misinteraction is the one allowing SRA to produce
> types with larger alignments, introduced on Mar 2015 for PR 65310, to fix a
> regression a previous change on the PR exposed against PR 50819.
>
> We see the the spurious error on our gcc-6 based compilers, not on those based
> on gcc-4.9 so this qualifies as a regression I think.
>
> OK to commit ?
I think a simpler fix, doing
type = TYPE_MAIN_VARIANT (type);
should work equally well.
Richard.
> Thanks in advance!
>
> With Kind Regards,
>
> Olivier
>
> 2017-03-09 Olivier Hainque <hain...@adacore.com>
>
> * tree-switch-conversion (array_value_type): Start by adjusting the
> alignment of the candidate type back to a value lower than its size.
>
> gnat.dg/
> * opt64.ad[bs], opt64_pkg.ad[bs]: New test.
>
> --
>
> package body Opt64_PKG is
>
> procedure Encode (X : Integer) is
> result : Hash;
> begin
> case X is
> when 1 => result := "1";
> when 2 => result := "2";
> when 3 => result := "3";
> when others => Result := "?";
> end case;
> Last_Hash := Result;
> end;
> end;
>
> package Opt64_PKG is
> type Hash is new string (1 .. 1);
> Last_Hash : Hash;
>
> procedure Encode (X : Integer);
> end;
>
