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

[Eli Friedman]

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