On Wed, Nov 28, 2012 at 2:08 PM, Manman Ren <[email protected]> wrote: > Author: mren > Date: Wed Nov 28 16:08:52 2012 > New Revision: 168820 > > URL: http://llvm.org/viewvc/llvm-project?rev=168820&view=rev > Log: > ABI: modify CreateCoercedLoad and CreateCoercedStore to not use load or store > of > the original parameter or return type. > > Since we do not accurately represent the data fields of a union, we should not > directly load or store a union type. > > As an exmple, if we have i8,i8, i32, i32 as one field type and i32,i32 as > another field type, the first field type will be chosen to represent the > union. > If we load with the union's type, the 3rd byte and the 4th byte will be > skipped. > > rdar://12723368 > > Modified: > cfe/trunk/lib/CodeGen/CGCall.cpp > cfe/trunk/test/CodeGen/x86_64-arguments.c > > Modified: cfe/trunk/lib/CodeGen/CGCall.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=168820&r1=168819&r2=168820&view=diff > ============================================================================== > --- cfe/trunk/lib/CodeGen/CGCall.cpp (original) > +++ cfe/trunk/lib/CodeGen/CGCall.cpp Wed Nov 28 16:08:52 2012 > @@ -692,12 +692,12 @@ > // Otherwise do coercion through memory. This is stupid, but > // simple. > llvm::Value *Tmp = CGF.CreateTempAlloca(Ty); > - llvm::Value *Casted = > - CGF.Builder.CreateBitCast(Tmp, llvm::PointerType::getUnqual(SrcTy)); > - llvm::StoreInst *Store = > - CGF.Builder.CreateStore(CGF.Builder.CreateLoad(SrcPtr), Casted); > - // FIXME: Use better alignment / avoid requiring aligned store. > - Store->setAlignment(1); > + llvm::Type *I8PtrTy = CGF.Builder.getInt8PtrTy(); > + llvm::Value *Casted = CGF.Builder.CreateBitCast(Tmp, I8PtrTy); > + llvm::Value *SrcCasted = CGF.Builder.CreateBitCast(SrcPtr, I8PtrTy); > + CGF.Builder.CreateMemCpy(Casted, SrcCasted, > + llvm::ConstantInt::get(CGF.IntPtrTy, SrcSize), > + 1, false); > return CGF.Builder.CreateLoad(Tmp); > } > > @@ -779,12 +779,12 @@ > // to that information. > llvm::Value *Tmp = CGF.CreateTempAlloca(SrcTy); > CGF.Builder.CreateStore(Src, Tmp); > - llvm::Value *Casted = > - CGF.Builder.CreateBitCast(Tmp, llvm::PointerType::getUnqual(DstTy)); > - llvm::LoadInst *Load = CGF.Builder.CreateLoad(Casted); > - // FIXME: Use better alignment / avoid requiring aligned load. > - Load->setAlignment(1); > - CGF.Builder.CreateStore(Load, DstPtr, DstIsVolatile); > + llvm::Type *I8PtrTy = CGF.Builder.getInt8PtrTy(); > + llvm::Value *Casted = CGF.Builder.CreateBitCast(Tmp, I8PtrTy); > + llvm::Value *DstCasted = CGF.Builder.CreateBitCast(DstPtr, I8PtrTy); > + CGF.Builder.CreateMemCpy(DstCasted, Casted, > + llvm::ConstantInt::get(CGF.IntPtrTy, DstSize), > + 1, false); > } > }
The FIXME is still relevant, as far as I can tell: we should try to improve the alignment of the memcpy where possible. > Modified: cfe/trunk/test/CodeGen/x86_64-arguments.c > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/x86_64-arguments.c?rev=168820&r1=168819&r2=168820&view=diff > ============================================================================== > --- cfe/trunk/test/CodeGen/x86_64-arguments.c (original) > +++ cfe/trunk/test/CodeGen/x86_64-arguments.c Wed Nov 28 16:08:52 2012 > @@ -354,3 +354,23 @@ > struct s47 { unsigned a; }; > void f47(int,int,int,int,int,int,struct s47); > void test47(int a, struct s47 b) { f47(a, a, a, a, a, a, b); } > + > +// rdar://12723368 > +// In the following example, there are holes in T4 at the 3rd byte and the > 4th > +// byte, however, T2 does not have those holes. T4 is chosen to be the > +// representing type for union T1, but we can't use load or store of T4 since > +// it will skip the 3rd byte and the 4th byte. > +// In general, Since we don't accurately represent the data fields of a > union, > +// do not use load or store of the representing llvm type for the union. > +typedef _Complex int T2; > +typedef _Complex char T5; > +typedef _Complex int T7; > +typedef struct T4 { T5 field0; T7 field1; } T4; > +typedef union T1 { T2 field0; T4 field1; } T1; > +extern T1 T1_retval; > +T1 test48(void) { > +// CHECK: @test48 > +// CHECK-NOT: load %struct.T4* %{{.*}} > +// CHECK-NOT: store %struct.T4 %{{.*}}, %struct.T4* %{{.*}} > + return T1_retval; > +} Please write CHECK lines that checks for the memcpys which we do expect to generate; CHECK-NOTs tend to be fragile. -Eli _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
