Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

<4 x i1> & <4 x i8> can break in compiler in TargetData::getAlignmentInfo #1954

Closed Quuxplusone closed 13 years ago

Quuxplusone commented 16 years ago
Bugzilla Link PR1845
Status RESOLVED FIXED
Importance P normal
Reported by Chuck Rose (cfr@adobe.com)
Reported on 2007-12-07 20:03:10 -0800
Last modified on 2011-09-22 13:33:26 -0700
Version 2.1
Hardware PC Windows XP
CC anton@korobeynikov.info, baldrick@free.fr, clattner@nondot.org, evan.cheng@apple.com, llvm-bugs@lists.llvm.org, nadav.rotem@me.com
Fixed by commit(s)
Attachments fibonacci.cpp (3707 bytes, text/plain)
Blocks
Blocked by
See also
Created attachment 1280
Modification of the fibonacci example

The following function will fail to compile:

; ModuleID = 'test'

define void @boolVectorSelect(<4 x i1>* %boolVectorPtr) {
Body:
        %castPtr = bitcast <4 x i1>* %boolVectorPtr to <4 x i1>*
; <<4 x i1>*> [#uses=1]
        %someBools = load <4 x i1>* %castPtr, align 1           ; <<4 x i1>> [#u
ses=1]
        %internal = alloca <4 x i1>, align 16           ; <<4 x i1>*> [#uses=1]
        store <4 x i1> %someBools, <4 x i1>* %internal, align 1
        ret void
}
verifying... Assertion failed: BestMatchIdx != -1 && "Didn't find alignment info
 for this datatype!", file ..\..\lib\Target\TargetData.cpp, line 303

Which can be repro'd with the attached program.  (based on fibonacci llvm
example)
Quuxplusone commented 16 years ago

Attached fibonacci.cpp (3707 bytes, text/plain): Modification of the fibonacci example

Quuxplusone commented 16 years ago

I'm fairly certain that <2 x i1> and <2 x i8> are also affected.

Quuxplusone commented 16 years ago
This is a symptom of a deeper ill: vectors with gaps in them are
really not handled properly.  Here there is a 7 bit gap between
successive i1's in the vector.  The same goes for vectors of other
funky length integers, and for vectors of 80-bit x86 long double.
That said, probably something can be done so that simple uses like
this will work.  It seems like quite a bit of work though since the
denseness of vectors seems to be assumed in a bunch of places in the
basic vector code.  Also, I know of at least one optimizer (SROA)
that can make wrong deductions about vectors that are not densely
packed.  In my opinion you were lucky to have gotten away with this
before :)
Quuxplusone commented 16 years ago
I just verified that it repros on <4 x i8> which shouldn't have gaps.  Don't
know if that is more of a surface area failure in getAlignmentInfo or if it
would also be affected by the deeper issues you indicate below.

Thanks,
Chuck.
Quuxplusone commented 16 years ago

OK, I will take a look.

Quuxplusone commented 16 years ago
The problem is that there is no alignment info for <4 x i8>,
at least on x86.  The alignment is calculated from the bit
size, which is 32 in this case.  However on x86 there is
only alignment info for 64 bit and 128 bit vectors, so
any attempt to get alignment for this type bombs out.  My
recent changes cause alignment to be needed more often,
exposing this problem.  I have no idea what the right way
of determining alignment for vectors is.  I expected the
alignment to be equal to the alignment of the element
type, but it seems it is more complicated than that.
Quuxplusone commented 16 years ago
I think this one is for Chris, since he wrote the alignment info
code for vectors.
Quuxplusone commented 16 years ago
PS: I've changed my mind about bit-packing vector elements: I now think
that vector elements have to be bit-packed or perhaps byte-packed or
"naturally aligned".  The reason is that vectors are primitive types.
As such, their size is target independent, which implies that the
spacing between consecutive elements must be target independent.
Thus it is not possible for vector elements to be aligned according
to the target ABI.  This leaves only a few possibilities: (a) align
elements to a multiple of their size (rounded up to the nearest
power of two), which should work on all platforms (but for example
would align x86 long double to 16 bytes rather than to 4 bytes
on x86 linux); or (b) don't align them at all, start each element
at the next available byte; or (c) bit-pack the elements.  The
advantage of bit-packing is that then there are no gaps, which
avoids problems with sroa etc.

That said, I don't know what codegen currently does exactly,
but I'm sure it doesn't bitpack, and I suspect it just ignores
these issues and codegens to a target dependent size, i.e.
inconsistently with what getPrimitiveSizeInBits returns.
Quuxplusone commented 16 years ago
PPS: The bitpacking or not of vectors is irrelevant to the problem
exposed by this PR - the lack of alignment info for 32 bit vectors.
Quuxplusone commented 16 years ago
Patch here:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20080107/057009.html

Testcase here: test/CodeGen/Generic/bool-vector.ll

Thanks!

-Chris
Quuxplusone commented 13 years ago

The testcase in 45796 breaks when enabling -promote-elements (required for vector-select). The codegen does not support store-trunc of .