[llvm-commits] [llvm-gcc-4.2] r93368 - /llvm-gcc-4.2/trunk/gcc/llvm-abi.h
Bob Wilson
bob.wilson at apple.com
Wed Jan 13 15:21:51 PST 2010
Author: bwilson
Date: Wed Jan 13 17:21:50 2010
New Revision: 93368
URL: http://llvm.org/viewvc/llvm-project?rev=93368&view=rev
Log:
Fix problems passing aggregates for ARM. There are several components of this:
* The PassInIntegerRegisters method needs to divide the aggregate into 32-bit
units and not use 64-bit integers for ARM. The patch adds a check to see if
the target supports 64-bit integers as a legal type. If not, it will use
32-bit integers. It is possible that some target might want to do something
different, but this seems like a much more reasonable default behavior.
Every 32-bit target that I've worked with divides arguments into 32-bit units
like this.
* If there are any "leftover" bytes that do not fit into the array of 32- or
64-bit integers, they need to be passed as a single integer that is big
enough to hold them (possibly bigger). The previous code that split those
bytes into multiple integers was wrong for any target ABI where each of those
integers will be passed in a separate register.
* For the last "leftover" value, the HandleScalarArgument method needs to have
its "RealSize" argument set. This is to handle cases where the "leftover"
integer type is bigger than than actual data (e.g., using an i32 to pass 3
bytes). This is already done by the PassInMixedRegisters code.
There was some discussion on the mailing list, following previous attempts
to fix this, about using "byval" to pass aggregates for ARM. I looked into
that and decided that it would be more work than I have time for right now.
This patch makes the default behavior much more reasonable. It also correctly
handles the ARM AAPCS ABI for both big- and little-endian targets and the
APCS ABI for little-endian targets. The big-endian APCS ABI pads small
aggregates (smaller than 32-bits) differently, and "byval" might still be
the best way to handle that, if anyone even cares about big-endian APCS.
I've run the GCC "compat" testsuite for x86-32, x86-64, powerpc and ARM (all on Darwin) to verify that this does not break anything. For ARM (little-endian
APCS), it fixes almost all of the previous failures:
Running ....testsuite/gcc.dg/compat/compat.exp ...
WARNING: program timed out.
FAIL: gcc.dg/compat/struct-by-value-15 c_compat_x_tst.o compile
WARNING: program timed out.
FAIL: gcc.dg/compat/struct-by-value-17 c_compat_x_tst.o compile
WARNING: program timed out.
FAIL: gcc.dg/compat/struct-by-value-18 c_compat_x_tst.o compile
FAIL: gcc.dg/compat/struct-by-value-9 c_compat_x_tst.o-c_compat_y_tst.o execute
FAIL: gcc.dg/compat/struct-by-value-9 c_compat_x_tst.o-c_compat_y_alt.o execute
FAIL: gcc.dg/compat/vector-1 c_compat_x_tst.o-c_compat_y_alt.o execute
FAIL: gcc.dg/compat/vector-1 c_compat_x_alt.o-c_compat_y_tst.o execute
(I was testing a debug build, so hopefully those timeouts would not occur with
an optimized build.) The other targets were unaffected by this change.
This fixes pr5406 and Apple radars 7226213 and 7226380. Many thanks to
Rafael Ávila de Espíndola for his help with this!
Modified:
llvm-gcc-4.2/trunk/gcc/llvm-abi.h
Modified: llvm-gcc-4.2/trunk/gcc/llvm-abi.h
URL: http://llvm.org/viewvc/llvm-project/llvm-gcc-4.2/trunk/gcc/llvm-abi.h?rev=93368&r1=93367&r2=93368&view=diff
==============================================================================
--- llvm-gcc-4.2/trunk/gcc/llvm-abi.h (original)
+++ llvm-gcc-4.2/trunk/gcc/llvm-abi.h Wed Jan 13 17:21:50 2010
@@ -610,8 +610,9 @@
// from Int64 alignment. ARM backend needs this.
unsigned Align = TYPE_ALIGN(type)/8;
unsigned Int64Align =
- getTargetData().getABITypeAlignment(Type::getInt64Ty(getGlobalContext()));
- bool UseInt64 = DontCheckAlignment ? true : (Align >= Int64Align);
+ getTargetData().getABITypeAlignment(Type::getInt64Ty(getGlobalContext()));
+ bool UseInt64 = (getTargetData().isLegalInteger(64) &&
+ (DontCheckAlignment || Align >= Int64Align));
// FIXME: In cases where we can, we should use the original struct.
// Consider cases like { int, int } and {int, short} for example! This will
@@ -621,29 +622,36 @@
unsigned ElementSize = UseInt64 ? 8:4;
unsigned ArraySize = Size / ElementSize;
+ // Put as much of the aggregate as possible into an array.
const Type *ATy = NULL;
const Type *ArrayElementType = NULL;
if (ArraySize) {
Size = Size % ElementSize;
- ArrayElementType = (UseInt64) ?
- Type::getInt64Ty(getGlobalContext()) : Type::getInt32Ty(getGlobalContext());
+ ArrayElementType = (UseInt64 ?
+ Type::getInt64Ty(getGlobalContext()) :
+ Type::getInt32Ty(getGlobalContext()));
ATy = ArrayType::get(ArrayElementType, ArraySize);
Elts.push_back(ATy);
}
- if (Size >= 4) {
- Elts.push_back(Type::getInt32Ty(getGlobalContext()));
- Size -= 4;
- }
- if (Size >= 2) {
- Elts.push_back(Type::getInt16Ty(getGlobalContext()));
- Size -= 2;
- }
- if (Size >= 1) {
- Elts.push_back(Type::getInt8Ty(getGlobalContext()));
- Size -= 1;
+ // Pass any leftover bytes as a separate element following the array.
+ unsigned LastEltRealSize = 0;
+ const llvm::Type *LastEltTy = 0;
+ if (Size > 4) {
+ LastEltTy = Type::getInt64Ty(getGlobalContext());
+ } else if (Size > 2) {
+ LastEltTy = Type::getInt32Ty(getGlobalContext());
+ } else if (Size > 1) {
+ LastEltTy = Type::getInt16Ty(getGlobalContext());
+ } else if (Size > 0) {
+ LastEltTy = Type::getInt8Ty(getGlobalContext());
+ }
+ if (LastEltTy) {
+ Elts.push_back(LastEltTy);
+ if (Size != getTargetData().getTypeAllocSize(LastEltTy))
+ LastEltRealSize = Size;
}
- assert(Size == 0 && "Didn't cover value?");
+
const StructType *STy = StructType::get(getGlobalContext(), Elts, false);
unsigned i = 0;
@@ -658,10 +666,10 @@
C.ExitField();
++i;
}
- for (unsigned e = Elts.size(); i != e; ++i) {
+ if (LastEltTy) {
C.EnterField(i, STy);
- C.HandleScalarArgument(Elts[i], 0);
- ScalarElts.push_back(Elts[i]);
+ C.HandleScalarArgument(LastEltTy, 0, LastEltRealSize);
+ ScalarElts.push_back(LastEltTy);
C.ExitField();
}
}
More information about the llvm-commits
mailing list