[LLVMdev] RFC: Store alignment should be LValue alignment, not source alignment
Evan Cheng
evan.cheng at apple.com
Fri Jan 9 11:05:45 PST 2009
Hi all,
Please review this patch. It's fixing PR3232 comment #8. Function bar
from 2008-03-24-BitFiled-And-Alloca.c compiles to:
%struct.Key = type { { i32, i32 } }
...
define i32 @bar(i64 %key_token2) nounwind {
entry:
%key_token2_addr = alloca i64 ; <i64*> [#uses=2]
%retval = alloca i32 ; <i32*> [#uses=2]
%iospec = alloca %struct.Key ; <%struct.Key*>
[#uses=3]
%ret = alloca i32 ; <i32*> [#uses=2]
%0 = alloca i32 ; <i32*> [#uses=2]
%"alloca point" = bitcast i32 0 to i32 ; <i32>
[#uses=0]
store i64 %key_token2, i64* %key_token2_addr
%1 = getelementptr %struct.Key* %iospec, i32 0, i32
0 ; <{ i32, i32 }*> [#uses=2]
%2 = getelementptr { i32, i32 }* %1, i32 0, i32 0 ;
<i32*> [#uses=1]
store i32 0, i32* %2, align 4
%3 = getelementptr { i32, i32 }* %1, i32 0, i32
1 ; <i32*> [#uses=1]
store i32 0, i32* %3, align 4
%4 = getelementptr %struct.Key* %iospec, i32 0, i32
0 ; <{ i32, i32 }*> [#uses=1]
%5 = bitcast { i32, i32 }* %4 to i64* ; <i64*>
[#uses=1]
%6 = load i64* %key_token2_addr, align 8 ;
<i64> [#uses=1]
store i64 %6, i64* %5, align 8
...
The store alignment 8 is wrong. The address iospec has 4-byte
alignment. The problem is llvm-gcc TreeToLLVM::EmitMODIFY_EXPR:
LValue LV = EmitLV(lhs);
bool isVolatile = TREE_THIS_VOLATILE(lhs);
unsigned Alignment = expr_align(exp) / 8
It's using the alignment of the expression, rather than the memory
object of LValue.
The patch saves the alignment of the memory object in LValue returned
by EmitLV(). Please review it carefully as I am not entirely
comfortable hacking on llvm-gcc. :-)
Evan
Index: gcc/llvm-convert.cpp
===================================================================
--- gcc/llvm-convert.cpp (revision 61984)
+++ gcc/llvm-convert.cpp (working copy)
@@ -1150,9 +1150,18 @@ LValue TreeToLLVM::EmitLV(tree exp) {
case IMAGPART_EXPR: return EmitLV_XXXXPART_EXPR(exp, 1);
// Constants.
- case LABEL_DECL: return TreeConstantToLLVM::EmitLV_LABEL_DECL(exp);
- case COMPLEX_CST: return
LValue(TreeConstantToLLVM::EmitLV_COMPLEX_CST(exp));
- case STRING_CST: return
LValue(TreeConstantToLLVM::EmitLV_STRING_CST(exp));
+ case LABEL_DECL: {
+ Value *Ptr = TreeConstantToLLVM::EmitLV_LABEL_DECL(exp);
+ return LValue(Ptr, DECL_ALIGN(exp) / 8);
+ }
+ case COMPLEX_CST: {
+ Value *Ptr = TreeConstantToLLVM::EmitLV_COMPLEX_CST(exp);
+ return LValue(Ptr, TYPE_ALIGN(TREE_TYPE(exp)) / 8);
+ }
+ case STRING_CST: {
+ Value *Ptr = TreeConstantToLLVM::EmitLV_STRING_CST(exp);
+ return LValue(Ptr, TYPE_ALIGN(TREE_TYPE(exp)) / 8);
+ }
// Type Conversion.
case VIEW_CONVERT_EXPR: return EmitLV_VIEW_CONVERT_EXPR(exp);
@@ -1165,9 +1174,11 @@ LValue TreeToLLVM::EmitLV(tree exp) {
case WITH_SIZE_EXPR:
// The address is the address of the operand.
return EmitLV(TREE_OPERAND(exp, 0));
- case INDIRECT_REF:
+ case INDIRECT_REF: {
// The lvalue is just the address.
- return Emit(TREE_OPERAND(exp, 0), 0);
+ tree Op = TREE_OPERAND(exp, 0);
+ return LValue(Emit(Op, 0), expr_align(Op) / 8);
+ }
}
}
@@ -2290,7 +2301,7 @@ Value *TreeToLLVM::EmitLoadOfLValue(tree
LValue LV = EmitLV(exp);
bool isVolatile = TREE_THIS_VOLATILE(exp);
const Type *Ty = ConvertType(TREE_TYPE(exp));
- unsigned Alignment = expr_align(exp) / 8;
+ unsigned Alignment = LV.getAlignment();
if (TREE_CODE(exp) == COMPONENT_REF)
if (const StructType *STy =
dyn_cast<StructType>(ConvertType(TREE_TYPE(TREE_OPERAND(exp,
0)))))
@@ -2963,7 +2974,7 @@ Value *TreeToLLVM::EmitMODIFY_EXPR(tree
LValue LV = EmitLV(lhs);
bool isVolatile = TREE_THIS_VOLATILE(lhs);
- unsigned Alignment = expr_align(lhs) / 8;
+ unsigned Alignment = LV.getAlignment();
if (TREE_CODE(lhs) == COMPONENT_REF)
if (const StructType *STy =
dyn_cast<StructType>(ConvertType(TREE_TYPE(TREE_OPERAND(lhs,
0)))))
@@ -3157,7 +3168,7 @@ Value *TreeToLLVM::EmitVIEW_CONVERT_EXPR
LValue LV = EmitLV(Op);
assert(!LV.isBitfield() && "Expected an aggregate operand!");
bool isVolatile = TREE_THIS_VOLATILE(Op);
- unsigned Alignment = expr_align(Op) / 8;
+ unsigned Alignment = LV.getAlignment();
EmitAggregateCopy(Target, MemRef(LV.Ptr, Alignment, isVolatile),
TREE_TYPE(exp));
@@ -5885,9 +5896,10 @@ LValue TreeToLLVM::EmitLV_DECL(tree exp)
Value *Decl = DECL_LLVM(exp);
if (Decl == 0) {
if (errorcount || sorrycount) {
- const PointerType *Ty =
- PointerType::getUnqual(ConvertType(TREE_TYPE(exp)));
- return ConstantPointerNull::get(Ty);
+ const Type *Ty = ConvertType(TREE_TYPE(exp));
+ const PointerType *PTy = PointerType::getUnqual(Ty);
+ LValue LV(ConstantPointerNull::get(PTy),
TD.getABITypeAlignment(Ty));
+ return LV;
}
assert(0 && "INTERNAL ERROR: Referencing decl that hasn't been
laid out");
abort();
@@ -5924,7 +5936,13 @@ LValue TreeToLLVM::EmitLV_DECL(tree exp)
// type void.
if (Ty == Type::VoidTy) Ty = StructType::get(NULL, NULL);
const PointerType *PTy = PointerType::getUnqual(Ty);
- return BitCastToType(Decl, PTy);
+ unsigned Alignment = Ty->isSized() ? TD.getABITypeAlignment(Ty) : 1;
+ if (DECL_ALIGN_UNIT(exp)) {
+ if (DECL_USER_ALIGN(exp) || Alignment <
(unsigned)DECL_ALIGN_UNIT(exp))
+ Alignment = DECL_ALIGN_UNIT(exp);
+ }
+
+ return LValue(BitCastToType(Decl, PTy), Alignment);
}
LValue TreeToLLVM::EmitLV_ARRAY_REF(tree exp) {
@@ -5932,22 +5950,23 @@ LValue TreeToLLVM::EmitLV_ARRAY_REF(tree
// of ElementTy in the case of ARRAY_RANGE_REF.
tree Array = TREE_OPERAND(exp, 0);
- tree ArrayType = TREE_TYPE(Array);
+ tree ArrayTreeType = TREE_TYPE(Array);
tree Index = TREE_OPERAND(exp, 1);
tree IndexType = TREE_TYPE(Index);
- tree ElementType = TREE_TYPE(ArrayType);
+ tree ElementType = TREE_TYPE(ArrayTreeType);
- assert((TREE_CODE (ArrayType) == ARRAY_TYPE ||
- TREE_CODE (ArrayType) == POINTER_TYPE ||
- TREE_CODE (ArrayType) == REFERENCE_TYPE ||
- TREE_CODE (ArrayType) == BLOCK_POINTER_TYPE) &&
+ assert((TREE_CODE (ArrayTreeType) == ARRAY_TYPE ||
+ TREE_CODE (ArrayTreeType) == POINTER_TYPE ||
+ TREE_CODE (ArrayTreeType) == REFERENCE_TYPE ||
+ TREE_CODE (ArrayTreeType) == BLOCK_POINTER_TYPE) &&
"Unknown ARRAY_REF!");
// As an LLVM extension, we allow ARRAY_REF with a pointer as the
first
// operand. This construct maps directly to a getelementptr
instruction.
Value *ArrayAddr;
+ unsigned ArrayAlign;
- if (TREE_CODE(ArrayType) == ARRAY_TYPE) {
+ if (TREE_CODE(ArrayTreeType) == ARRAY_TYPE) {
// First subtract the lower bound, if any, in the type of the
index.
tree LowerBound = array_ref_low_bound(exp);
if (!integer_zerop(LowerBound))
@@ -5956,8 +5975,10 @@ LValue TreeToLLVM::EmitLV_ARRAY_REF(tree
LValue ArrayAddrLV = EmitLV(Array);
assert(!ArrayAddrLV.isBitfield() && "Arrays cannot be
bitfields!");
ArrayAddr = ArrayAddrLV.Ptr;
+ ArrayAlign = ArrayAddrLV.Alignment;
} else {
ArrayAddr = Emit(Array, 0);
+ ArrayAlign = expr_align(ArrayTreeType) / 8;
}
Value *IndexVal = Emit(Index, 0);
@@ -5971,20 +5992,27 @@ LValue TreeToLLVM::EmitLV_ARRAY_REF(tree
IndexVal = CastToSIntType(IndexVal, IntPtrTy);
// If this is an index into an LLVM array, codegen as a GEP.
- if (isArrayCompatible(ArrayType)) {
+ if (isArrayCompatible(ArrayTreeType)) {
Value *Idxs[2] = { ConstantInt::get(Type::Int32Ty, 0), IndexVal };
Value *Ptr = Builder.CreateGEP(ArrayAddr, Idxs, Idxs + 2);
- return BitCastToType(Ptr,
-
PointerType::getUnqual(ConvertType(TREE_TYPE(exp))));
+ const Type *ATy = cast<PointerType>(ArrayAddr->getType())-
>getElementType();
+ const Type *ElementTy = cast<ArrayType>(ATy)->getElementType();
+ unsigned Alignment = MinAlign(ArrayAlign,
TD.getABITypeSize(ElementTy));
+ return LValue(BitCastToType(Ptr,
+
PointerType::getUnqual(ConvertType(TREE_TYPE(exp)))),
+ Alignment);
}
// If we are indexing over a fixed-size type, just use a GEP.
- if (isSequentialCompatible(ArrayType)) {
- const Type *PtrElementTy =
PointerType::getUnqual(ConvertType(ElementType));
+ if (isSequentialCompatible(ArrayTreeType)) {
+ const Type *ElementTy = ConvertType(ElementType);
+ const Type *PtrElementTy = PointerType::getUnqual(ElementTy);
ArrayAddr = BitCastToType(ArrayAddr, PtrElementTy);
Value *Ptr = Builder.CreateGEP(ArrayAddr, IndexVal);
- return BitCastToType(Ptr,
-
PointerType::getUnqual(ConvertType(TREE_TYPE(exp))));
+ unsigned Alignment = MinAlign(ArrayAlign,
TD.getABITypeAlignment(ElementTy));
+ return LValue(BitCastToType(Ptr,
+
PointerType::getUnqual(ConvertType(TREE_TYPE(exp)))),
+ Alignment);
}
// Otherwise, just do raw, low-level pointer arithmetic. FIXME:
this could be
@@ -5992,14 +6020,21 @@ LValue TreeToLLVM::EmitLV_ARRAY_REF(tree
// float foo(int w, float A[][w], int g) { return A[g][0]; }
ArrayAddr = BitCastToType(ArrayAddr,
PointerType::getUnqual(Type::Int8Ty));
- if (VOID_TYPE_P(TREE_TYPE(ArrayType)))
- return Builder.CreateGEP(ArrayAddr, IndexVal);
+ if (VOID_TYPE_P(TREE_TYPE(ArrayTreeType))) {
+ unsigned Alignment = MinAlign(ArrayAlign,
+
TD.getABITypeAlignment(Type::Int8Ty));
+ return LValue(Builder.CreateGEP(ArrayAddr, IndexVal), Alignment);
+ }
Value *TypeSize = Emit(array_ref_element_size(exp), 0);
TypeSize = CastToUIntType(TypeSize, IntPtrTy);
IndexVal = Builder.CreateMul(IndexVal, TypeSize);
Value *Ptr = Builder.CreateGEP(ArrayAddr, IndexVal);
- return
BitCastToType(Ptr,PointerType::getUnqual(ConvertType(TREE_TYPE(exp))));
+ unsigned Alignment = MinAlign(ArrayAlign,
+ cast<ConstantInt>(IndexVal)-
>getZExtValue());
+ return LValue(BitCastToType(Ptr,
+
PointerType::getUnqual(ConvertType(TREE_TYPE(exp)))),
+ Alignment);
}
/// getFieldOffsetInBits - Return the offset (in bits) of a
FIELD_DECL in a
@@ -6028,8 +6063,9 @@ static unsigned getComponentRefOffsetInB
LValue TreeToLLVM::EmitLV_COMPONENT_REF(tree exp) {
LValue StructAddrLV = EmitLV(TREE_OPERAND(exp, 0));
- tree FieldDecl = TREE_OPERAND(exp, 1);
-
+ tree FieldDecl = TREE_OPERAND(exp, 1);
+ unsigned LVAlign = DECL_PACKED(FieldDecl) ? 1 :
StructAddrLV.Alignment;
+
assert((TREE_CODE(DECL_CONTEXT(FieldDecl)) == RECORD_TYPE ||
TREE_CODE(DECL_CONTEXT(FieldDecl)) == UNION_TYPE ||
TREE_CODE(DECL_CONTEXT(FieldDecl)) == QUAL_UNION_TYPE));
@@ -6064,7 +6100,9 @@ LValue TreeToLLVM::EmitLV_COMPONENT_REF(
// the offset from BitStart.
if (MemberIndex) {
const StructLayout *SL =
TD.getStructLayout(cast<StructType>(StructTy));
- BitStart -= SL->getElementOffset(MemberIndex) * 8;
+ unsigned Offset = SL->getElementOffset(MemberIndex);
+ BitStart -= Offset * 8;
+ LVAlign = MinAlign(LVAlign, Offset);
}
// If the FIELD_DECL has an annotate attribute on it, emit it.
@@ -6130,7 +6168,7 @@ LValue TreeToLLVM::EmitLV_COMPONENT_REF(
if (AnnotateAttr)
AnnotateAttr = lookup_attribute("annotate", AnnotateAttr);
}
- }
+ }
} else {
Value *Offset = Emit(field_offset, 0);
@@ -6150,6 +6188,7 @@ LValue TreeToLLVM::EmitLV_COMPONENT_REF(
Offset = Builder.CreateAdd(Offset,
ConstantInt::get(Offset->getType(), ByteOffset));
BitStart -= ByteOffset*8;
+ LVAlign = MinAlign(LVAlign, ByteOffset);
}
Value *Ptr = CastToType(Instruction::PtrToInt, StructAddrLV.Ptr,
@@ -6221,6 +6260,7 @@ LValue TreeToLLVM::EmitLV_COMPONENT_REF(
// Compute the byte offset, and add it to the pointer.
unsigned ByteOffset = NumAlignmentUnits*ByteAlignment;
+ LVAlign = MinAlign(LVAlign, ByteOffset);
Constant *Offset = ConstantInt::get(TD.getIntPtrType(),
ByteOffset);
FieldPtr = CastToType(Instruction::PtrToInt, FieldPtr,
@@ -6242,17 +6282,18 @@ LValue TreeToLLVM::EmitLV_COMPONENT_REF(
// Okay, everything is good. Return this as a bitfield if we
can't
// return it as a normal l-value. (e.g. "struct X { int X :
32 };" ).
+ // Conservatively return LValue with alignment 1.
if (BitfieldSize != LLVMValueBitSize || BitStart != 0)
- return LValue(FieldPtr, BitStart, BitfieldSize);
+ return LValue(FieldPtr, 1, BitStart, BitfieldSize);
} else {
// Make sure we return a pointer to the right type.
- FieldPtr = BitCastToType(FieldPtr,
-
PointerType::getUnqual(ConvertType(TREE_TYPE(exp))));
+ const Type *EltTy = ConvertType(TREE_TYPE(exp));
+ FieldPtr = BitCastToType(FieldPtr, PointerType::getUnqual(EltTy));
}
assert(BitStart == 0 &&
"It's a bitfield reference or we didn't get to the field!");
- return LValue(FieldPtr);
+ return LValue(FieldPtr, LVAlign);
}
LValue TreeToLLVM::EmitLV_BIT_FIELD_REF(tree exp) {
@@ -6284,17 +6325,27 @@ LValue TreeToLLVM::EmitLV_BIT_FIELD_REF(
}
// If this is referring to the whole field, return the whole thing.
- if (BitStart == 0 && BitSize == ValueSizeInBits)
- return LValue(BitCastToType(Ptr.Ptr,
PointerType::getUnqual(ValTy)));
+ if (BitStart == 0 && BitSize == ValueSizeInBits) {
+ return LValue(BitCastToType(Ptr.Ptr,
PointerType::getUnqual(ValTy)),
+ Ptr.Alignment);
+ }
- return LValue(BitCastToType(Ptr.Ptr,
PointerType::getUnqual(ValTy)), BitStart,
- BitSize);
+ return LValue(BitCastToType(Ptr.Ptr,
PointerType::getUnqual(ValTy)), 1,
+ BitStart, BitSize);
}
LValue TreeToLLVM::EmitLV_XXXXPART_EXPR(tree exp, unsigned Idx) {
LValue Ptr = EmitLV(TREE_OPERAND(exp, 0));
- assert(!Ptr.isBitfield() && "BIT_FIELD_REF operands cannot be
bitfields!");
- return LValue(Builder.CreateStructGEP(Ptr.Ptr, Idx));
+ assert(!Ptr.isBitfield() &&
+ "REALPART_EXPR / IMAGPART_EXPR operands cannot be
bitfields!");
+ unsigned Alignment;
+ if (Idx == 0)
+ // REALPART alignment is same as the complex operand.
+ Alignment = Ptr.Alignment;
+ else
+ // IMAGPART alignment = MinAlign(Ptr.Alignment, sizeof field);
+ Alignment = MinAlign(Ptr.Alignment, TD.getABITypeSize(Ptr.Ptr-
>getType()));
+ return LValue(Builder.CreateStructGEP(Ptr.Ptr, Idx), Alignment);
}
LValue TreeToLLVM::EmitLV_VIEW_CONVERT_EXPR(tree exp) {
@@ -6310,24 +6361,30 @@ LValue TreeToLLVM::EmitLV_VIEW_CONVERT_E
} else {
// If the input is a scalar, emit to a temporary.
Value *Dest = CreateTemporary(ConvertType(TREE_TYPE(Op)));
- Builder.CreateStore(Emit(Op, 0), Dest);
+ StoreInst *S = Builder.CreateStore(Emit(Op, 0), Dest);
// The type is the type of the expression.
Dest = BitCastToType(Dest,
PointerType::getUnqual(ConvertType(TREE_TYPE(exp))));
- return LValue(Dest);
+ return LValue(Dest, S->getAlignment());
}
}
LValue TreeToLLVM::EmitLV_EXC_PTR_EXPR(tree exp) {
CreateExceptionValues();
// Cast the address pointer to the expected type.
- return BitCastToType(ExceptionValue,
-
PointerType::getUnqual(ConvertType(TREE_TYPE(exp))));
+ unsigned Alignment =
TD.getABITypeAlignment(cast<PointerType>(ExceptionValue->
+ getType())-
>getElementType());
+ return LValue(BitCastToType(ExceptionValue,
+
PointerType::getUnqual(ConvertType(TREE_TYPE(exp)))),
+ Alignment);
}
LValue TreeToLLVM::EmitLV_FILTER_EXPR(tree exp) {
CreateExceptionValues();
- return ExceptionSelectorValue;
+ unsigned Alignment =
+ TD.getABITypeAlignment(cast<PointerType>(ExceptionSelectorValue->
+ getType())-
>getElementType());
+ return LValue(ExceptionSelectorValue, Alignment);
}
//
=
=
=----------------------------------------------------------------------
===//
Index: gcc/llvm-internal.h
===================================================================
--- gcc/llvm-internal.h (revision 61984)
+++ gcc/llvm-internal.h (working copy)
@@ -251,23 +251,30 @@ struct MemRef {
};
/// LValue - This struct represents an lvalue in the program. In
particular,
-/// the Ptr member indicates the memory that the lvalue lives in. If
this is
-/// a bitfield reference, BitStart indicates the first bit in the
memory that
-/// is part of the field and BitSize indicates the extent.
+/// the Ptr member indicates the memory that the lvalue lives in.
Alignment
+/// is the alignment of the memory (in bytes).If this is a bitfield
reference,
+/// BitStart indicates the first bit in the memory that is part of
the field
+/// and BitSize indicates the extent.
///
/// "LValue" is intended to be a light-weight object passed around
by-value.
struct LValue {
Value *Ptr;
+ unsigned char Alignment;
unsigned char BitStart;
unsigned char BitSize;
- LValue(Value *P) : Ptr(P), BitStart(255), BitSize(255) {}
- LValue(Value *P, unsigned BSt, unsigned BSi)
- : Ptr(P), BitStart(BSt), BitSize(BSi) {
+ LValue(Value *P, unsigned Align)
+ : Ptr(P), Alignment(Align), BitStart(255), BitSize(255) {}
+ LValue(Value *P, unsigned Align, unsigned BSt, unsigned BSi)
+ : Ptr(P), Alignment(Align), BitStart(BSt), BitSize(BSi) {
assert(BitStart == BSt && BitSize == BSi &&
"Bit values larger than 256?");
+ }
+
+ unsigned getAlignment() const {
+ assert(Alignment && "LValue alignment cannot be zero!");
+ return Alignment;
}
-
bool isBitfield() const { return BitStart != 255; }
};
More information about the llvm-dev
mailing list