Thanks for taking a look!
Comments inline, new patch attached.

On Thu, Feb 9, 2012 at 9:34 AM, Eli Friedman <[email protected]> wrote:
> Indentation generally looks strange... maybe some stray tabs?

Fixed.

> +         llvm::Value *Casted =
> +           Builder.CreateBitCast(TempAlloca,
> llvm::PointerType::getUnqual(ConvertTypeForMem(Ty)));
> +         llvm::LoadInst *Load = Builder.CreateLoad(Casted);
> +         Builder.CreateStore(Load, Ptr);
>
> Please use memcpy to copy structs, not load+store.

Done.

> -      if (llvm::StructType *STy =
> -            dyn_cast<llvm::StructType>(ArgI.getCoerceToType())) {
> -        Ptr = Builder.CreateBitCast(Ptr, llvm::PointerType::getUnqual(STy));
> +      llvm::StructType *STy =
> dyn_cast<llvm::StructType>(ArgI.getCoerceToType());
> +      if (STy && STy->getNumElements() > 1) {
>
> For the case where STy->getNumElements() == 1, do we avoid storing a
> struct type?  If not, can you fix that?

Yes, as far as I can see. We go into CreateCoercedStore, then
EnterStructPointerForCoercedAccess.
Index: test/CodeGen/arm-arguments.c
===================================================================
--- test/CodeGen/arm-arguments.c	(revision 150158)
+++ test/CodeGen/arm-arguments.c	(working copy)
@@ -153,3 +153,15 @@
 // AAPCS: define arm_aapcscc void @f30({{.*}} noalias sret
 struct s30 { _Complex int f0; };
 struct s30 f30() {}
+
+// PR11905
+struct s31 { char x; };
+void f31(struct s31 s) { }
+// AAPCS: @f31([1 x i32] %s.coerce)
+// AAPCS: %s = alloca %struct.s31, align 4
+// AAPCS: %tmp = alloca [1 x i32]
+// AAPCS: store [1 x i32] %s.coerce, [1 x i32]* %tmp
+// APCS-GNU: @f31([1 x i32] %s.coerce)
+// APCS-GNU: %s = alloca %struct.s31, align 4
+// APCS-GNU: %tmp = alloca [1 x i32]
+// APCS-GNU: store [1 x i32] %s.coerce, [1 x i32]* %tmp
Index: lib/CodeGen/CGCall.cpp
===================================================================
--- lib/CodeGen/CGCall.cpp	(revision 150158)
+++ lib/CodeGen/CGCall.cpp	(working copy)
@@ -1013,7 +1013,7 @@
         break;
       }
 
-      llvm::AllocaInst *Alloca = CreateMemTemp(Ty, "coerce");
+      llvm::AllocaInst *Alloca = CreateMemTemp(Ty, Arg->getName());
 
       // The alignment we need to use is the max of the requested alignment for
       // the argument plus the alignment required by our access code below.
@@ -1037,15 +1037,33 @@
       // If the coerce-to type is a first class aggregate, we flatten it and
       // pass the elements. Either way is semantically identical, but fast-isel
       // and the optimizer generally likes scalar values better than FCAs.
-      if (llvm::StructType *STy =
-            dyn_cast<llvm::StructType>(ArgI.getCoerceToType())) {
-        Ptr = Builder.CreateBitCast(Ptr, llvm::PointerType::getUnqual(STy));
+      llvm::StructType *STy = dyn_cast<llvm::StructType>(ArgI.getCoerceToType());
+      if (STy && STy->getNumElements() > 1) {
+        uint64_t SrcSize = CGM.getTargetData().getTypeAllocSize(STy);
+        llvm::Type *DstTy =
+          cast<llvm::PointerType>(Ptr->getType())->getElementType();
+        uint64_t DstSize = CGM.getTargetData().getTypeAllocSize(DstTy);
 
-        for (unsigned i = 0, e = STy->getNumElements(); i != e; ++i) {
-          assert(AI != Fn->arg_end() && "Argument mismatch!");
-          AI->setName(Arg->getName() + ".coerce" + Twine(i));
-          llvm::Value *EltPtr = Builder.CreateConstGEP2_32(Ptr, 0, i);
-          Builder.CreateStore(AI++, EltPtr);
+        if (SrcSize <= DstSize) {
+          Ptr = Builder.CreateBitCast(Ptr, llvm::PointerType::getUnqual(STy));
+
+          for (unsigned i = 0, e = STy->getNumElements(); i != e; ++i) {
+            assert(AI != Fn->arg_end() && "Argument mismatch!");
+            AI->setName(Arg->getName() + ".coerce" + Twine(i));
+            llvm::Value *EltPtr = Builder.CreateConstGEP2_32(Ptr, 0, i);
+            Builder.CreateStore(AI++, EltPtr);
+          }
+        } else {
+          llvm::Value *TempAlloca = CreateTempAlloca(ArgI.getCoerceToType(), "coerce");
+
+          for (unsigned i = 0, e = STy->getNumElements(); i != e; ++i) {
+            assert(AI != Fn->arg_end() && "Argument mismatch!");
+            AI->setName(Arg->getName() + ".coerce" + Twine(i));
+            llvm::Value *EltPtr = Builder.CreateConstGEP2_32(TempAlloca, 0, i);
+            Builder.CreateStore(AI++, EltPtr);
+          }
+
+          Builder.CreateMemCpy(Ptr, TempAlloca, DstSize, AlignmentToUse);
         }
       } else {
         // Simple case, just do a coerced store of the argument into the alloca.
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to