i1 vectors (Re: [PATCH] Fix PR15267)

[Nadav Rotem]

On Apr 9, 2014, at 1:28 PM, Hal Finkel <hfinkel at anl.gov> wrote:

> ----- Original Message -----
>> From: "Nadav Rotem" <nrotem at apple.com>
>> To: "Hal Finkel" <hfinkel at anl.gov>
>> Cc: "Duncan Sands" <baldrick at free.fr>, llvm-commits at cs.uiuc.edu, "Elena Demikhovsky" <elena.demikhovsky at intel.com>
>> Sent: Wednesday, April 9, 2014 3:20:04 PM
>> Subject: Re: i1 vectors (Re: [PATCH] Fix PR15267)
>> 
>> 
>> On Apr 9, 2014, at 1:15 PM, Hal Finkel <hfinkel at anl.gov> wrote:
>> 
>>> ----- Original Message -----
>>>> From: "Duncan Sands" <baldrick at free.fr>
>>>> To: "Hal Finkel" <hfinkel at anl.gov>
>>>> Cc: llvm-commits at cs.uiuc.edu, "Nadav Rotem" <nrotem at apple.com>
>>>> Sent: Wednesday, February 20, 2013 11:38:45 AM
>>>> Subject: Re: [PATCH] Fix PR15267
>>>> 
>>>> Hi Hal,
>>>> 
>>>>>>>> it sounds like you consider <4 x i1> to be bitpacked in
>>>>>>>> memory,
>>>>>>>> like i4.
>>>>>>>> Is that right?
>>>>>>> 
>>>>>>> Yes. We need this in order to be consistent with targets that
>>>>>>> support mask
>>>>>>> registers.
>>>>>> 
>>>>>> while it makes sense to me, can you please document this
>>>>>> somewhere
>>>>>> and ensure
>>>>>> that getTypeAllocSize and friends are fixed (should return 1
>>>>>> byte!).
>>>>> 
>>>>> Also, we need to make sure that global constants are emitted
>>>>> correctly. Currently, if you make a <N x i1> global constant, it
>>>>> will emit each element into a separate byte.
>>>> 
>>>> all kinds of codegen stuff is broken for these types.  However
>>>> even
>>>> before
>>>> getting as far as codegen, there are issues at the IR level (type
>>>> sizes as
>>>> I mentioned, and also passes that get it wrong for these types).
>>>> Someone
>>>> really needs to audit the IR carefully (I would volunteer but I
>>>> don't
>>>> think
>>>> I have time), and then take a careful look at codegen
>>>> (particularly
>>>> the type
>>>> legalization logic; I'm probably the right guy to do that,
>>>> but...).
>>> 
>>> To bring this up again, I'm becoming increasingly nervous, seeing
>>> all of the i1 vector support, not only in AVX but also in AVX-512,
>>> going into the x86 backend while this has not yet been addressed.
>>> 
>>> I think that either we should come up with a definitive plan for
>>> dealing with this, or we should forbid outright the
>>> loading/storing of i1 vectors until we do. The in-register
>>> representations of the i1 vectors, as far as I can tell, is fine
>>> (so long as the backend provides a self-consistent implementation
>>> of build_vector, extract_elemt, etc.). However, providing i1
>>> vectors in their current state, as part of the general LLVM IR
>>> interface, with an exposed yet inconsistent memory representation,
>>> feels like asking for trouble.
>>> 
>> 
>> I think that at this point the representation of i1 vectors is clear.
>> i1s are bit packed in memory. When AVX512 masks are saved to memory
>> they are copied as-is. When an AVX mask are saved to memory they
>> need to be packed. At the moment we have a bug in the code that
>> packs i1s into memory. But this can be fixed. With the exception of
>> the load/store bug everything else already works.
> 
> Globals, at least, are still broken:
> 
> $ cat /tmp/g.ll 
> @Q = global <4 x i1> <i1 0, i1 undef, i1 1, i1 1>, align 16
> 
> $ llc -mtriple=x86_64-pc-linux -mcpu=corei7-avx < /tmp/g.ll 
> 	.text
> 	.file	"<stdin>"
> 	.type	Q, at object               # @Q
> 	.data
> 	.globl	Q
> 	.align	16
> Q:
> 	.byte	0                       # 0x0
> 	.zero	1
> 	.byte	1                       # 0x1
> 	.byte	1                       # 0x1
> 	.size	Q, 4
> 
> And so if you try to load from this value then you'll obviously have a problem. Is this what you were talking about, or are you referring to something else?

I forgot about globals. Yes, this is also a problem.  I meant that SelectionDAGLegalize::LegalizeLoadOps and SelectionDAGLegalize::LegalizeStoreOps don’t know how to pack i1 vectors.


> 
> -Hal
> 
>> 
>> 
>>> -Hal
>>> 
>>>> 
>>>> Ciao, Duncan.
>>>> 
>>>> 
>>> 
>>> --
>>> Hal Finkel
>>> Assistant Computational Scientist
>>> Leadership Computing Facility
>>> Argonne National Laboratory
>> 
>> 
> 
> -- 
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140409/c0a0826e/attachment.html>