SIL: introduce a PlaceholderValue and use it in the parser and deserializer.

... instead of a GlobalAddrInst.
This is cleaner and makes the handling of forward-referenced values in the deserializer a bit simpler.

Author: eeckstein

include/swift/SIL/SILNodes.def

@@ -415,6 +415,10 @@ VALUE(MultipleValueInstructionResult, ValueBase)
 
 VALUE(SILUndef, ValueBase)
 
+/// Used internally in e.g. the SIL parser and deserializer.
+/// A PlaceholderValue must not appear in valid SIL.
+VALUE(PlaceholderValue, ValueBase)
+
 ABSTRACT_NODE(SILInstruction, SILNode)
 
 ABSTRACT_VALUE_AND_INST(SingleValueInstruction, ValueBase, SILInstruction)

include/swift/SIL/SILValue.h

@@ -37,6 +37,7 @@ class PostOrderFunctionInfo;
 class ReversePostOrderInfo;
 class Operand;
 class SILInstruction;
+class SILArgument;
 class SILLocation;
 class DeadEndBlocks;
 class ValueBaseUseIterator;
@@ -1353,6 +1354,25 @@ inline llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, SILValue V) {
   return OS;
 }
 
+/// Used internally in e.g. the SIL parser and deserializer to handle forward-
+/// referenced values.
+/// A PlaceholderValue must not appear in valid SIL.
+class PlaceholderValue : public ValueBase {
+  static int numPlaceholderValuesAlive;
+
+public:
+  PlaceholderValue(SILType type);
+  ~PlaceholderValue();
+
+  static int getNumPlaceholderValuesAlive() { return numPlaceholderValuesAlive; }
+
+  static bool classof(const SILArgument *) = delete;
+  static bool classof(const SILInstruction *) = delete;
+  static bool classof(SILNodePointer node) {
+    return node->getKind() == SILNodeKind::PlaceholderValue;
+  }
+};
+
 } // end namespace swift
 
 

lib/SIL/IR/SILModule.cpp

@@ -169,6 +169,9 @@ void SILModule::checkForLeaks() const {
     llvm::errs() << "Instructions in module: " << instsInModule << '\n';
     llvm_unreachable("leaking instructions");
   }
