From 4a2dcfe5eea2eac447bfd2b4620b69a19ccf0ff2 Mon Sep 17 00:00:00 2001 From: Bill Schmidt Date: Fri, 26 Oct 2012 19:59:03 +0000 Subject: [PATCH] This patch addresses a 64-bit PowerPC ELF ABI compatibility issue with varargs parameter passing. A strict reading of the ABI indicates that any argument with alignment greater than 8 may require skipping doublewords in the parameter save area to align the argument, and hence require skipping GPRs. In practice, this is not done by GCC. The alignment restriction is used for internal alignment of a structure, but a structure with 16-byte alignment, for example, is not itself 16-byte aligned in the parameter save area. Although this is messy, it has become the de facto standard used in building existing libraries. My initial varargs support followed the ABI language, but not the de facto standard. Running the GCC compatibility test suite exposed this issue, and indeed showed that LLVM didn't pass parameters self-consistently with my original logic. Removing the additional alignment logic allows the affected tests to now pass. I modified the ppc64-varargs-struct.c test case to remove the existing test for generation of alignment code, which is no longer appropriate. Built and tested on powerpc64-unknown-linux-gnu with no new regressions. llvm-svn: 166805 --- clang/lib/CodeGen/TargetInfo.cpp | 15 --------------- clang/test/CodeGen/ppc64-varargs-struct.c | 6 ------ 2 files changed, 21 deletions(-) diff --git a/clang/lib/CodeGen/TargetInfo.cpp b/clang/lib/CodeGen/TargetInfo.cpp index 0a6ebb897e98..58cbf2e20684 100644 --- a/clang/lib/CodeGen/TargetInfo.cpp +++ b/clang/lib/CodeGen/TargetInfo.cpp @@ -2755,21 +2755,6 @@ llvm::Value *PPC64_SVR4_ABIInfo::EmitVAArg(llvm::Value *VAListAddr, llvm::Value *VAListAddrAsBPP = Builder.CreateBitCast(VAListAddr, BPP, "ap"); llvm::Value *Addr = Builder.CreateLoad(VAListAddrAsBPP, "ap.cur"); - // Handle address alignment for type alignment > 64 bits. Although - // long double normally requires 16-byte alignment, this is not the - // case when it is passed as an argument; so handle that special case. - const BuiltinType *BT = Ty->getAs(); - unsigned TyAlign = CGF.getContext().getTypeAlign(Ty) / 8; - - if (TyAlign > 8 && (!BT || !BT->isFloatingPoint())) { - assert((TyAlign & (TyAlign - 1)) == 0 && - "Alignment is not power of 2!"); - llvm::Value *AddrAsInt = Builder.CreatePtrToInt(Addr, CGF.Int64Ty); - AddrAsInt = Builder.CreateAdd(AddrAsInt, Builder.getInt64(TyAlign - 1)); - AddrAsInt = Builder.CreateAnd(AddrAsInt, Builder.getInt64(~(TyAlign - 1))); - Addr = Builder.CreateIntToPtr(AddrAsInt, BP); - } - // Update the va_list pointer. unsigned SizeInBytes = CGF.getContext().getTypeSize(Ty) / 8; unsigned Offset = llvm::RoundUpToAlignment(SizeInBytes, 8); diff --git a/clang/test/CodeGen/ppc64-varargs-struct.c b/clang/test/CodeGen/ppc64-varargs-struct.c index 3c7e7cb6bc10..e69332ead434 100644 --- a/clang/test/CodeGen/ppc64-varargs-struct.c +++ b/clang/test/CodeGen/ppc64-varargs-struct.c @@ -18,12 +18,6 @@ void testva (int n, ...) // CHECK: bitcast %struct.x* %{{[0-9]+}} to i8* // CHECK: call void @llvm.memcpy - __int128_t u = va_arg (ap, __int128_t); -// CHECK: ptrtoint i8* %{{[a-z.0-9]*}} to i64 -// CHECK: add i64 %{{[0-9]+}}, 15 -// CHECK: and i64 %{{[0-9]+}}, 4294967280 -// CHECK: inttoptr i64 %{{[0-9]+}} to i8* - int v = va_arg (ap, int); // CHECK: ptrtoint i8* %{{[a-z.0-9]*}} to i64 // CHECK: add i64 %{{[0-9]+}}, 4