PNaCl: Add ExpandSmallArguments pass to widen parameters to 32 bits

This widens i1, i8 and i16 function arguments and return types.

Factor out RecreateFunction() helper function from existing PNaCl
passes since this is a reoccurring code fragment.

BUG=https://code.google.com/p/nativeclient/issues/detail?id=3342
TEST=*.ll tests + PNaCl toolchain trybots + GCC torture tests + LLVM test suite

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

10 files changed, 352 insertions, 24 deletions

diff --git a/include/llvm/InitializePasses.h b/include/llvm/InitializePasses.h
index 968503d28e..53357cca0a 100644
--- a/include/llvm/InitializePasses.h
+++ b/include/llvm/InitializePasses.h
@@ -283,6 +283,7 @@ void initializeExpandByValPass(PassRegistry&);
 void initializeExpandConstantExprPass(PassRegistry&);
 void initializeExpandCtorsPass(PassRegistry&);
 void initializeExpandGetElementPtrPass(PassRegistry&);
+void initializeExpandSmallArgumentsPass(PassRegistry&);
 void initializeExpandTlsConstantExprPass(PassRegistry&);
 void initializeExpandTlsPass(PassRegistry&);
 void initializeExpandVarArgsPass(PassRegistry&);
diff --git a/include/llvm/Transforms/NaCl.h b/include/llvm/Transforms/NaCl.h
index 92838e5fc7..cfd4c6e51d 100644
--- a/include/llvm/Transforms/NaCl.h
+++ b/include/llvm/Transforms/NaCl.h
@@ -13,7 +13,9 @@
 namespace llvm {
 
 class BasicBlockPass;
+class Function;
 class FunctionPass;
+class FunctionType;
 class Instruction;
 class ModulePass;
 class PassManager;
@@ -26,6 +28,7 @@ ModulePass *createExpandByValPass();
 FunctionPass *createExpandConstantExprPass();
 ModulePass *createExpandCtorsPass();
 BasicBlockPass *createExpandGetElementPtrPass();
+ModulePass *createExpandSmallArgumentsPass();
 ModulePass *createExpandTlsPass();
 ModulePass *createExpandTlsConstantExprPass();
 ModulePass *createExpandVarArgsPass();
@@ -48,6 +51,13 @@ void PhiSafeReplaceUses(Use *U, Value *NewVal);
 // Copy debug information from Original to NewInst, and return NewInst.
 Instruction *CopyDebug(Instruction *NewInst, Instruction *Original);
 
+// In order to change a function's type, the function must be
+// recreated.  RecreateFunction() recreates Func with type NewType.
+// It copies or moves across everything except the argument values,
+// which the caller must update because the argument types might be
+// different.
+Function *RecreateFunction(Function *Func, FunctionType *NewType);
+
 }
 
 #endif
