Lower signatures exposing struct registers to byval struct pointers

lib/Transforms/NaCl/SimplifyStructRegSignatures.cpp

Index: lib/Transforms/NaCl/SimplifyStructRegSignatures.cpp
diff --git a/lib/Transforms/NaCl/SimplifyStructRegSignatures.cpp b/lib/Transforms/NaCl/SimplifyStructRegSignatures.cpp
new file mode 100644
index 0000000000000000000000000000000000000000..3e2c1163da780e7585da214496564548fdb579dd
--- /dev/null
+++ b/lib/Transforms/NaCl/SimplifyStructRegSignatures.cpp
@@ -0,0 +1,598 @@
+//===- SimplifyStructRegSignatures.cpp - struct regs to struct pointers----===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This pass replaces function signatures exposing struct registers
+// to byval pointer-based signatures.
+//
+// There are 2 types of signatures that are thus changed:
+//
+// @foo(%some_struct %val) -> @foo(%some_struct* byval %val)
+//      and
+// %someStruct @bar(<other_args>) -> void @bar(%someStruct* sret, <other_args>)
+//
+// Such function types may appear in other type declarations, for example:
+//
+// %a_struct = type { void (%some_struct)*, i32 }
+//
+// We map such types to corresponding types, mapping the function types
+// appropriately:
+//
+// %a_struct.0 = type { void (%some_struct*)*, i32 }
+//===----------------------------------------------------------------------===//
+
+#include <cassert>
+#include <cstddef>
+#include <llvm/ADT/SmallString.h>
+
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/ilist.h"
+#include "llvm/ADT/SetVector.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/Twine.h"
+#include "llvm/IR/Argument.h"
+#include "llvm/IR/Attributes.h"
+#include "llvm/IR/BasicBlock.h"
+#include "llvm/IR/DerivedTypes.h"
+#include "llvm/IR/Function.h"
+#include "llvm/IR/GlobalValue.h"
+#include "llvm/IR/Instructions.h"
+#include "llvm/IR/Module.h"
+#include "llvm/IR/Type.h"
+#include "llvm/IR/Use.h"
+#include "llvm/IR/User.h"
+#include "llvm/IR/Value.h"
+#include "llvm/Pass.h"
+#include "llvm/PassInfo.h"
+#include "llvm/PassRegistry.h"
+#include "llvm/PassSupport.h"
+#include "llvm/Transforms/NaCl.h"
+#include "llvm/Support/Debug.h"
+
+using namespace llvm;
+
+namespace {
+class MappingResult {
+public:
+  MappingResult(Type *ATy, bool Chg) {
+    Ty = ATy;
+    Changed = Chg;
+  }
+
+  bool isChanged() { return Changed; }
+
+  Type *operator->() { return Ty; }
+
+  operator Type *() { return Ty; }
+
+private:
+  Type *Ty;
+  bool Changed;
+};
+
+// Utility class. For any given type, get the associated type that is free of
+// struct register arguments.
+class TypeMapper {
+public:
+  Type *getSimpleType(Type *Ty);
+
+private:
+  DenseMap<Type *, Type *> MappedTypes;
+  MappingResult getSimpleArgumentType(Type *Ty);
+  MappingResult getSimpleAggregateTypeInternal(Type *Ty);
+};
+
+// This is a ModulePass because the pass recreates functions in
+// order to change their signatures.
+class SimplifyStructRegSignatures : public ModulePass {
+public:
+  static char ID;
+
+  SimplifyStructRegSignatures() : ModulePass(ID) {
+    initializeSimplifyStructRegSignaturesPass(*PassRegistry::getPassRegistry());
+  }
+  virtual bool runOnModule(Module &M);
+
+private:
+  unsigned PreferredAlignment = 0;
+  TypeMapper Mapper;
+  DenseSet<Function *> FunctionsToDelete;
+  SetVector<CallInst *> CallsToPatch;
+  SetVector<InvokeInst *> InvokesToPatch;
+  DenseMap<Function *, Function *> FunctionMap;
+  bool simplifyFunction(Function *OldFunc, Module &M);
+  void scheduleCallsForCleanup(Function *NewFunc);
+  template <class TCall> void fixCallSite(TCall *Call);
+  void fixFunctionBody(Function *OldFunc, Function *NewFunc);
+};
+}
+
+static const unsigned int TypicalFuncArity = 8;
+static const unsigned int TypicalStructArity = 8;
+
+char SimplifyStructRegSignatures::ID = 0;
+
+INITIALIZE_PASS(
+    SimplifyStructRegSignatures, "simplify-struct-reg-signatures",
+    "Simplify function signatures by removing struct register parameters",
+    false, false)
+
+// The type is "simple" if it does not recursively reference a
+// function type with at least an operand (arg or return) typed as struct
+// register
+Type *TypeMapper::getSimpleType(Type *Ty) {
+  return getSimpleAggregateTypeInternal(Ty);
+}
+
+// Transforms any type that could transitively reference a function pointer
+// into a simplified type.
+// We enter this function trying to determine the mapping of a type. Because
+// of how structs are handled (not interned - see further comments below)
+// we may be working with temporary types - types referencing "tentative"
+// structs.
+// TODO(mtrofin): is there a more maintainable algorithm?
+MappingResult TypeMapper::getSimpleAggregateTypeInternal(Type *Ty) {
+  auto Found = MappedTypes.find(Ty);
+  if (Found != MappedTypes.end())
+    return {Found->second, false};
+
+  LLVMContext &Ctx = Ty->getContext();

[JF, 2015/03/14 18:42:58]

You can have a single getContext in this file in runOnModule, and pass it to
each function that needs it.

[Mircea Trofin, 2015/03/16 18:38:45]

Done.

+
+  if (auto *OldFnTy = dyn_cast<FunctionType>(Ty)) {
+    Type *OldRetType = OldFnTy->getReturnType();
+    Type *NewRetType = OldRetType;
+    Type *Void = Type::getVoidTy(Ctx);
+    SmallVector<Type *, TypicalFuncArity> NewArgs;
+    bool Changed = false;
+    // struct register returns become the first parameter of the new FT.
+    // the new FT has void for the return type

[JF, 2015/03/14 18:42:58]

Capitalize and punctuation on comments, here and elsewhere.

[Mircea Trofin, 2015/03/16 18:38:45]

Done.

[JF, 2015/03/16 21:53:47]

Punctuation is still missing in a few places.

+    if (OldRetType->isAggregateType()) {
+      NewRetType = Void;
+      Changed = true;
+      NewArgs.push_back(getSimpleArgumentType(OldRetType));
+    }
+    for (auto OldParam : OldFnTy->params()) {
+      auto NewType = getSimpleArgumentType(OldParam);
+      Changed |= NewType.isChanged();
+      NewArgs.push_back(NewType);
+    }
+    Type *NewFuncType =
+        FunctionType::get(NewRetType, NewArgs, OldFnTy->isVarArg());
+    return {NewFuncType, Changed};
+  }
+
+  if (auto PtrTy = dyn_cast<PointerType>(Ty)) {
+    auto NewTy = getSimpleAggregateTypeInternal(PtrTy->getPointerElementType());
+
+    return {NewTy->getPointerTo(PtrTy->getAddressSpace()), NewTy.isChanged()};
+  }
+
+  if (auto ArrTy = dyn_cast<ArrayType>(Ty)) {
+    auto NewTy = getSimpleAggregateTypeInternal(ArrTy->getArrayElementType());
+    return {ArrayType::get(NewTy, ArrTy->getArrayNumElements()),
+            NewTy.isChanged()};
+  }
+
+  if (auto VecTy = dyn_cast<VectorType>(Ty)) {
+    auto NewTy = getSimpleAggregateTypeInternal(VecTy->getVectorElementType());
+    return {VectorType::get(NewTy, VecTy->getVectorNumElements()),
+            NewTy.isChanged()};
+  }
+
+  if (auto StructTy = dyn_cast<StructType>(Ty)) {
+    if (!StructTy->isLiteral()) {
+      // LLVM doesn't intern identified structs (the ones with a name). This,
+      // together with the fact that such structs can be recursive,
+      // complicates things a bit. We want to make sure that we only change
+      // "unsimplified" structs (those that somehow reference funcs that
+      // are not simple).
+      // We don't want to change "simplified" structs, otherwise converting
+      // instruction types will become trickier.
+      auto CandidateMapping = MappedTypes.find(Ty);
+      if (CandidateMapping == MappedTypes.end()) {

[JF, 2015/03/14 18:42:58]

I would flip the polarity of this:
  if (... != end) {
    ...
    return ...;
  }

  // Rest goes here.

+        // We don't have a mapping, and we don't know if the struct is recursive
+        // so we create an empty one and hypothesize that it is the
+        // mapping.
+        SmallString<256> Storage;
+        Twine NewName = StructTy->getStructName() + ".simplified";

[JF, 2015/03/14 18:42:58]

You can 's/Twine/StringRef/' and drop the Storage.

[Mircea Trofin, 2015/03/16 18:38:45]

If StructType::create accepted a Twine &, that'd work, but it wants a StringRef

[JF, 2015/03/16 21:53:46]

Twine has a StringRef ctor, so it'll work. All you need is:
  MappedTypes[Ty] = StructType::create(Ctx, StructTy->getStructName() +
".simplified");

+        MappedTypes[Ty] = StructType::create(Ctx, NewName.toStringRef(Storage));
+      } else {
+        // We either have a finished mapping or this is the empty placeholder
+        // created above, and we are in the process of finalizing it.
+        // 1) if this is a mapping, it must have the same element count
+        // as the original struct, so we mark a change if the types are
+        // different objects
+        // 2) if this is a placeholder, the element count
+        // getStructNumElements() sill differ.
+        // Since we don't know yet if this is a change or not - because we
+        // are constructing the mapping - we don't mark as change. We decide
+        // if it is a change below, based on the other struct elements.
+        Type *CurrentlyMapped = CandidateMapping->second;

[JF, 2015/03/14 18:42:58]

You could cast<StructType>(CandidateMapping->second) here, drop the assert and
simplify Changed's get* methods (which themselves contain a cast<StructType>).

+        assert(CurrentlyMapped->isStructTy());
+        bool Changed = StructTy != CurrentlyMapped &&
+                       StructTy->getStructNumElements() ==
+                           CurrentlyMapped->getStructNumElements();
+        return {CurrentlyMapped, Changed};
+      }
+    }
+
+    SmallVector<Type *, TypicalStructArity> ElemTypes;
+    bool Changed = false;
+    unsigned StructElemCount = StructTy->getStructNumElements();
+    for (unsigned I = 0; I < StructElemCount; I++) {
+      auto NewElem =
+          getSimpleAggregateTypeInternal(Ty->getStructElementType(I));
+      ElemTypes.push_back(NewElem);
+      Changed |= NewElem.isChanged();
+    }
+    if (!Changed) {
+      // We are leaking the created struct here, but there is no way to
+      // correctly delete it.
+      // Replace whatever mapping we had with Ty
+      return {MappedTypes[Ty] = Ty, false};
+    }
+
+    if (StructTy->isLiteral()) {
+      return {MappedTypes[Ty] =
+                  StructType::get(Ctx, ElemTypes, StructTy->isPacked()),
+              Changed};
+    } else {
+      // Lookup the mapping again, as it may have been changed/replaced
+      auto CandidateMapping = MappedTypes.find(Ty);
+      StructType *NewStruct = dyn_cast<StructType>(CandidateMapping->second);
+      assert(NewStruct);

[JF, 2015/03/14 18:42:58]

You can just use cast<> instead of dyn_cast<>, and drop the assert.

[Mircea Trofin, 2015/03/16 18:38:45]

Done.

+      NewStruct->setBody(ElemTypes, StructTy->isPacked());
+      return {NewStruct, true};
+    }
+  }
+
+  // Anything else stays the same.
+  return {Ty, false};
+}
+
+// get the simplified type of a function argument.
+MappingResult TypeMapper::getSimpleArgumentType(Type *Ty) {
+  // struct registers become pointers to simple structs
+  if (Ty->isAggregateType()) {
+    return MappingResult(
+        PointerType::get(getSimpleAggregateTypeInternal(Ty), 0), true);
+  }
+
+  return getSimpleAggregateTypeInternal(Ty);
+}
+
+// apply 'byval' to func arguments that used to be struct regs.
+// apply 'sret' to the argument corresponding to the return in the old signature
+static void ApplyByValAndSRet(Function *OldFunc, Function *NewFunc) {
+  // when calling addAttribute, the first one refers to the function, so we
+  // skip past that.
+  unsigned ArgOffset = 1;
+  if (OldFunc->getReturnType()->isAggregateType()) {
+    NewFunc->addAttribute(1, Attribute::AttrKind::StructRet);
+    ArgOffset++;
+  }
+
+  auto &NewArgList = NewFunc->getArgumentList();
+  auto NewArg = NewArgList.begin();
+  for (const Argument &OldArg : OldFunc->getArgumentList()) {
+    if (OldArg.getType()->isAggregateType()) {
+      NewFunc->addAttribute(NewArg->getArgNo() + ArgOffset,
+                            Attribute::AttrKind::ByVal);
+    }
+    NewArg++;
+  }
+}
+
+// update the arg names for a newly created function
+static void UpdateArgNames(Function *OldFunc, Function *NewFunc) {
+  auto NewArgIter = NewFunc->arg_begin();
+  if (OldFunc->getReturnType()->isAggregateType()) {
+    NewArgIter->setName("retVal");
+    NewArgIter++;
+  }
+
+  for (const Argument &OldArg : OldFunc->args()) {
+    Argument *NewArg = NewArgIter++;
+    if (OldArg.getType()->isAggregateType()) {
+      NewArg->setName(OldArg.getName() + ".ptr");
+    } else {
+      NewArg->setName(OldArg.getName());
+    }

[JF, 2015/03/14 18:42:58]

Personal style, I'd go with:
  NewArg->setName(OldArg.getName() + (
    OldArg.getType()->isAggregateType() ? ".ptr" : ""));

[Mircea Trofin, 2015/03/16 18:38:45]

Done.

+  }
+}
+
+// Replace all uses of an old value with a new one, disregarding the type. We
+// correct the types after we wire the new parameters in, in fixFunctionBody.
+static void BlindReplace(Value *Old, Value *New) {
+  for (auto UseIter = Old->use_begin(), E = Old->use_end(); E != UseIter;) {
+    Use &AUse = *(UseIter++);
+    AUse.set(New);
+  }
+}
+
+// Adapt the body of a function for the new arguments.
+static void ConvertArgumentValue(Value *Old, Value *New,
+                                 Instruction *InsPoint) {
+  if (Old == New)
+    return;
+
+  if (Old->getType() == New->getType()) {
+    Old->replaceAllUsesWith(New);
+    New->takeName(Old);
+    return;
+  }
+
+  if (Old->getType()->isAggregateType() && New->getType()->isPointerTy()) {
+    Value *Load = new LoadInst(New, Old->getName() + ".sreg", InsPoint);
+    BlindReplace(Old, Load);
+  } else {
+    BlindReplace(Old, New);
+  }

[JF, 2015/03/14 18:42:59]

Same persona style here, I'd do:
bool isAggregateToPtr = ...;
BlindReplace(Old, isAggregateToPtr ? new LoadInst(...) : New);

[Mircea Trofin, 2015/03/16 18:38:45]

Done.

+}
+
+// Fix returns. Return true if fixes were needed.
+static void FixReturn(Function *OldFunc, Function *NewFunc) {
+
+  Argument *FirstNewArg = NewFunc->getArgumentList().begin();
+  auto &Ctx = NewFunc->getContext();
+
+  for (auto BIter = NewFunc->begin(), LastBlock = NewFunc->end();
+       LastBlock != BIter;) {
+    BasicBlock *BB = BIter++;
+    for (auto IIter = BB->begin(), LastI = BB->end(); LastI != IIter;) {
+      Instruction *Instr = IIter++;
+      if (ReturnInst *Ret = dyn_cast<ReturnInst>(Instr)) {
+        auto RetVal = Ret->getReturnValue();
+        ReturnInst *NewRet = ReturnInst::Create(Ctx, nullptr, Ret);
+        StoreInst *Store = new StoreInst(RetVal, FirstNewArg, NewRet);
+        Store->setAlignment(FirstNewArg->getParamAlignment());
+        Store->setDebugLoc(Ret->getDebugLoc());
+        Ret->eraseFromParent();
+        Ret = nullptr;

[JF, 2015/03/14 18:42:58]

No point in setting Ret to nullptr since it's going out of scope.

[Mircea Trofin, 2015/03/16 18:38:45]

Maintainability - makes it clear that Ret is invalid after eraseFromParent()

+      }
+    }
+  }
+}
+
+// TODO (mtrofin): is this comprehensive?
+template <class TCall>
+void CopyCallAttributesAndMetadata(TCall *Orig, TCall *NewCall) {
+  NewCall->setCallingConv(Orig->getCallingConv());
+  NewCall->setAttributes(NewCall->getAttributes().addAttributes(
+      Orig->getContext(), AttributeSet::FunctionIndex,
+      Orig->getAttributes().getFnAttributes()));
+  NewCall->setDebugLoc(Orig->getDebugLoc());
+  NewCall->takeName(Orig);
+}
+
+static InvokeInst *CreateCallFrom(InvokeInst *Orig, Value *Target,
+                                  ArrayRef<Value *> &Args) {
+  InvokeInst *Ret = InvokeInst::Create(Target, Orig->getNormalDest(),
+                                       Orig->getUnwindDest(), Args);
+  CopyCallAttributesAndMetadata(Orig, Ret);
+  return Ret;
+}
+
+static CallInst *CreateCallFrom(CallInst *Orig, Value *Target,
+                                ArrayRef<Value *> &Args) {
+
+  CallInst *Ret = CallInst::Create(Target, Args);
+  Ret->setTailCallKind(Orig->getTailCallKind());
+  CopyCallAttributesAndMetadata(Orig, Ret);
+  return Ret;
+}
+
+// Fix a call site by handing return type changes and/or parameter type and
+// attribute changes.
+template <class TCall>
+void SimplifyStructRegSignatures::fixCallSite(TCall *OldCall) {
+  Value *NewTarget = OldCall->getCalledValue();
+
+  if (Function *CalledFunc = dyn_cast<Function>(NewTarget)) {
+    NewTarget = this->FunctionMap[CalledFunc];
+  }
+  assert(NewTarget);
+
+  FunctionType *NewType = cast<FunctionType>(
+      Mapper.getSimpleType(NewTarget->getType())->getPointerElementType());
+
+  Type *OldRetType = OldCall->getType();
+  const bool isSRet =
+      !OldCall->getType()->isVoidTy() && NewType->getReturnType()->isVoidTy();
+
+  const unsigned argOffset = isSRet ? 1 : 0;
+
+  SmallVector<Value *, TypicalFuncArity> NewArgs;
+
+  if (isSRet) {
+    AllocaInst *Alloca = new AllocaInst(OldRetType);
+    Alloca->takeName(OldCall);
+    Alloca->insertBefore(OldCall);
+    Alloca->setAlignment(PreferredAlignment);
+    NewArgs.push_back(Alloca);
+
+    LoadInst *Load = new LoadInst(Alloca, Alloca->getName() + ".sreg",
+                                  (Instruction *)nullptr);
+    Load->setAlignment(Alloca->getAlignment());
+    Load->insertAfter(OldCall);
+    OldCall->replaceAllUsesWith(Load);
+  }
+
+  SmallSetVector<unsigned, TypicalFuncArity> ByRefPlaces;
+
+  for (unsigned ArgPos = 0;
+       ArgPos < NewType->getFunctionNumParams() - argOffset; ArgPos++) {
+
+    Use &OldArgUse = OldCall->getOperandUse(ArgPos);
+    Value *OldArg = OldArgUse;
+    Type *OldArgType = OldArg->getType();
+    unsigned NewArgPos = OldArgUse.getOperandNo() + argOffset;
+    Type *NewArgType = NewType->getFunctionParamType(NewArgPos);
+
+    if (OldArgType != NewArgType && OldArgType->isAggregateType()) {
+      AllocaInst *Alloca =
+          new AllocaInst(OldArgType, OldArg->getName() + ".ptr", OldCall);
+      new StoreInst(OldArg, Alloca, OldCall);
+      ByRefPlaces.insert(NewArgPos);
+      NewArgs.push_back(Alloca);
+    } else {
+      NewArgs.push_back(OldArg);
+    }
+  }
+
+  ArrayRef<Value *> ArrRef = NewArgs;
+  TCall *NewCall = CreateCallFrom(OldCall, NewTarget, ArrRef);
+
+  // copy the attributes over, and add byref/sret as necessary
+  const AttributeSet &OldAttrSet = OldCall->getAttributes();
+  const AttributeSet &NewAttrSet = NewCall->getAttributes();
+  LLVMContext &Ctx = OldCall->getContext();
+  AttrBuilder Builder(OldAttrSet, 0);
+
+  for (unsigned I = 0; I < NewCall->getNumArgOperands(); I++) {
+    NewCall->setAttributes(NewAttrSet.addAttributes(
+        Ctx, I + argOffset + 1, OldAttrSet.getParamAttributes(I + 1)));
+    if (ByRefPlaces.count(I)) {
+      NewCall->addAttribute(I + 1, Attribute::AttrKind::ByVal);
+    }
+  }
+
+  if (isSRet) {
+    NewAttrSet.addAttributes(Ctx, 1, OldAttrSet.getRetAttributes());
+    NewCall->addAttribute(1, Attribute::AttrKind::StructRet);
+  } else {
+    NewCall->setAttributes(NewAttrSet.addAttributes(
+        Ctx, AttributeSet::ReturnIndex, OldAttrSet.getRetAttributes()));
+    // if we still return something, this is the value to replace the old
+    // call with
+    OldCall->replaceAllUsesWith(NewCall);
+  }
+
+  NewCall->insertBefore(OldCall);
+  OldCall->eraseFromParent();
+  OldCall = NULL;

[JF, 2015/03/14 18:42:58]

nullptr, but there's no point in doing this since it's going out of scope.

[Mircea Trofin, 2015/03/16 18:38:45]

Just maintainability, clarifies the effect of eraseFromParent().

+}
+
+void SimplifyStructRegSignatures::scheduleCallsForCleanup(Function *NewFunc) {
+  for (auto &BBIter : NewFunc->getBasicBlockList()) {
+    for (auto &IIter : BBIter.getInstList()) {
+      if (CallInst *Call = dyn_cast<CallInst>(&IIter)) {
+        CallsToPatch.insert(Call);
+      } else if (InvokeInst *Invoke = dyn_cast<InvokeInst>(&IIter)) {
+        InvokesToPatch.insert(Invoke);
+      }
+    }
+  }
+}
+
+// Change function body in the light of type changes.
+void SimplifyStructRegSignatures::fixFunctionBody(Function *OldFunc,
+                                                  Function *NewFunc) {
+  if (NewFunc->empty())
+    return;
+
+  bool returnWasFixed = OldFunc->getReturnType()->isAggregateType();
+
+  Instruction *InsPoint = NewFunc->begin()->begin();
+  auto NewArgIter = NewFunc->arg_begin();
+  // advance one more if we used to return a struct register
+  if (returnWasFixed)
+    NewArgIter++;
+
+  // wire new parameters in
+  for (auto ArgIter = OldFunc->arg_begin(), E = OldFunc->arg_end();
+       E != ArgIter;) {
+    Argument *OldArg = ArgIter++;
+    Argument *NewArg = NewArgIter++;
+    ConvertArgumentValue(OldArg, NewArg, InsPoint);
+  }
+
+  // Now fix instruction types. Calls are dealt with separately, but we still
+  // update the types here. We know that each value could only possibly be
+  // of a simplified type. At the end of this, call sites will be invalid, but
+  // we handle that afterwards, to make sure we have all the functions changed
+  // first (so that calls have valid targets)
+  for (auto BBIter = NewFunc->begin(), LBlock = NewFunc->end();
+       LBlock != BBIter;) {
+    auto Block = BBIter++;
+    for (auto IIter = Block->begin(), LIns = Block->end(); LIns != IIter;) {
+      auto Instr = IIter++;
+      Instr->mutateType(Mapper.getSimpleType(Instr->getType()));
+    }
+  }
+  if (returnWasFixed)
+    FixReturn(OldFunc, NewFunc);
+}
+
+// Ensure function is simplified, returning true if the function
+// had to be changed.
+bool SimplifyStructRegSignatures::simplifyFunction(Function *OldFunc,
+                                                   Module &M) {
+  FunctionType *OldFT = OldFunc->getFunctionType();
+  FunctionType *NewFT = dyn_cast<FunctionType>(Mapper.getSimpleType(OldFT));
+  assert(NewFT);

[JF, 2015/03/14 18:42:58]

cast<FunctionType> above, drop the assert.

+
+  Function *&AssociatedFctLoc = FunctionMap[OldFunc];
+  if (NewFT != OldFT) {
+    Function *NewFunc = Function::Create(NewFT, OldFunc->getLinkage());
+    AssociatedFctLoc = NewFunc;
+
+    NewFunc->copyAttributesFrom(OldFunc);
+    OldFunc->getParent()->getFunctionList().insert(OldFunc, NewFunc);
+    NewFunc->takeName(OldFunc);
+
+    UpdateArgNames(OldFunc, NewFunc);
+    ApplyByValAndSRet(OldFunc, NewFunc);
+
+    NewFunc->getBasicBlockList().splice(NewFunc->begin(),
+                                        OldFunc->getBasicBlockList());
+
+    fixFunctionBody(OldFunc, NewFunc);
+    FunctionsToDelete.insert(OldFunc);
+  } else {
+    AssociatedFctLoc = OldFunc;
+  }
+  scheduleCallsForCleanup(AssociatedFctLoc);
+  return NewFT != OldFT;
+}
+
+bool SimplifyStructRegSignatures::runOnModule(Module &M) {
+  bool Changed = false;
+
+  const DataLayout *DL = M.getDataLayout();
+  if (DL)
+    PreferredAlignment = DL->getStackAlignment();
+
+  // change function signatures and fix a changed function body by
+  // wiring the new arguments. Call sites are unchanged at this point
+  for (Module::iterator Iter = M.begin(), E = M.end(); Iter != E;) {
+    Function *Func = Iter++;
+    Changed |= simplifyFunction(Func, M);
+  }
+
+  // fix call sites
+  for (auto &CallToFix : CallsToPatch) {
+    fixCallSite(CallToFix);
+  }
+
+  for (auto &InvokeToFix : InvokesToPatch) {
+    fixCallSite(InvokeToFix);
+  }
+
+  // delete leftover functions - the ones with old signatures
+  for (auto &ToDelete : FunctionsToDelete) {
+    // this also frees the memory
+    ToDelete->eraseFromParent();
+  }
+  return Changed;
+}
+
+ModulePass *llvm::createSimplifyStructRegSignaturesPass() {
+  return new SimplifyStructRegSignatures();
+}