[cfe-commits] r168820 - in /cfe/trunk: lib/CodeGen/CGCall.cpp test/CodeGen/x86_64-arguments.c

[Manman Ren]

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