diff --git a/lib/Transforms/NaCl/CMakeLists.txt b/lib/Transforms/NaCl/CMakeLists.txt
index ab6b5fba8a..027bb46a14 100644
--- a/lib/Transforms/NaCl/CMakeLists.txt
+++ b/lib/Transforms/NaCl/CMakeLists.txt
@@ -5,6 +5,7 @@ add_llvm_library(LLVMNaClTransforms
   ExpandConstantExpr.cpp
   ExpandCtors.cpp
   ExpandGetElementPtr.cpp
+  ExpandSmallArguments.cpp
   ExpandTls.cpp
   ExpandTlsConstantExpr.cpp
   ExpandUtils.cpp
diff --git a/lib/Transforms/NaCl/ExpandSmallArguments.cpp b/lib/Transforms/NaCl/ExpandSmallArguments.cpp
new file mode 100644
index 0000000000..c8a321edb9
--- /dev/null
+++ b/lib/Transforms/NaCl/ExpandSmallArguments.cpp
@@ -0,0 +1,217 @@
+//===- ExpandSmallArguments.cpp - Expand out arguments smaller than i32----===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// LLVM IR allows function return types and argument types such as
+// "zeroext i8" and "signext i8".  The Language Reference says that
+// zeroext "indicates to the code generator that the parameter or
+// return value should be zero-extended to the extent required by the
+// target's ABI (which is usually 32-bits, but is 8-bits for a i1 on
+// x86-64) by the caller (for a parameter) or the callee (for a return
+// value)".
+//
+// This can lead to non-portable behaviour when calling functions
+// without C prototypes or with wrong C prototypes.
+//
+// In order to remove this non-portability from PNaCl, and to simplify
+// the language that the PNaCl translator accepts, the
+// ExpandSmallArguments pass widens integer arguments and return types
+// to be at least 32 bits.  The pass inserts explicit cast
+// instructions (ZExtInst/SExtInst/TruncInst) as needed.
+//
+// The pass chooses between ZExtInst and SExtInst widening based on
+// whether a "signext" attribute is present.  However, in principle
+// the pass could always use zero-extension, because the extent to
+// which either zero-extension or sign-extension is done is up to the
+// target ABI, which is up to PNaCl to specify.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/IR/Function.h"
+#include "llvm/IR/Instructions.h"
+#include "llvm/IR/IntrinsicInst.h"
+#include "llvm/IR/Module.h"
+#include "llvm/Pass.h"
+#include "llvm/Transforms/NaCl.h"
+
+using namespace llvm;
+
+namespace {
+  // This is a ModulePass because the pass recreates functions in
+  // order to change their arguments' types.
+  class ExpandSmallArguments : public ModulePass {
+  public:
+    static char ID; // Pass identification, replacement for typeid
+    ExpandSmallArguments() : ModulePass(ID) {
+      initializeExpandSmallArgumentsPass(*PassRegistry::getPassRegistry());
+    }
+
+    virtual bool runOnModule(Module &M);
+  };
+}
+
+char ExpandSmallArguments::ID = 0;
+INITIALIZE_PASS(ExpandSmallArguments, "expand-small-arguments",
+                "Expand function arguments to be at least 32 bits in size",
+                false, false)
+
+// Returns the normalized version of the given argument/return type.
+static Type *NormalizeType(Type *Ty) {
+  if (IntegerType *IntTy = dyn_cast<IntegerType>(Ty)) {
+    if (IntTy->getBitWidth() < 32) {
+      return IntegerType::get(Ty->getContext(), 32);
+    }
+  }
+  return Ty;
+}
+
+// Returns the normalized version of the given function type.
+static FunctionType *NormalizeFunctionType(FunctionType *FTy) {
+  if (FTy->isVarArg()) {
+    report_fatal_error(
+        "ExpandSmallArguments does not handle varargs functions");
+  }
+  SmallVector<Type *, 8> ArgTypes;
+  for (unsigned I = 0; I < FTy->getNumParams(); ++I) {
+    ArgTypes.push_back(NormalizeType(FTy->getParamType(I)));
+  }
+  return FunctionType::get(NormalizeType(FTy->getReturnType()),
+                           ArgTypes, false);
+}
+
+// Convert the given function to use normalized argument/return types.
+static bool ConvertFunction(Function *Func) {
+  FunctionType *FTy = Func->getFunctionType();
+  FunctionType *NFTy = NormalizeFunctionType(FTy);
+  if (NFTy == FTy)
+    return false; // No change needed.
+  Function *NewFunc = RecreateFunction(Func, NFTy);
+
+  // Move the arguments across to the new function.
+  for (Function::arg_iterator Arg = Func->arg_begin(), E = Func->arg_end(),
+         NewArg = NewFunc->arg_begin();
+       Arg != E; ++Arg, ++NewArg) {
+    NewArg->takeName(Arg);
+    if (Arg->getType() == NewArg->getType()) {
+      Arg->replaceAllUsesWith(NewArg);
+    } else {
+      Instruction *Trunc = new TruncInst(
+          NewArg, Arg->getType(), NewArg->getName() + ".arg_trunc",
+          NewFunc->getEntryBlock().getFirstInsertionPt());
+      Arg->replaceAllUsesWith(Trunc);
+    }
+  }
+
+  if (FTy->getReturnType() != NFTy->getReturnType()) {
+    // Fix up return instructions.
+    Instruction::CastOps CastType =
+        Func->getAttributes().hasAttribute(0, Attribute::SExt) ?
+        Instruction::SExt : Instruction::ZExt;
+    for (Function::iterator BB = NewFunc->begin(), E = NewFunc->end();
+         BB != E;
+         ++BB) {
+      for (BasicBlock::iterator Iter = BB->begin(), E = BB->end();
+           Iter != E; ) {
+        Instruction *Inst = Iter++;
+        if (ReturnInst *Ret = dyn_cast<ReturnInst>(Inst)) {
+          Value *Ext = CopyDebug(
+              CastInst::Create(CastType, Ret->getReturnValue(),
+                               NFTy->getReturnType(),
+                               Ret->getReturnValue()->getName() + ".ret_ext",
+                               Ret),
+              Ret);
+          CopyDebug(ReturnInst::Create(Ret->getContext(), Ext, Ret), Ret);
+          Ret->eraseFromParent();
+        }
+      }
+    }
+  }
+
+  Func->eraseFromParent();
+  return true;
+}
+
+// Convert the given call to use normalized argument/return types.
+static bool ConvertCall(CallInst *Call) {
+  // Don't try to change calls to intrinsics.
+  if (isa<IntrinsicInst>(Call))
+    return false;
+  FunctionType *FTy = cast<FunctionType>(
+      Call->getCalledValue()->getType()->getPointerElementType());
+  FunctionType *NFTy = NormalizeFunctionType(FTy);
+  if (NFTy == FTy)
+    return false; // No change needed.
+
+  // Convert arguments.
+  SmallVector<Value *, 8> Args;
+  for (unsigned I = 0; I < Call->getNumArgOperands(); ++I) {
+    Value *Arg = Call->getArgOperand(I);
+    if (NFTy->getParamType(I) != FTy->getParamType(I)) {
+      Instruction::CastOps CastType =
+          Call->getAttributes().hasAttribute(I + 1, Attribute::SExt) ?
+          Instruction::SExt : Instruction::ZExt;
+      Arg = CopyDebug(CastInst::Create(CastType, Arg, NFTy->getParamType(I),
+                                       "arg_ext", Call), Call);
+    }
+    Args.push_back(Arg);
+  }
+  Value *CastFunc =
+    CopyDebug(new BitCastInst(Call->getCalledValue(), NFTy->getPointerTo(),
+                              Call->getName() + ".arg_cast", Call), Call);
+  CallInst *NewCall = CallInst::Create(CastFunc, Args, "", Call);
+  CopyDebug(NewCall, Call);
+  NewCall->takeName(Call);
+  NewCall->setAttributes(Call->getAttributes());
+  NewCall->setCallingConv(Call->getCallingConv());
+  NewCall->setTailCall(Call->isTailCall());
+  Value *Result = NewCall;
+  if (FTy->getReturnType() != NFTy->getReturnType()) {
+    Result = CopyDebug(new TruncInst(NewCall, FTy->getReturnType(),
+                                     NewCall->getName() + ".ret_trunc",
+                                     Call), Call);
+  }
+  Call->replaceAllUsesWith(Result);
+  Call->eraseFromParent();
+  return true;
+}
+
+bool ExpandSmallArguments::runOnModule(Module &M) {
+  bool Changed = false;
+  for (Module::iterator Iter = M.begin(), E = M.end(); Iter != E; ) {
+    Function *Func = Iter++;
+    // Don't try to change intrinsic declarations because intrinsics
+    // will continue to have non-normalized argument types.  For
+    // example, memset() takes an i8 argument.  It shouldn't matter
+    // whether we modify the types of other function declarations, but
+    // we don't expect to see non-intrinsic function declarations in a
+    // PNaCl pexe.
+    if (Func->empty())
+      continue;
+
+    for (Function::iterator BB = Func->begin(), E = Func->end();
+         BB != E; ++BB) {
+      for (BasicBlock::iterator Iter = BB->begin(), E = BB->end();
+           Iter != E; ) {
+        Instruction *Inst = Iter++;
+        if (CallInst *Call = dyn_cast<CallInst>(Inst)) {
+          Changed |= ConvertCall(Call);
+        } else if (isa<InvokeInst>(Inst)) {
+          report_fatal_error(
+              "ExpandSmallArguments does not handle invoke instructions");
+        }
+      }
+    }
+
+    Changed |= ConvertFunction(Func);
+  }
+  return Changed;
+}
+
+ModulePass *llvm::createExpandSmallArgumentsPass() {
+  return new ExpandSmallArguments();
+}
diff --git a/lib/Transforms/NaCl/ExpandUtils.cpp b/lib/Transforms/NaCl/ExpandUtils.cpp
index 507760603f..913e0d6607 100644
--- a/lib/Transforms/NaCl/ExpandUtils.cpp
+++ b/lib/Transforms/NaCl/ExpandUtils.cpp
@@ -8,7 +8,9 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/IR/BasicBlock.h"
+#include "llvm/IR/Function.h"
 #include "llvm/IR/Instructions.h"
+#include "llvm/IR/Module.h"
 #include "llvm/Transforms/NaCl.h"
 
 using namespace llvm;
@@ -43,3 +45,16 @@ Instruction *llvm::CopyDebug(Instruction *NewInst, Instruction *Original) {
   NewInst->setDebugLoc(Original->getDebugLoc());
   return NewInst;
 }
+
+Function *llvm::RecreateFunction(Function *Func, FunctionType *NewType) {
+  Function *NewFunc = Function::Create(NewType, Func->getLinkage());
+  NewFunc->copyAttributesFrom(Func);
+  Func->getParent()->getFunctionList().insert(Func, NewFunc);
+  NewFunc->takeName(Func);
+  NewFunc->getBasicBlockList().splice(NewFunc->begin(),
+                                      Func->getBasicBlockList());
+  Func->replaceAllUsesWith(
+      ConstantExpr::getBitCast(NewFunc,
+                               Func->getFunctionType()->getPointerTo()));
+  return NewFunc;
+}
diff --git a/lib/Transforms/NaCl/ExpandVarArgs.cpp b/lib/Transforms/NaCl/ExpandVarArgs.cpp
index 3ea093f8f8..36f58416e2 100644
--- a/lib/Transforms/NaCl/ExpandVarArgs.cpp
+++ b/lib/Transforms/NaCl/ExpandVarArgs.cpp
@@ -72,15 +72,7 @@ static void ExpandVarArgFunc(Function *Func) {
   SmallVector<Type *, 8> Params(FTy->param_begin(), FTy->param_end());
   Params.push_back(PtrType);
   FunctionType *NFTy = FunctionType::get(FTy->getReturnType(), Params, false);
-
-  // In order to change the function's arguments, we have to recreate
-  // the function.
-  Function *NewFunc = Function::Create(NFTy, Func->getLinkage());
-  NewFunc->copyAttributesFrom(Func);
-  Func->getParent()->getFunctionList().insert(Func, NewFunc);
-  NewFunc->takeName(Func);
-  NewFunc->getBasicBlockList().splice(NewFunc->begin(),
-                                      Func->getBasicBlockList());
+  Function *NewFunc = RecreateFunction(Func, NFTy);
 
   // Declare the new argument as "noalias".
   NewFunc->setAttributes(
@@ -95,8 +87,6 @@ static void ExpandVarArgFunc(Function *Func) {
     NewArg->takeName(Arg);
   }
 
-  Func->replaceAllUsesWith(
-      ConstantExpr::getBitCast(NewFunc, FTy->getPointerTo()));
   Func->eraseFromParent();
 
   Value *VarArgsArg = --NewFunc->arg_end();
diff --git a/lib/Transforms/NaCl/PNaClABISimplify.cpp b/lib/Transforms/NaCl/PNaClABISimplify.cpp
index 47e5fb67e6..3c59546275 100644
--- a/lib/Transforms/NaCl/PNaClABISimplify.cpp
+++ b/lib/Transforms/NaCl/PNaClABISimplify.cpp
@@ -49,6 +49,12 @@ void llvm::PNaClABISimplifyAddPostOptPasses(PassManager &PM) {
   // ExpandByVal expands it to.
   PM.add(createExpandByValPass());
 
+  // We place ExpandSmallArguments after optimization passes because
+  // some optimizations undo its changes.  Note that
+  // ExpandSmallArguments requires that ExpandVarArgs has already been
+  // run.
+  PM.add(createExpandSmallArgumentsPass());
+
   // We place StripMetadata after optimization passes because
   // optimizations depend on the metadata.
   PM.add(createStripMetadataPass());
diff --git a/lib/Transforms/NaCl/ReplacePtrsWithInts.cpp b/lib/Transforms/NaCl/ReplacePtrsWithInts.cpp
index 619ae04826..00d374dc7b 100644
--- a/lib/Transforms/NaCl/ReplacePtrsWithInts.cpp
+++ b/lib/Transforms/NaCl/ReplacePtrsWithInts.cpp
@@ -552,17 +552,9 @@ bool ReplacePtrsWithInts::runOnModule(Module &M) {
 
     FunctionConverter FC(IntPtrType);
     FunctionType *NFTy = FC.convertFuncType(OldFunc->getFunctionType());
-
-    // In order to change the function's argument types, we have to
-    // recreate the function.
-    Function *NewFunc = Function::Create(NFTy, OldFunc->getLinkage());
-    NewFunc->copyAttributesFrom(OldFunc);
-    NewFunc->setAttributes(RemovePointerAttrs(M.getContext(),
-                                              NewFunc->getAttributes()));
-    M.getFunctionList().insert(OldFunc, NewFunc);
-    NewFunc->takeName(OldFunc);
-    NewFunc->getBasicBlockList().splice(NewFunc->begin(),
-                                        OldFunc->getBasicBlockList());
+    OldFunc->setAttributes(RemovePointerAttrs(M.getContext(),
+                                              OldFunc->getAttributes()));
+    Function *NewFunc = RecreateFunction(OldFunc, NFTy);
 
     // Move the arguments across to the new function.
     for (Function::arg_iterator Arg = OldFunc->arg_begin(),
@@ -594,8 +586,6 @@ bool ReplacePtrsWithInts::runOnModule(Module &M) {
       }
     }
     FC.eraseReplacedInstructions();
-    OldFunc->replaceAllUsesWith(ConstantExpr::getBitCast(NewFunc,
-                                                         OldFunc->getType()));
     OldFunc->eraseFromParent();
   }
   // Now that all functions have their normalized types, we can remove
diff --git a/test/Transforms/NaCl/expand-small-arguments.ll b/test/Transforms/NaCl/expand-small-arguments.ll
new file mode 100644
index 0000000000..48a62d80d7
--- /dev/null
+++ b/test/Transforms/NaCl/expand-small-arguments.ll
@@ -0,0 +1,97 @@
+; RUN: opt %s -expand-small-arguments -S | FileCheck %s
+
+@var = global i8 0
+
+
+define void @small_arg(i8 %val) {
+  store i8 %val, i8* @var
+  ret void
+}
+; CHECK: define void @small_arg(i32 %val) {
+; CHECK-NEXT: %val.arg_trunc = trunc i32 %val to i8
+; CHECK-NEXT: store i8 %val.arg_trunc, i8* @var
+
+
+define i8 @small_result() {
+  %val = load i8* @var
+  ret i8 %val
+}
+; CHECK: define i32 @small_result() {
+; CHECK-NEXT: %val = load i8* @var
+; CHECK-NEXT: %val.ret_ext = zext i8 %val to i32
+; CHECK-NEXT: ret i32 %val.ret_ext
+
+define signext i8 @small_result_signext() {
+  %val = load i8* @var
+  ret i8 %val
+}
+; CHECK: define signext i32 @small_result_signext() {
+; CHECK-NEXT: %val = load i8* @var
+; CHECK-NEXT: %val.ret_ext = sext i8 %val to i32
+; CHECK-NEXT: ret i32 %val.ret_ext
+
+
+define void @call_small_arg() {
+  call void @small_arg(i8 100)
+  ret void
+}
+; CHECK: define void @call_small_arg() {
+; CHECK-NEXT: %arg_ext = zext i8 100 to i32
+; CHECK-NEXT: %.arg_cast = bitcast {{.*}} @small_arg
+; CHECK-NEXT: call void %.arg_cast(i32 %arg_ext)
+
+define void @call_small_arg_signext() {
+  call void @small_arg(i8 signext 100)
+  ret void
+}
+; CHECK: define void @call_small_arg_signext() {
+; CHECK-NEXT: %arg_ext = sext i8 100 to i32
+; CHECK-NEXT: %.arg_cast = bitcast {{.*}} @small_arg
+; CHECK-NEXT: call void %.arg_cast(i32 signext %arg_ext)
+
+
+define void @call_small_result() {
+  %r = call i8 @small_result()
+  store i8 %r, i8* @var
+  ret void
+}
+; CHECK: define void @call_small_result() {
+; CHECK-NEXT: %r.arg_cast = bitcast {{.*}} @small_result
+; CHECK-NEXT: %r = call i32 %r.arg_cast()
+; CHECK-NEXT: %r.ret_trunc = trunc i32 %r to i8
+; CHECK-NEXT: store i8 %r.ret_trunc, i8* @var
+
+
+; Check that various attributes are preserved.
+define i1 @attributes(i8 %arg) nounwind {
+  %r = tail call fastcc i1 @attributes(i8 %arg) nounwind
+  ret i1 %r
+}
+; CHECK: define i32 @attributes(i32 %arg) [[NOUNWIND:#[0-9]+]] {
+; CHECK: tail call fastcc i32 {{.*}} [[NOUNWIND]]
+
+
+; These arguments and results should be left alone.
+define i64 @larger_arguments(i32 %a, i64 %b, i8* %ptr, double %d) {
+  %r = call i64 @larger_arguments(i32 %a, i64 %b, i8* %ptr, double %d)
+  ret i64 %r
+}
+; CHECK: define i64 @larger_arguments(i32 %a, i64 %b, i8* %ptr, double %d) {
+; CHECK-NEXT: %r = call i64 @larger_arguments(i32 %a, i64 %b, i8* %ptr, double %d)
+; CHECK-NEXT: ret i64 %r
+
+
+; Intrinsics must be left alone since the pass cannot change their types.
+
+declare void @llvm.memset.p0i8.i32(i8* nocapture, i8, i32, i32, i1)
+; CHECK: declare void @llvm.memset.p0i8.i32(i8* nocapture, i8, i32, i32, i1)
+
+define void @intrinsic_call(i8* %ptr) {
+  call void @llvm.memset.p0i8.i32(i8* %ptr, i8 99, i32 256, i32 1, i1 0)
+  ret void
+}
+; CHECK: define void @intrinsic_call
+; CHECK-NEXT: call void @llvm.memset.p0i8.i32(i8* %ptr, i8 99,
+
+
+; CHECK: attributes [[NOUNWIND]] = { nounwind }
diff --git a/tools/opt/opt.cpp b/tools/opt/opt.cpp
index e44cfe06f4..bec40a97d9 100644
--- a/tools/opt/opt.cpp
+++ b/tools/opt/opt.cpp
@@ -604,6 +604,7 @@ int main(int argc, char **argv) {
   initializeExpandConstantExprPass(Registry);
   initializeExpandCtorsPass(Registry);
   initializeExpandGetElementPtrPass(Registry);
+  initializeExpandSmallArgumentsPass(Registry);
   initializeExpandTlsPass(Registry);
   initializeExpandTlsConstantExprPass(Registry);
   initializeExpandVarArgsPass(Registry);