llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.68k stars 11.86k forks source link

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

Closed llvmbot closed 13 years ago

llvmbot commented 16 years ago
Bugzilla Link 1845
Resolution FIXED
Resolved on Sep 22, 2011 13:33
Version 2.1
OS Windows XP
Attachments Modification of the fibonacci example
Reporter LLVM Bugzilla Contributor
CC @asl,@lattner

Extended Description

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)

llvmbot 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 .

lattner 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

llvmbot 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.

llvmbot 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.

llvmbot commented 16 years ago

I think this one is for Chris, since he wrote the alignment info code for vectors.

llvmbot 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.

llvmbot commented 16 years ago

OK, I will take a look.

llvmbot 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.

llvmbot 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 :)

llvmbot commented 16 years ago

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

llvmbot commented 16 years ago

assigned to @lattner