i1 vectors (Re: [PATCH] Fix PR15267)

[Hal Finkel]

----- 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?

 -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