[llvm-commits] [LLVM, llc] TypeLegalization, DAGCombining, vectors loading: Obvious fixes.

[Duncan Sands]

Hi Stepan, it is good to discuss how vectors should be represented
in memory, but unfortunately I will be away for the next two weeks
so now is not a good time for me.

Ciao, Duncan.

On 16/12/11 12:23, Stepan Dyatkovskiy wrote:
> This was fix for ExpandLoad. Reading this method code, I found that it assumes that vector element are already not bitpacked.
>
> SrcVT.getScalarType() returns vector element type. If vector element size less than 8, then "element-size/8" will always zero. So if we decided to expand load, the stride for each vector element will wrong:
> unsigned Stride = SrcVT.getScalarType().getSizeInBits()/8; // is always zero.
>
> For simple vector definition like
> @V = global<2 x i1>  <i1 0, i1 1>
> No any bitpacking/unpacking produced. Is there any attribute that set the vector bitpacked?
>
> Relative to PR1784. Dan proposes bitcast redefinition here:
> http://llvm.org/bugs/show_bug.cgi?id=1784#c15
>
> If I get right, this approach allows to store any vectors without bitpacking.
>
> -Stepan
>
> 16.12.2011, 11:56, "Duncan Sands"<baldrick at free.fr>:
>> Hi Stepan,
>>
>>>   I just make code workable for cases where it failes now (of course for 2 x i1 and friends). Two of them you can found in attachment.
>>>
>>>     SrcVT.getScalarType().getSizeInBits()/8 - is obvious mistake IMHO. It weakly depends on main problem.
>>
>> if<2 x i1>  is stored bit packed then advancing the pointer here is also wrong.
>>
>> Ciao, Duncan.
>>
>>>   The same with trunc store.
>>>
>>>   -Stepan
>>>
>>>   15.12.2011, 12:41, "Duncan Sands"<baldrick at free.fr>:
>>>>   Hi Stepan,
>>>>>     For 'load' I fixed the next:
>>>>>
>>>>>     Next string will always wrong for non round types. For 2 x i7 it returns 1 byte!
>>>>>     unsigned Stride = SrcVT.getScalarType().getSizeInBits()/8;
>>>>>
>>>>>     I just replaced it with this one:
>>>>>
>>>>>     unsigned Stride = SrcVT.getScalarType().getStoreSize();
>>>>>
>>>>>     This code just calculates proper size in bytes.
>>>>   I think it is pointless trying to fix things for<2 x i7>    and friends until it
>>>>   is decided how they should be represented in memory.
>>>>
>>>>   Ciao, Duncan.
>>>>>     -Stepan
>>>>>
>>>>>     15.12.2011, 04:48, "Dan Gohman"<gohman at apple.com>:
>>>>>>     On Dec 14, 2011, at 12:57 PM, Stepan Dyatkovskiy wrote:
>>>>>>>       Hi all. Please find some obvious fixes for<n x i1>      ..<n x i7>      vectors.
>>>>>>>       I fixed 'load' expansion, since there was improper stride calculation.
>>>>>>>       I also fixed some assertion for trunc store. Assertion dropped for trunc stores with non simple MemoryVT. MemoryVT<n x i1>      will never became simple in ToT. I inserted the store expansion in this case.
>>>>>>     At least the load part of this patch is wrong. PR1784 specifically says that
>>>>>>     vector elements are to be bit-packed.
>>>>>>
>>>>>>     Dan
>>>>>>
>>>>>>     _______________________________________________
>>>>>>     llvm-commits mailing list
>>>>>>     llvm-commits at cs.uiuc.edu
>>>>>>     http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>>     _______________________________________________
>>>>>     llvm-commits mailing list
>>>>>     llvm-commits at cs.uiuc.edu
>>>>>     http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>   _______________________________________________
>>>>   llvm-commits mailing list
>>>>   llvm-commits at cs.uiuc.edu
>>>>   http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits