SILBuilder: require an insertion point when creating instructions.

This helps to avoid instruction leaks.
It's a NFC.

Author: eeckstein

include/swift/SIL/SILBuilder.h

@@ -2358,15 +2358,11 @@ class SILBuilder {
   }
 
   void insertImpl(SILInstruction *TheInst) {
-    if (BB == 0)
-      return;
-
+    assert(hasValidInsertionPoint());
     BB->insert(InsertPt, TheInst);
 
     C.notifyInserted(TheInst);
 
-// TODO: We really shouldn't be creating instructions unless we are going to
-// insert them into a block... This failed in SimplifyCFG.
 #ifndef NDEBUG
     TheInst->verifyOperandOwnership();
 #endif

lib/SIL/IR/SILInstruction.cpp

@@ -1149,11 +1149,14 @@ namespace {
     friend class SILCloner<TrivialCloner>;
     friend class SILInstructionVisitor<TrivialCloner>;
     SILInstruction *Result = nullptr;
-    TrivialCloner(SILFunction *F) : SILCloner(*F) {}
+    TrivialCloner(SILFunction *F, SILInstruction *InsertPt) : SILCloner(*F) {
+      Builder.setInsertionPoint(InsertPt);
+    }
+
   public:
 
-    static SILInstruction *doIt(SILInstruction *I) {
-      TrivialCloner TC(I->getFunction());
+    static SILInstruction *doIt(SILInstruction *I, SILInstruction *InsertPt) {
+      TrivialCloner TC(I->getFunction(), InsertPt);
       TC.visit(I);
       return TC.Result;
     }
@@ -1204,10 +1207,7 @@ bool SILInstruction::isDeallocatingStack() const {
 /// then the new instruction is inserted before the specified point, otherwise
 /// the new instruction is returned without a parent.
 SILInstruction *SILInstruction::clone(SILInstruction *InsertPt) {
-  SILInstruction *NewInst = TrivialCloner::doIt(this);
-
-  if (NewInst && InsertPt)
-    InsertPt->getParent()->insert(InsertPt, NewInst);
+  SILInstruction *NewInst = TrivialCloner::doIt(this, InsertPt);
   return NewInst;
 }
 

lib/SILOptimizer/IPO/CrossModuleSerializationSetup.cpp

@@ -96,8 +96,10 @@ class InstructionVisitor : public SILCloner<InstructionVisitor> {
   SILInstruction *result = nullptr;
 
 public:
-  InstructionVisitor(SILFunction *F, CrossModuleSerializationSetup &CMS) :
-    SILCloner(*F), CMS(CMS) {}
+  InstructionVisitor(SILInstruction *I, CrossModuleSerializationSetup &CMS) :
+    SILCloner(*I->getFunction()), CMS(CMS) {
+    Builder.setInsertionPoint(I);
+  }
 
   SILType remapType(SILType Ty) {
     CMS.makeTypeUsableFromInline(Ty.getASTType());
@@ -124,11 +126,9 @@ class InstructionVisitor : public SILCloner<InstructionVisitor> {
   SILBasicBlock *remapBasicBlock(SILBasicBlock *BB) { return BB; }
 
   static void visitInst(SILInstruction *I, CrossModuleSerializationSetup &CMS) {
-    InstructionVisitor visitor(I->getFunction(), CMS);
+    InstructionVisitor visitor(I, CMS);
     visitor.visit(I);
-
-    SILInstruction::destroy(visitor.result);
-    CMS.M.deallocateInst(visitor.result);
+    visitor.result->eraseFromParent();
   }
 };
 

lib/SILOptimizer/IPO/LetPropertiesOpts.cpp

@@ -190,11 +190,12 @@ class InitSequenceCloner : public SILClonerWithScopes<InitSequenceCloner> {
   friend class SILCloner<InitSequenceCloner>;
 
   const InitSequence &Init;
-  SILInstruction *DestIP;
 
 public:
   InitSequenceCloner(const InitSequence &init, SILInstruction *destIP)
-    : SILClonerWithScopes(*destIP->getFunction()), Init(init), DestIP(destIP) {}
+    : SILClonerWithScopes(*destIP->getFunction()), Init(init) {
+    Builder.setInsertionPoint(destIP);
+  }
 
   void process(SILInstruction *I) { visit(I); }
 
@@ -204,12 +205,6 @@ class InitSequenceCloner : public SILClonerWithScopes<InitSequenceCloner> {
     return SILCloner<InitSequenceCloner>::getMappedValue(Value);
   }
 
-  void postProcess(SILInstruction *orig, SILInstruction *cloned) {
-    DestIP->getParent()->push_front(cloned);
-    cloned->moveBefore(DestIP);
-    SILClonerWithScopes<InitSequenceCloner>::postProcess(orig, cloned);
-  }
-
   /// Clone all the instructions from Insns into the destination function,
   /// immediately before the destination block, and return the value of
   /// the result.