PNaCl: ExpandVarArgs: Use memcpy() instead of struct load+store for struct args

Although PNaCl doesn't fully support struct types as varargs
arguments, there is a test in Spec2k (255.vortex) that passes a struct
as a varargs argument but never reads the argument using va_arg (which
is legal, but strange).

ExpandVarArgs was handling the struct argument by copying it with a
struct load+store.  This is undesirable because currently SROA
converts that into code that uses extractvalue, which was rejected by
the PNaCl ABI checker.  Struct load/store will soon be rejected by the
ABI checker too.

We could fix this by running ExpandStructRegs after ExpandVarArgs, but
it's cleaner for ExpandVarArgs to use memcpy() instead of struct
load+store.  memcpy() is potentially more efficient because it avoids
having a temporary copy of the struct, and using memcpy() avoids
dependencies between IR passes.

BUG=https://code.google.com/p/nativeclient/issues/detail?id=3338
BUG=https://code.google.com/p/nativeclient/issues/detail?id=3343
TEST=*.ll tests + built 255.vortex from Spec2k

Review URL: https://codereview.chromium.org/16232021

2 files changed, 19 insertions, 9 deletions

diff --git a/lib/Transforms/NaCl/ExpandVarArgs.cpp b/lib/Transforms/NaCl/ExpandVarArgs.cpp
index 36f58416e2..0fd1a3cb44 100644
--- a/lib/Transforms/NaCl/ExpandVarArgs.cpp
+++ b/lib/Transforms/NaCl/ExpandVarArgs.cpp
@@ -38,6 +38,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Function.h"
+#include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Module.h"
@@ -196,14 +197,13 @@ static bool ExpandVarArgCall(InstType *Call, DataLayout *DL) {
   for (unsigned I = FuncType->getNumParams();
        I < Call->getNumArgOperands(); ++I) {
     Value *ArgVal = Call->getArgOperand(I);
+    VarArgs.push_back(ArgVal);
     if (Call->getAttributes().hasAttribute(I + 1, Attribute::ByVal)) {
-      // For "byval" arguments we must dereference the pointer and
-      // make a copy of the struct being passed by value.
-      ArgVal = CopyDebug(new LoadInst(ArgVal, "vararg_struct_copy", Call),
-                         Call);
+      // For "byval" arguments we must dereference the pointer.
+      VarArgsTypes.push_back(ArgVal->getType()->getPointerElementType());
+    } else {
+      VarArgsTypes.push_back(ArgVal->getType());
     }
-    VarArgs.push_back(ArgVal);
-    VarArgsTypes.push_back(ArgVal->getType());
   }
   if (VarArgsTypes.size() == 0) {
     // Some buggy code (e.g. 176.gcc in Spec2k) uses va_arg on an
@@ -249,7 +249,16 @@ static bool ExpandVarArgCall(InstType *Call, DataLayout *DL) {
     Indexes.push_back(ConstantInt::get(*Context, APInt(32, Index)));
     Value *Ptr = CopyDebug(GetElementPtrInst::Create(
                                Buf, Indexes, "vararg_ptr", Call), Call);
-    CopyDebug(new StoreInst(*Iter, Ptr, Call), Call);
+    if (Call->getAttributes().hasAttribute(
+            FuncType->getNumParams() + Index + 1, Attribute::ByVal)) {
+      IRBuilder<> Builder(Call);
+      Builder.CreateMemCpy(
+          Ptr, *Iter,
+          DL->getTypeAllocSize((*Iter)->getType()->getPointerElementType()),
+          /* Align= */ 1);
+    } else {
+      CopyDebug(new StoreInst(*Iter, Ptr, Call), Call);
+    }
   }
 
   // Cast function to new type to add our extra pointer argument.
diff --git a/test/Transforms/NaCl/expand-varargs-struct.ll b/test/Transforms/NaCl/expand-varargs-struct.ll
index 7bb310fd40..b96b41875c 100644
--- a/test/Transforms/NaCl/expand-varargs-struct.ll
+++ b/test/Transforms/NaCl/expand-varargs-struct.ll
@@ -11,6 +11,7 @@ define i32 @varargs_call_struct(%MyStruct* %ptr) {
   ret i32 %result
 }
 ; CHECK: define i32 @varargs_call_struct(%MyStruct* %ptr) {
-; CHECK: %vararg_struct_copy = load %MyStruct* %ptr
 ; CHECK: %vararg_ptr1 = getelementptr <{ i64, %MyStruct }>* %vararg_buffer, i32 0, i32 1
-; CHECK: store %MyStruct %vararg_struct_copy, %MyStruct* %vararg_ptr1
+; CHECK: %1 = bitcast %MyStruct* %vararg_ptr1 to i8*
+; CHECK: %2 = bitcast %MyStruct* %ptr to i8*
+; CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* %1, i8* %2, i64 16, i32 1, i1 false)