So I finally came up with a way to fix the alignment issue.
Please review the attached patch & test case.
The idea is to add a parameter to ABIArgInfo::getDirect that is used to set the
size of a padding inserted before a byval aggregate. The test case shows
padding is inserted before the first element of a 16-byte aligned byval
structure when the first element is not aligned to a 16-byte boundary.
________________________________________
From: Eli Friedman [[email protected]]
Sent: Monday, November 07, 2011 8:04 PM
To: Hatanaka, Akira
Cc: [email protected]
Subject: Re: [cfe-commits] r143596 - /cfe/trunk/lib/CodeGen/TargetInfo.cpp
On Mon, Nov 7, 2011 at 7:27 PM, Hatanaka, Akira <[email protected]> wrote:
> Just to add to my previous email,
>
> tail call void @callee(i32 3, double %0, i64 %1, i64 %2, i64 %3) nounwind
>
> Without alignment information attached to "double %0", this is how the
> backend will pass arguments in registers, when the function above is lowered:
> i32 3 => 1st integer register
> double %0 => 2nd FP register
> i64 %1 => 3rd integer register
> i64 %2, i64 %3 => 4th and 5th integer register
>
> The correct way to pass arguments is the following:
> i32 3 => 1st integer register
> double %0 => 3rd FP register
> i64 %1 => 4th integer register
> i64 %2, i64 %3 => 5th and 6th integer register
As you've figured out, you can't attach alignment to scalars. IIRC,
x86 doesn't run into this issue. Something like "tail call void
@callee(i32 3, i32 undef, double %0, i64 %1, i64 %2, i64 %3) nounwind"
probably does what you want. I'm not entirely sure how you would
convince clang to emit that, though; it might require extending the
ABI interface a bit.
-Eli
// RUN: %clang_cc1 -emit-llvm %s -o /dev/null
/* Regression test. Just compile .c -> .ll to test */
int foo(void) {
unsigned char *pp;
unsigned w_cnt;
w_cnt += *pp;
return w_cnt;
}
Index: lib/CodeGen/ABIInfo.h
===================================================================
--- lib/CodeGen/ABIInfo.h (revision 147536)
+++ lib/CodeGen/ABIInfo.h (working copy)
@@ -69,19 +69,22 @@
private:
Kind TheKind;
llvm::Type *TypeData;
- unsigned UIntData;
+ unsigned UIntData0;
+ unsigned UIntData1;
bool BoolData0;
bool BoolData1;
- ABIArgInfo(Kind K, llvm::Type *TD=0,
- unsigned UI=0, bool B0 = false, bool B1 = false)
- : TheKind(K), TypeData(TD), UIntData(UI), BoolData0(B0), BoolData1(B1) {}
+ ABIArgInfo(Kind K, llvm::Type *TD=0, unsigned UI0=0, unsigned UI1=0,
+ bool B0 = false, bool B1 = false)
+ : TheKind(K), TypeData(TD), UIntData0(UI0), UIntData1(UI1), BoolData0(B0),
+ BoolData1(B1) {}
public:
- ABIArgInfo() : TheKind(Direct), TypeData(0), UIntData(0) {}
+ ABIArgInfo() : TheKind(Direct), TypeData(0), UIntData0(0), UIntData1(0) {}
- static ABIArgInfo getDirect(llvm::Type *T = 0, unsigned Offset = 0) {
- return ABIArgInfo(Direct, T, Offset);
+ static ABIArgInfo getDirect(llvm::Type *T = 0, unsigned Offset = 0,
+ unsigned PaddingSize = 0) {
+ return ABIArgInfo(Direct, T, Offset, PaddingSize);
}
static ABIArgInfo getExtend(llvm::Type *T = 0) {
return ABIArgInfo(Extend, T, 0);
@@ -91,7 +94,7 @@
}
static ABIArgInfo getIndirect(unsigned Alignment, bool ByVal = true
, bool Realign = false) {
- return ABIArgInfo(Indirect, 0, Alignment, ByVal, Realign);
+ return ABIArgInfo(Indirect, 0, Alignment, 0, ByVal, Realign);
}
static ABIArgInfo getExpand() {
return ABIArgInfo(Expand);
@@ -111,8 +114,14 @@
// Direct/Extend accessors
unsigned getDirectOffset() const {
assert((isDirect() || isExtend()) && "Not a direct or extend kind");
- return UIntData;
+ return UIntData0;
}
+
+ unsigned getPaddingSize() const {
+ assert(isDirect() && "Not a direct kind");
+ return UIntData1;
+ }
+
llvm::Type *getCoerceToType() const {
assert(canHaveCoerceToType() && "Invalid kind!");
return TypeData;
@@ -126,7 +135,7 @@
// Indirect accessors
unsigned getIndirectAlign() const {
assert(TheKind == Indirect && "Invalid kind!");
- return UIntData;
+ return UIntData0;
}
bool getIndirectByVal() const {
Index: lib/CodeGen/TargetInfo.cpp
===================================================================
--- lib/CodeGen/TargetInfo.cpp (revision 147536)
+++ lib/CodeGen/TargetInfo.cpp (working copy)
@@ -3047,7 +3047,7 @@
ABIInfo(CGT), IsO32(_IsO32), MinABIStackAlignInBytes(IsO32 ? 4 : 8) {}
ABIArgInfo classifyReturnType(QualType RetTy) const;
- ABIArgInfo classifyArgumentType(QualType RetTy) const;
+ ABIArgInfo classifyArgumentType(QualType RetTy, uint64_t &Offset) const;
virtual void computeInfo(CGFunctionInfo &FI) const;
virtual llvm::Value *EmitVAArg(llvm::Value *VAListAddr, QualType Ty,
CodeGenFunction &CGF) const;
@@ -3132,28 +3132,40 @@
return llvm::StructType::get(getVMContext(), ArgList);
}
-ABIArgInfo MipsABIInfo::classifyArgumentType(QualType Ty) const {
+ABIArgInfo
+MipsABIInfo::classifyArgumentType(QualType Ty, uint64_t &Offset) const {
if (isAggregateTypeForABI(Ty)) {
// Ignore empty aggregates.
- if (getContext().getTypeSize(Ty) == 0)
+ uint64_t TySize = getContext().getTypeSize(Ty);
+ if (TySize == 0)
return ABIArgInfo::getIgnore();
// Records with non trivial destructors/constructors should not be passed
// by value.
- if (isRecordWithNonTrivialDestructorOrCopyConstructor(Ty))
+ if (isRecordWithNonTrivialDestructorOrCopyConstructor(Ty)) {
+ Offset += 8;
return ABIArgInfo::getIndirect(0, /*ByVal=*/false);
+ }
- llvm::Type *ResType;
- if ((ResType = HandleStructTy(Ty)))
- return ABIArgInfo::getDirect(ResType);
-
- return ABIArgInfo::getIndirect(0);
+ // If we have reached here, aggregates are passed either indirectly via a
+ // byval pointer or directly by coercing to another structure type. In the
+ // latter case, padding is inserted if the offset of the aggregate is
+ // unaligned.
+ llvm::Type *ResType = HandleStructTy(Ty);
+ uint64_t Align = getContext().getTypeAlign(Ty) / 8;
+ assert(Align <= 16 && "Alignment larger than 16 not handled.");
+ unsigned PaddingSize = (Offset % Align) * 8;
+ Offset = llvm::RoundUpToAlignment(Offset, std::max(Align, (uint64_t)8));
+ Offset += llvm::RoundUpToAlignment(TySize, 8);
+ return ResType ? ABIArgInfo::getDirect(ResType, 0, PaddingSize) :
+ ABIArgInfo::getIndirect(0);
}
// Treat an enum type as its underlying type.
if (const EnumType *EnumTy = Ty->getAs<EnumType>())
Ty = EnumTy->getDecl()->getIntegerType();
+ Offset += 8;
return (Ty->isPromotableIntegerType() ?
ABIArgInfo::getExtend() : ABIArgInfo::getDirect());
}
@@ -3230,9 +3242,10 @@
void MipsABIInfo::computeInfo(CGFunctionInfo &FI) const {
FI.getReturnInfo() = classifyReturnType(FI.getReturnType());
+ uint64_t Offset = 0;
for (CGFunctionInfo::arg_iterator it = FI.arg_begin(), ie = FI.arg_end();
it != ie; ++it)
- it->info = classifyArgumentType(it->type);
+ it->info = classifyArgumentType(it->type, Offset);
}
llvm::Value* MipsABIInfo::EmitVAArg(llvm::Value *VAListAddr, QualType Ty,
Index: lib/CodeGen/CGCall.cpp
===================================================================
--- lib/CodeGen/CGCall.cpp (revision 147536)
+++ lib/CodeGen/CGCall.cpp (working copy)
@@ -691,6 +691,9 @@
// generally likes scalar values better than FCAs.
llvm::Type *argType = argAI.getCoerceToType();
if (llvm::StructType *st = dyn_cast<llvm::StructType>(argType)) {
+ unsigned Size = argAI.getPaddingSize();
+ if (Size)
+ argTypes.push_back(llvm::IntegerType::get(getLLVMContext(), Size));
for (unsigned i = 0, e = st->getNumElements(); i != e; ++i)
argTypes.push_back(st->getElementType(i));
} else {
@@ -842,7 +845,8 @@
if (llvm::StructType *STy =
dyn_cast<llvm::StructType>(AI.getCoerceToType()))
- Index += STy->getNumElements()-1; // 1 will be added below.
+ // 1 will be added below.
+ Index += STy->getNumElements() + (AI.getPaddingSize() != 0) - 1;
break;
case ABIArgInfo::Indirect:
@@ -1030,7 +1034,8 @@
if (llvm::StructType *STy =
dyn_cast<llvm::StructType>(ArgI.getCoerceToType())) {
Ptr = Builder.CreateBitCast(Ptr, llvm::PointerType::getUnqual(STy));
-
+ if (ArgI.getPaddingSize())
+ ++AI;
for (unsigned i = 0, e = STy->getNumElements(); i != e; ++i) {
assert(AI != Fn->arg_end() && "Argument mismatch!");
AI->setName(Arg->getName() + ".coerce" + Twine(i));
@@ -1705,6 +1710,12 @@
dyn_cast<llvm::StructType>(ArgInfo.getCoerceToType())) {
SrcPtr = Builder.CreateBitCast(SrcPtr,
llvm::PointerType::getUnqual(STy));
+ unsigned Size = ArgInfo.getPaddingSize();
+ if (Size) {
+ llvm::Type *Ty = llvm::IntegerType::get(getLLVMContext(), Size);
+ Args.push_back(llvm::UndefValue::get(Ty));
+ ++IRArgNo;
+ }
for (unsigned i = 0, e = STy->getNumElements(); i != e; ++i) {
llvm::Value *EltPtr = Builder.CreateConstGEP2_32(SrcPtr, 0, i);
llvm::LoadInst *LI = Builder.CreateLoad(EltPtr);
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
