Re: IVopts bug?

2011-11-01 Thread Yuehai Du
2011/11/1 Richard Guenther :
> 2011/11/1 杜越海 :
>> Hi all
>>
>>  I found IVopts rewrite a memory access with a weird iv candidate,
>> which make it lost its original memory attribute.
>>  a non-local memory access' base pointer was rewrite into a local one,
>> and  it was deleted in pass_cd_dce since
>> it was recognized as a local memory access.
>>
>> here is the case i simplified from a decoder source
>>
>> foo1(unsigned char* pSrcLeft,
>> unsigned char* pSrcAbove,
>> unsigned char* pSrcAboveLeft,
>> unsigned char* pDst,
>> int dstStep,
>> int leftStep)
>> {
>>  signed int x, y, s;
>>  unsigned char  p1[5], p2[5],  p3;
>>
>>  p1[0] = *pSrcAboveLeft;
>>  p2[0] = p1[0];
>>  p2[1] = pSrcLeft[0];
>>  pSrcLeft += leftStep;
>>  p2[2] = pSrcLeft[0];
>>  pSrcLeft += leftStep;
>>  p2[3] = pSrcLeft[0];
>>  pSrcLeft += leftStep;
>>  p2[4] = pSrcLeft[0];
>>
>>  p1[1] = pSrcAbove[0];
>>  p1[2] = pSrcAbove[1];
>>  p1[3] = pSrcAbove[2];
>>  p1[4] = pSrcAbove[3];
>>
>>  p3 = (unsigned char)(((signed int)p1[1] + (signed int)p2[1] +
>> (signed int)p1[0]
>>+(signed int)p1[0] + 2 ) >> 2 );
>>
>>  for( y=0; y<4; y++, pDst += dstStep ) {
>>for( x=y+1; x<4; x++ ) {
>>s = ( p1[x-y-1] + p1[x-y] + p1[x-y] + p1[x-y+1] + 2 ) >> 
>> 2;
>>pDst[x] = (unsigned char)s;
>>}
>>
>>pDst[y] = p3; -This memory access
>>  }
>> }
>>
>> before IVopts
>>
>>  D.6508_65 = pDst_88 + y.6_64;
>>  *D.6508_65 = p3_37;
>>
>> after IVopts
>> it was rewrite to
>> MEM[symbol: p1, index: ivtmp.161_200, offset: 0B] = p3_37 ,
>>
>> by
>> candidate 15
>>  depends on 3
>>  var_before ivtmp.161
>>  var_after ivtmp.161
>>  incremented before exit test
>>  type unsigned int
>>  base (unsigned int) pDst_39(D) - (unsigned int) &p1
>>  step (unsigned int) (pretmp.28_118 + 1)
>>
>> so it still is &p1+ pDst - &p1 + step = pDst + step,
>> and in pass_cd_dce, is_hidden_global_store () return false for this memory
>> since it think this stmt only access local array p1.
>>
>>
>>
>> gcc version r180694
>>
>> Configured with: /home/croseadu/android/_src/src/gcc-src/configure
>> --host=i486-linux-gnu --build=i486-linux-gnu
>> --target=arm-none-linux-gnueabi
>> --prefix=/home/croseadu/android/_src/install/arm-none-linux-gnueabi
>> --enable-threads --disable-libmudflap --disable-libssp
>> --disable-libstdcxx-pch --with-gnu-as --with-gnu-ld
>> --enable-languages=c,c++ --enable-shared --enable-symvers=gnu
>> --enable-__cxa_atexit
>> --with-specs='%{funwind-tables|fno-unwind-tables|mabi=*|ffreestanding|nostdlib:;:-funwind-tables}'
>> --disable-nls --enable-lto
>> --with-sysroot=/home/croseadu/android/_src/install/arm-none-linux-gnueabi/arm-none-linux-gnueabi/libc
>> --with-build-sysroot=/home/croseadu/android/_src/install/arm-none-linux-gnueabi/arm-none-linux-gnueabi/libc
>> --with-gmp=/home/croseadu/android/_src/objs/arm-none-linux-gnueabi/obj/host-libs-/usr
>> --with-mpfr=/home/croseadu/android/_src/objs/arm-none-linux-gnueabi/obj/host-libs-/usr
>> --with-ppl=/home/croseadu/android/_src/objs/arm-none-linux-gnueabi/obj/host-libs-/usr
>> --with-host-libstdcxx='-static-libgcc -Wl,-Bstatic,-lstdc++,-Bdynamic
>> -lm' 
>> --with-cloog=/home/croseadu/android/_src/objs/arm-none-linux-gnueabi/obj/host-libs-/usr
>> --enable-cloog-backend=isl
>> --with-mpc=/home/croseadu/android/_src/objs/arm-none-linux-gnueabi/obj/host-libs-/usr
>> --enable-poison-system-directories --disable-libquadmath --enable-lto
>> --enable-libgomp
>> --with-build-time-tools=/home/croseadu/android/_src/install/arm-none-linux-gnueabi/arm-none-linux-gnueabi/bin
>> --with-cpu=cortex-a8 --with-float=soft
>>
>> compile flags:
>> -O3 -mfpu=neon -mfloat-abi=softfp -mvectorize-with-neon-double
>>
>> need file a bug?
>
> Yes, it definitely should not do this kind of stupid (and invalid) thing.
>
> Richard.
>
>>
>> Yuehai Du
>>
>
file a bug http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50955,  Could
someboy help me to fix this PR? Thank you very much.

