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 ?

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;

Attachment: switch-conv-align.diff
Description: Binary data

Attachment: switch-conv-align-test.diff
Description: Binary data

Reply via email to