[PATCH, i386]: Committed: Fix PR target/35496

[PATCH, i386]: Committed: Fix PR target/35496

[Uros Bizjak]

Hello!

We have to compute alignment of VECTOR_CST and INTEGER_CST constants via 
ALIGN_MODE_128 in the same way as other ALIGN_MODE_128 data is aligned.

2008-03-09  Uros Bizjak  <ubizjak@gmail.com>

        PR target/35496
        * config/i386/i386.c (ix86_constant_algnment): Compute alignment 
using
        ALIGN_MODE_128 for VECTOR_CST and INTEGER_CST in addition to 
REAL_CST.

Patch was bootstrapped and regression tested on x86_64-pc-linux-gnu 
{,-m32}. Patch is committed to mainline, but I think that it should be 
committed to 4.3 branch as well.

Uros.

Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c  (revision 133051)
+++ config/i386/i386.c  (working copy)
@@ -16789,7 +16789,8 @@
 int
 ix86_constant_alignment (tree exp, int align)
 {
-  if (TREE_CODE (exp) == REAL_CST)
+  if (TREE_CODE (exp) == REAL_CST || TREE_CODE (exp) == VECTOR_CST
+      || TREE_CODE (exp) == INTEGER_CST)
     {
       if (TYPE_MODE (TREE_TYPE (exp)) == DFmode && align < 64)
        return 64;

Re: [PATCH, i386]: Committed: Fix PR target/35496

[H.J. Lu]

On Sun, Mar 9, 2008 at 5:38 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> Hello!
>
>  We have to compute alignment of VECTOR_CST and INTEGER_CST constants via
>  ALIGN_MODE_128 in the same way as other ALIGN_MODE_128 data is aligned.
>
>  2008-03-09  Uros Bizjak  <ubizjak@gmail.com>
>
>         PR target/35496
>         * config/i386/i386.c (ix86_constant_algnment): Compute alignment
>  using
>         ALIGN_MODE_128 for VECTOR_CST and INTEGER_CST in addition to
>  REAL_CST.
>
>  Patch was bootstrapped and regression tested on x86_64-pc-linux-gnu
>  {,-m32}. Patch is committed to mainline, but I think that it should be
>  committed to 4.3 branch as well.
>

Can we add a few testcases for Linux/ia32?


H.J.

Re: [PATCH, i386]: Committed: Fix PR target/35496

[Uros Bizjak]

[-- Attachment #1: Type: text/plain, Size: 677 bytes --]

H.J. Lu wrote:

>>  Patch was bootstrapped and regression tested on x86_64-pc-linux-gnu
>>  {,-m32}. Patch is committed to mainline, but I think that it should be
>>  committed to 4.3 branch as well.
>>     
>
> Can we add a few testcases for Linux/ia32?
>   

Actually, I have a testcase (attached), that clearly shows the problem 
in the asm dump, but it looks that linker somehow fixed this alignment, 
so I was not able to fail it (-m32 -O2 -msse2):

        .section        .rodata
        .align 8
.LC0:
        .long   1
        .long   0
        .long   2
        .long   0

It looks that this fixup is the reason that the problem went undetected 
for so long.

Uros.


[-- Attachment #2: vv.c --]
[-- Type: text/x-csrc, Size: 339 bytes --]

typedef long long __v2di __attribute__ ((__vector_size__ (16)));

typedef union
{
  __v2di _v;
  long long _a[2];
} __v2di_t;

void abort (void);

static __v2di x = { 1, 2 };

__v2di __attribute__((noinline))
test (void)
{
  return x;
}

int main()
{
  __v2di_t val;

  val._v = test ();

  if (val._a[0] + val._a[1] != 3)
    abort ();
}

Re: [PATCH, i386]: Committed: Fix PR target/35496

[H.J. Lu]

Hi Uros,

Did you confirm it on Linux with visual inspection?

H.J.
2008/3/9 Uros Bizjak <ubizjak@gmail.com>:
> H.J. Lu wrote:
>
>  >>  Patch was bootstrapped and regression tested on x86_64-pc-linux-gnu
>  >>  {,-m32}. Patch is committed to mainline, but I think that it should be
>  >>  committed to 4.3 branch as well.
>  >>
>  >
>  > Can we add a few testcases for Linux/ia32?
>  >
>
>  Actually, I have a testcase (attached), that clearly shows the problem
>  in the asm dump, but it looks that linker somehow fixed this alignment,
>  so I was not able to fail it (-m32 -O2 -msse2):
>
>         .section        .rodata
>         .align 8
>  .LC0:
>         .long   1
>         .long   0
>         .long   2
>         .long   0
>
>  It looks that this fixup is the reason that the problem went undetected
>  for so long.
>
>  Uros.
>
>

Re: [PATCH, i386]: Committed: Fix PR target/35496

[Uros Bizjak]

H.J. Lu wrote:

> Did you confirm it on Linux with visual inspection?
>   

No, fortran testcase failed with -O2 -msse2 -m32.

Uros.

Re: [PATCH, i386]: Committed: Fix PR target/35496

[H.J. Lu]

On Sun, Mar 09, 2008 at 07:39:36PM +0100, Uros Bizjak wrote:
> H.J. Lu wrote:
>
>> Did you confirm it on Linux with visual inspection?
>>   
>
> No, fortran testcase failed with -O2 -msse2 -m32.

Can we create a few Fortran tests with -O2 -msse2 -m32?


H.J.

Re: [PATCH, i386]: Committed: Fix PR target/35496

[Uros Bizjak]

H.J. Lu wrote:

>>> Did you confirm it on Linux with visual inspection?
>>>   
>>>       
>> No, fortran testcase failed with -O2 -msse2 -m32.
>>     
>
> Can we create a few Fortran tests with -O2 -msse2 -m32?
>   

No, because fortran testsuite can't detect SSE2 support at runtime. See 
fortran vectorizer tests...

Uros.

Re: [PATCH, i386]: Committed: Fix PR target/35496

[Kaveh R. GHAZI]

On Sun, 9 Mar 2008, Uros Bizjak wrote:

> H.J. Lu wrote:
>
> >>> Did you confirm it on Linux with visual inspection?
> >>>
> >>>
> >> No, fortran testcase failed with -O2 -msse2 -m32.
> >>
> >
> > Can we create a few Fortran tests with -O2 -msse2 -m32?
> >
>
> No, because fortran testsuite can't detect SSE2 support at runtime. See
> fortran vectorizer tests...
> Uros.

See PRs 34168 & 34878, (hoping someone will fix them...)

		Thanks,
		--Kaveh
--
Kaveh R. Ghazi			ghazi@caip.rutgers.edu