[cfe-commits] r166805 - in /cfe/trunk: lib/CodeGen/TargetInfo.cpp test/CodeGen/ppc64-varargs-struct.c

[Bill Schmidt]

On Fri, 2012-10-26 at 16:09 -0400, Rafael Espíndola wrote:
> you should probably add a CHECK-NOT or change a CHECK to CHECK-NEXT.
> The modified test would pass with the patch reverted, no?

That's reasonable.  I'll work something up.

> 
> 
> On 26 October 2012 15:59, Bill Schmidt <wschmidt at linux.vnet.ibm.com> wrote:
> > Author: wschmidt
> > Date: Fri Oct 26 14:59:03 2012
> > New Revision: 166805
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=166805&view=rev
> > Log:
> > 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.
> >
> > Modified:
> >     cfe/trunk/lib/CodeGen/TargetInfo.cpp
> >     cfe/trunk/test/CodeGen/ppc64-varargs-struct.c
> >
> > Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=166805&r1=166804&r2=166805&view=diff
> > ==============================================================================
> > --- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
> > +++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Fri Oct 26 14:59:03 2012
> > @@ -2755,21 +2755,6 @@
> >    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<BuiltinType>();
> > -  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);
> >
> > Modified: cfe/trunk/test/CodeGen/ppc64-varargs-struct.c
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ppc64-varargs-struct.c?rev=166805&r1=166804&r2=166805&view=diff
> > ==============================================================================
> > --- cfe/trunk/test/CodeGen/ppc64-varargs-struct.c (original)
> > +++ cfe/trunk/test/CodeGen/ppc64-varargs-struct.c Fri Oct 26 14:59:03 2012
> > @@ -18,12 +18,6 @@
> >  // 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
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>