[cfe-commits] r168820 - in /cfe/trunk: lib/CodeGen/CGCall.cpp test/CodeGen/x86_64-arguments.c
Manman Ren
mren at apple.com
Wed Nov 28 14:30:53 PST 2012
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 <mren at apple.com> 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
More information about the cfe-commits
mailing list