+  
+  assert(PlaceholderValue::getNumPlaceholderValuesAlive() == 0 &&
+         "leaking placeholders");
 }
 
 void SILModule::checkForLeaksAfterDestruction() {

lib/SIL/IR/SILPrinter.cpp

@@ -1042,6 +1042,12 @@ class SILPrinter : public SILInstructionVisitor<SILPrinter> {
       printSILUndef(cast<SILUndef>(node));
       return;
 
+    case SILNodeKind::PlaceholderValue:
+      // This should really only happen during debugging.
+      *this << "placeholder<" << cast<PlaceholderValue>(node)->getType()
+            << ">\n";
+      return;
+
     case SILNodeKind::MultipleValueInstructionResult:
       printSILMultipleValueInstructionResult(
           cast<MultipleValueInstructionResult>(node));

lib/SIL/IR/SILValue.cpp

@@ -388,3 +388,19 @@ llvm::raw_ostream &swift::operator<<(llvm::raw_ostream &os,
                                      const OperandOwnership &operandOwnership) {
   return os << operandOwnership.asString();
 }
+
+//===----------------------------------------------------------------------===//
+//                              PlaceholderValue
+//===----------------------------------------------------------------------===//
+
+int PlaceholderValue::numPlaceholderValuesAlive = 0;
+
+PlaceholderValue::PlaceholderValue(SILType type)
+      : ValueBase(ValueKind::PlaceholderValue, type) {
+  numPlaceholderValuesAlive++;
+}
+
+PlaceholderValue::~PlaceholderValue() {
+  numPlaceholderValuesAlive--;
+}
+

lib/SIL/IR/ValueOwnership.cpp

@@ -287,6 +287,11 @@ ValueOwnershipKind ValueOwnershipKindClassifier::visitSILUndef(SILUndef *arg) {
   return arg->getOwnershipKind();
 }
 
+ValueOwnershipKind ValueOwnershipKindClassifier::
+visitPlaceholderValue(PlaceholderValue *v) {
+  return OwnershipKind::None;
+}
+
 ValueOwnershipKind ValueOwnershipKindClassifier::
 visitMultipleValueInstructionResult(MultipleValueInstructionResult *Result) {
   return Result->getOwnershipKind();

lib/SIL/Parser/ParseSIL.cpp

@@ -500,8 +500,7 @@ SILParser::~SILParser() {
   for (auto &Entry : ForwardRefLocalValues) {
     if (ValueBase *dummyVal = LocalValues[Entry.first()]) {
       dummyVal->replaceAllUsesWith(SILUndef::get(dummyVal->getType(), SILMod));
-      SILInstruction::destroy(cast<GlobalAddrInst>(dummyVal));
-      SILMod.deallocateInst(cast<GlobalAddrInst>(dummyVal));
+      ::delete cast<PlaceholderValue>(dummyVal);
     }
   }
 }
@@ -687,7 +686,7 @@ SILValue SILParser::getLocalValue(UnresolvedValueName Name, SILType Type,
   // it until we see a real definition.
   ForwardRefLocalValues[Name.Name] = Name.NameLoc;
 
-  Entry = new (SILMod) GlobalAddrInst(getDebugLoc(B, Loc), Type);
+  Entry = ::new PlaceholderValue(Type);
   return Entry;
 }
 
@@ -715,8 +714,7 @@ void SILParser::setLocalValue(ValueBase *Value, StringRef Name,
     } else {
       // Forward references only live here if they have a single result.
       Entry->replaceAllUsesWith(Value);
-      SILInstruction::destroy(cast<GlobalAddrInst>(Entry));
-      SILMod.deallocateInst(cast<GlobalAddrInst>(Entry));
+      ::delete cast<PlaceholderValue>(Entry);
     }
     Entry = Value;
     return;

lib/SIL/Verifier/SILVerifier.cpp

@@ -1111,6 +1111,8 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
     // Verify some basis structural stuff about an instruction's operands.
     for (auto &operand : I->getAllOperands()) {
       require(operand.get(), "instruction has null operand");
+      require(!isa<PlaceholderValue>(operand.get()),
+              "instruction has placeholder operand");
 
       if (auto *valueI = operand.get()->getDefiningInstruction()) {
         require(valueI->getParent(),

lib/Serialization/DeserializeSIL.cpp

@@ -262,29 +262,23 @@ SILDeserializer::readFuncTable(ArrayRef<uint64_t> fields, StringRef blobData) {
 
 /// A high-level overview of how forward references work in serializer and
 /// deserializer:
-/// In serializer, we pre-assign a value ID in order, to each basic block
+/// In the serializer, we pre-assign a value ID in order, to each basic block
 /// argument and each SILInstruction that has a value.
-/// In deserializer, we use LocalValues to store the definitions and
-/// ForwardLocalValues for forward-referenced values (values that are
-/// used but not yet defined). LocalValues are updated in setLocalValue where
-/// the ID passed in assumes the same ordering as in serializer: in-order
-/// for each basic block argument and each SILInstruction that has a value.
-/// We update ForwardLocalValues in getLocalValue and when a value is defined
-/// in setLocalValue, the corresponding entry in ForwardLocalValues will be
-/// erased.
+/// In the deserializer, we create a PlaceholderValue for a forward-referenced
+/// value (a value that is used but not yet defined). LocalValues are updated in
+/// setLocalValue where the ID passed in assumes the same ordering as in
+/// serializer: in-order for each basic block argument and each SILInstruction
+/// that has a value.
+/// When a forward-referenced value is defined, it replaces the PlaceholderValue
+/// in LocalValues.
 void SILDeserializer::setLocalValue(ValueBase *Value, ValueID Id) {
   ValueBase *&Entry = LocalValues[Id];
-  assert(!Entry && "We should not redefine the same value.");
 
-  auto It = ForwardLocalValues.find(Id);
-  if (It != ForwardLocalValues.end()) {
-    // Take the information about the forward ref out of the map.
-    ValueBase *Placeholder = It->second;
-
-    // Remove the entries from the map.
-    ForwardLocalValues.erase(It);
-
-    Placeholder->replaceAllUsesWith(Value);
+  if (auto *placeholder = dyn_cast_or_null<PlaceholderValue>(Entry)) {
+    placeholder->replaceAllUsesWith(Value);
+    ::delete placeholder;
+  } else {
+    assert(!Entry && "We should not redefine the same value.");
   }
 
   // Store it in our map.
@@ -301,19 +295,15 @@ SILValue SILDeserializer::getLocalValue(ValueID Id,
                     "changes that without updating this code if needed");
 
   // Check to see if this is already defined.
-  ValueBase *Entry = LocalValues.lookup(Id);
-  if (Entry) {
-    // If this value was already defined, check it to make sure types match.
-    assert(Entry->getType() == Type && "Value Type mismatch?");
-    return Entry;
-  }
-
-  // Otherwise, this is a forward reference.  Create a dummy node to represent
-  // it until we see a real definition.
-  ValueBase *&Placeholder = ForwardLocalValues[Id];
-  if (!Placeholder)
-    Placeholder = new (SILMod) GlobalAddrInst(SILDebugLocation(), Type);
-  return Placeholder;
+  ValueBase *&Entry = LocalValues[Id];
+  if (!Entry) {
+    // Otherwise, this is a forward reference.  Create a dummy node to represent
+    // it until we see a real definition.
+    Entry = ::new PlaceholderValue(Type);
+  }
+  // If this value was already defined, check it to make sure types match.
+  assert(Entry->getType() == Type && "Value Type mismatch?");
+  return Entry;
 }
 
 /// Return the SILBasicBlock of a given ID.
@@ -807,7 +797,6 @@ SILDeserializer::readSILFunctionChecked(DeclID FID, SILFunction *existingFn,
   // The first two IDs are reserved for SILUndef.
   LastValueID = 1;
   LocalValues.clear();
-  ForwardLocalValues.clear();
 
   SILOpenedArchetypesTracker OpenedArchetypesTracker(fn);
   SILBuilder Builder(*fn);
@@ -3034,11 +3023,9 @@ SILGlobalVariable *SILDeserializer::readGlobalVar(StringRef Name) {
   SILBuilder Builder(v);
 
   llvm::DenseMap<uint32_t, ValueBase*> SavedLocalValues;
-  llvm::DenseMap<uint32_t, ValueBase*> SavedForwardLocalValues;
   serialization::ValueID SavedLastValueID = 1;
 
   SavedLocalValues.swap(LocalValues);
-  SavedForwardLocalValues.swap(ForwardLocalValues);
   std::swap(SavedLastValueID, LastValueID);
 
   while (kind != SIL_FUNCTION && kind != SIL_VTABLE && kind != SIL_GLOBALVAR &&
@@ -3066,7 +3053,6 @@ SILGlobalVariable *SILDeserializer::readGlobalVar(StringRef Name) {
   }
 
   SavedLocalValues.swap(LocalValues);
-  SavedForwardLocalValues.swap(ForwardLocalValues);
   std::swap(SavedLastValueID, LastValueID);
 
   return v;

lib/Serialization/DeserializeSIL.h

@@ -85,7 +85,6 @@ namespace swift {
 
     /// Data structures used to perform name lookup for local values.
     llvm::DenseMap<uint32_t, ValueBase*> LocalValues;
-    llvm::DenseMap<uint32_t, ValueBase*> ForwardLocalValues;
 
     /// The first two local values are reserved for SILUndef.
     serialization::ValueID LastValueID = 1;