i1 vectors (Re: [PATCH] Fix PR15267)

[Demikhovsky, Elena]

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<mailto:hfinkel at anl.gov>> wrote:


----- Original Message -----

From: "Nadav Rotem" <nrotem at apple.com<mailto:nrotem at apple.com>>
To: "Hal Finkel" <hfinkel at anl.gov<mailto:hfinkel at anl.gov>>
Cc: "Duncan Sands" <baldrick at free.fr<mailto:baldrick at free.fr>>, llvm-commits at cs.uiuc.edu<mailto:llvm-commits at cs.uiuc.edu>, "Elena Demikhovsky" <elena.demikhovsky at intel.com<mailto: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<mailto:hfinkel at anl.gov>> wrote:


----- Original Message -----

From: "Duncan Sands" <baldrick at free.fr<mailto:baldrick at free.fr>>
To: "Hal Finkel" <hfinkel at anl.gov<mailto:hfinkel at anl.gov>>
Cc: llvm-commits at cs.uiuc.edu<mailto:llvm-commits at cs.uiuc.edu>, "Nadav Rotem" <nrotem at apple.com<mailto: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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140422/a6286e9e/attachment.html>