i1 vectors (Re: [PATCH] Fix PR15267)

[Hal Finkel]

As far as I can tell, unfortunately, this is not well defined. I believe you are correct, but we really need some LangRef updates to clarify this (and the behavior of <n x i1> too).

 -Hal

----- Original Message -----
> From: "Elena Demikhovsky" <elena.demikhovsky at intel.com>
> To: "Nadav Rotem" <nrotem at apple.com>, "Hal Finkel" <hfinkel at anl.gov>
> Cc: "Duncan Sands" <baldrick at free.fr>, llvm-commits at cs.uiuc.edu
> Sent: Tuesday, April 22, 2014 7:35:26 AM
> Subject: RE: i1 vectors (Re: [PATCH] Fix PR15267)
> 
> 
> 
> 
> I have even more problems with i1 scalar type which became legal in
> AVX-512.
> 
> 
> 
> “store i1” writes 8 bits to memory. Nobody expects that “store i1”
> will be done as
> 
> 
> 
> load i8 -> set LSB -> store i8
> 
> 
> 
> am I right?
> 
> 
> 
> 
> - Elena
> 
> 
> 
> 
> 
> From: Nadav Rotem [mailto:nrotem at apple.com]
> Sent: Wednesday, April 09, 2014 23:30
> To: Hal Finkel
> Cc: Duncan Sands; llvm-commits at cs.uiuc.edu; Demikhovsky, Elena
> Subject: Re: i1 vectors (Re: [PATCH] Fix PR15267)
> 
> 
> 
> 
> 
> 
> 
> 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
> 
> 
> 
> ---------------------------------------------------------------------
> Intel Israel (74) Limited
> 
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory