Bug 37194 - [4.3 Regression] Autovectorization of small constant iteration loop degrades performance
Summary: [4.3 Regression] Autovectorization of small constant iteration loop degrades ...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 4.4.0
: P2 normal
Target Milestone: 4.3.3
Assignee: Not yet assigned to anyone
URL:
Keywords: missed-optimization
: 37579 (view as bug list)
Depends on:
Blocks:
 
Reported: 2008-08-21 19:21 UTC by Pat Haugen
Modified: 2009-01-11 07:57 UTC (History)
9 users (show)

See Also:
Host:
Target: powerpc64-linux
Build:
Known to work:
Known to fail:
Last reconfirmed: 2008-08-21 19:32:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Pat Haugen 2008-08-21 19:21:55 UTC
Seeing a degradation in cpu2000 benchmark 252.eon that is caused by autovectorization of a simple loop in function ggSpectrum::Set(float).

Here's a simple C version.

void ggSpectrum_Set(float * data, float d) {
   int i;
   for (i = 0; i < 8; i++)
      data[i] = d;
}


When compiled with -O3 -mcpu=970 the following code is generated:

ggSpectrum_Set:
        mfvrsave 0
        stwu 1,-48(1)
        stw 0,44(1)
        oris 0,0,0x8000
        mtvrsave 0
        li 10,0
        rlwinm 0,3,30,30,31
        subfic 0,0,4
        andi. 9,0,3
        beq- 0,.L16
        mtctr 9
        .p2align 4,,15
.L10:
        slwi 0,10,2
        addi 10,10,1
        stfsx 1,3,0
        subfic 8,10,8
        bdnz .L10
.L3:
        subfic 6,9,8
        srwi 0,6,2
        slwi. 7,0,2
        beq- 0,.L5
        mtctr 0
        stfs 1,16(1)
        cmpwi 7,0,0
        li 0,16
        slwi 9,9,2
        li 11,0
        add 9,3,9
        lvewx 0,1,0
        vspltw 0,0,0
        beq- 7,.L17
        .p2align 4,,15
.L6:
        slwi 0,11,4
        addi 11,11,1
        stvx 0,9,0
        bdnz .L6
        cmpw 7,6,7
        subf 8,7,8
        add 10,10,7
        beq- 7,.L9
.L5:
        mtctr 8
        slwi 0,10,2
        add 3,3,0
        .p2align 4,,15
.L8:
        stfs 1,0(3)
        addi 3,3,4
        bdnz .L8
.L9:
        lwz 12,44(1)
        mtvrsave 12
        addi 1,1,48
        blr
.L16:
        mr 10,9
        li 8,8
        b .L3
.L17:
        li 0,1
        mtctr 0
        b .L6


Adding -mno-altivec results in this simpler sequence, and a significant boost in performance (~40% speedup for the benchmark):

ggSpectrum_Set:
        stfs 1,28(3)
        stfs 1,0(3)
        stfs 1,4(3)
        stfs 1,8(3)
        stfs 1,12(3)
        stfs 1,16(3)
        stfs 1,20(3)
        stfs 1,24(3)
        blr


Another thing that stood out from the benchmark run was that the code was taking a pretty big hit on a couple of the statically predicted branches (apparently the address was already 16 byte aligned a lot of the time). So it seems like it would be best to remove the static prediction and let the hardware prediction take over.
Comment 1 Drea Pinski 2008-08-21 19:32:00 UTC
Confirmed, this is true for the Cell too.  In fact is bad for the cell because of:

        stfs 1,16(1)
        cmpwi 7,0,0
        li 0,16
        slwi 9,9,2
        li 11,0
        add 9,3,9
        lvewx 0,1,0
        vspltw 0,0,0

The store is not 40-50 cycles before the load so we have a LHS.
Comment 2 Richard Biener 2008-08-22 09:53:10 UTC
The x86_64 generated code looks like

ggSpectrum_Set:
.LFB0:
        .cfi_startproc
        movq    %rdi, %rax
        xorl    %ecx, %ecx
        movq    %rdi, %rdx
        andl    $15, %eax
        shrq    $2, %rax
        negl    %eax
        andl    $3, %eax
        je      .L15
        movl    $8, %r8d
        .p2align 4,,10
        .p2align 3
.L10:
        addl    $1, %ecx
        movl    %r8d, %esi
        movss   %xmm0, (%rdx)
        subl    %ecx, %esi
        addq    $4, %rdx
        cmpl    %ecx, %eax
        ja      .L10
.L3:
        movl    $8, %r10d
        subl    %eax, %r10d
        movl    %r10d, %r8d
        shrl    $2, %r8d
        leal    0(,%r8,4), %r9d
        testl   %r9d, %r9d
        je      .L5
        movaps  %xmm0, %xmm2
        sall    $2, %eax
        mov     %eax, %eax
        xorl    %edx, %edx
        shufps  $0, %xmm2, %xmm2
        leaq    (%rdi,%rax), %rax
        movaps  %xmm2, %xmm1
        .p2align 4,,10
        .p2align 3
.L6:
        addl    $1, %edx
        movaps  %xmm1, (%rax)
        addq    $16, %rax
        cmpl    %r8d, %edx
        jb      .L6
        addl    %r9d, %ecx
        subl    %r9d, %esi
        cmpl    %r9d, %r10d
        je      .L9