Yuehai Du


a question about IVOPTS: find_interesting_uses_address

2011-11-13 Thread Yuehai Du
Hi

  i found IVOPTS didn't work well on some case if the loop contain
some unaligned access. it didn't take this kind of memory access into
account because this check in function:find_interesting_uses_address

     /* Moreover, on strict alignment platforms, check that it is
        sufficiently aligned.  */
     if (STRICT_ALIGNMENT && may_be_unaligned_p (base, step))
       goto fail;

 and this is used to fix http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17949
because it converting byte loads into a unaligned word load on strict
alignment platform.

  but there are some platform which is strict alignment and  it also
support unaligned access for special mode. for example, ARM NEON support
vector mode unaligned access  via VLD1/VST1, and they both support write back
address mode.

  Moreover, lately ARM add unaligned access support for  all ARMv6 , ARMv7-A,
ARMv7-R, and ARMv7-M architecture-based processors which controlled by the
option: munaligned-access.
see http://gcc.gnu.org/ml/gcc-patches/2011-05/msg00513.html.

  so what should we do now? should we use target hook to check this or
something else?

--
Yuehai Du


Re: a question about IVOPTS: find_interesting_uses_address

2011-11-17 Thread Yuehai Du
2011/11/18 Richard Guenther :
> On Thu, Nov 17, 2011 at 6:49 AM, Yuehai Du  wrote:
>> 2011/11/17 Richard Guenther :
>>> On Wed, Nov 16, 2011 at 12:28 PM, Eric Botcazou  
>>> wrote:
>>>>> Huh, IVOPTs should never cause a different size memory read.  I wonder
>>>>> if the original issue would still reproduce with the fix reverted.
>>>>
>>>> The original issue was unaligned arrays in packed structures.  I don't see 
>>>> what
>>>> could have changed since then.
>>>
>>> Ah, so it was expand being confused about the constructed access (not seeing
>>> a component-ref), thus the usual old issue we have there.  There's indeed no
>>> trivial fix for this but to create the [TARGET_]MEM_REF with an explicitely
>>> unaligned type and hope that the target implements the movmisalign optab
>>> (the only case this explicit alignment is honored).  And double-check the
>>> TARGET_MEM_REF expansion code properly duplicates the movmisalign
>>> path from the MEM_REF expander.
>>>
>>> I'd wish somebody fixed this path for non-movmisalign strict-align targets.
>>>
>>
>>  I'm a little bit confused, and i don't know if i understand you correctly.
>> are you saying that we can remove those check in 
>> find_interesting_uses_address,
>> and handle this misalign memory reference as aligned memory reference,
>> just attach the
>> misalign information in type and let expander to choose correct
>> expansion dependent on
>> movmisalign optab icode ?
>
> Yes.  But only if movmisalign is available.
>
>>  In that case if Target don't support misalign memory access, the
>> result of IVOPTS will
>> be non-optimal since the cost of IV used in misalign memory access is
>> wrong,  right?
>
> No, the cost of the IV is the same, independent on whether it is aligned or 
> not.
>
> Richard.

  I got you,thanks.
  Would it be ok if i file a bug about this,like missed optimization
for misaligned
memory access in IVOPTS?

  and since the path for non-movmisalign strict-align targets don't
exist now, could i
add a condition in that check to avoid this. like:

   if (STRICT_ALIGNMENT && may_be_unaligned_p (base, step)
   && optab_handler (movmisalign_optab, mode) == CODE_FOR_nothing)
goto fail;

  I don't know if it is appropriate to check optab in IVOPTS, sorry if
i go too far.

--
Yuehai

>
>> --
>> Yuehai
>>
>>
>>> Richard.
>>>
>>>> --
>>>> Eric Botcazou
>>>>
>>>
>>
>