In r168821. Thanks for reviewing, Manman
On Nov 28, 2012, at 2:17 PM, Eli Friedman wrote: > 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