.L5:
        movslq  %ecx,%rax
        leaq    (%rdi,%rax,4), %rax
        .p2align 4,,10
        .p2align 3
.L8:
        movss   %xmm0, (%rax)
        addq    $4, %rax
        subl    $1, %esi
        jne     .L8
.L9:
        rep
        ret
.L15:
        movl    $8, %esi
        movl    %eax, %ecx
        jmp     .L3
        .cfi_endproc

I wonder why we do not use movups instead.

t.i:3: note: Alignment of access forced using peeling.
t.i:3: note: Peeling for alignment will be applied.

t.i:3: note: Cost model analysis:
  Vector inside of loop cost: 1
  Vector outside of loop cost: 13
  Scalar iteration cost: 1
  Scalar outside cost: 7
  prologue iterations: 2
  epilogue iterations: 2
  Calculated minimum iters for profitability: 7

t.i:3: note: === vect_do_peeling_for_alignment ===
t.i:3: note: created vect_p.29_13
t.i:3: note: niters for prolog loop: (unsigned int) (4 - (((long unsigned int) vect_p.29_13 & 15) >> 2)) & 3
t.i:3: note: Vectorization may not be profitable.

Comment 3 dorit 2008-08-22 13:31:20 UTC
(In reply to comment #2)
> The x86_64 generated code looks like
...
> I wonder why we do not use movups instead.
> t.i:3: note: Alignment of access forced using peeling.
> t.i:3: note: Peeling for alignment will be applied.

because the vectorizer doesn't support misaligned stores. I think it should be easy to add - see this old patch: http://gcc.gnu.org/ml/gcc-patches/2007-01/msg00604.html (and also on http://gcc.gnu.org/wiki/VectorizationTasks, under "todo"). 
Comment 4 Drea Pinski 2008-12-27 05:53:54 UTC
*** Bug 37579 has been marked as a duplicate of this bug. ***
Comment 5 Drea Pinski 2008-12-27 05:54:55 UTC
I found this also too and saw it was a regression because -O3 changes.
Comment 6 Richard Biener 2008-12-29 21:06:11 UTC
t.i:3: note: Vectorization may not be profitable.

why doesn't the cost model then disallow vectorization here?
Comment 7 Ira Rosen 2008-12-30 14:57:00 UTC
(In reply to comment #6)
> t.i:3: note: Vectorization may not be profitable.
> why doesn't the cost model then disallow vectorization here?

This is misleading. It only means that there exists loop bound threshold either defined by the user or calculated with the cost model. It does not mean that the cost model's decision is that the vectorization is not profitable.

I am adding this to our cleanup todo list.
Comment 8 Ira Rosen 2009-01-05 13:58:27 UTC
To handle unknown alignment of data, the vectorizer creates a prolog loop to peel a statically unknown number of scalar iterations (0<=n<VF). This loop is followed by a vectorized loop (with the remaining, multiple of VF, number of iterations), and an epilog scalar loop that completes the iterations that were not executed (0<=n<VF).
Therefore, the created scalar loops have unknown number of iterations, which prevents their unrolling (while the original scalar loop is unrolled). Vectorizer cost model does not take possible unrolling into account. 

Another cost model problem is that the calculation of scalar outside cost for this case is performed not for the original scalar version, but includes run-time guards. Which seems to be wrong in case that the original loop bound is known. I am going to submit a patch to fix that.

Ira

Comment 9 irar 2009-01-08 07:59:59 UTC
Subject: Bug 37194

Author: irar
Date: Thu Jan  8 07:59:40 2009
New Revision: 143183

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=143183
Log:
	PR tree-optimization/37194
	* tree-vect-transform.c (vect_estimate_min_profitable_iters):
	Don't add the cost of cost model guard in prologue to scalar 
	outside cost in case of known number of iterations.


Added:
    trunk/gcc/testsuite/gcc.dg/vect/costmodel/ppc/costmodel-pr37194.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-vect-transform.c

Comment 10 Jakub Jelinek 2009-01-08 09:22:51 UTC
Fixed.
Comment 11 littlestar 2009-01-08 09:24:11 UTC
fixed for 4.3.3?
Thanks.
Comment 12 Ira Rosen 2009-01-08 09:25:57 UTC
(In reply to comment #11)
> fixed for 4.3.3?
> Thanks.

No, still waiting for approval.
Comment 13 irar 2009-01-11 07:54:50 UTC
Subject: Bug 37194

Author: irar
Date: Sun Jan 11 07:54:40 2009
New Revision: 143263

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=143263
Log:
	Backport from mainline:
	2009-01-08  Ira Rosen  <irar@il.ibm.com>

	PR tree-optimization/37194
	* tree-vect-transform.c (vect_estimate_min_profitable_iters):
	Don't add the cost of cost model guard in prologue to scalar
	outside cost in case of known number of iterations.


Added:
    branches/gcc-4_3-branch/gcc/testsuite/gcc.dg/vect/costmodel/ppc/costmodel-pr37194.c
Modified:
    branches/gcc-4_3-branch/gcc/ChangeLog
    branches/gcc-4_3-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_3-branch/gcc/tree-vect-transform.c

Comment 14 Ira Rosen 2009-01-11 07:57:07 UTC
Fixed.