Merge remote-tracking branch 'origin/master' into master-rebranch

swift-ci · swift-ci · commit 1ad1811668cd · 2019-09-01T01:04:11.000-07:00
diff --git a/lib/SILOptimizer/Mandatory/SemanticARCOpts.cpp b/lib/SILOptimizer/Mandatory/SemanticARCOpts.cpp
@@ -44,9 +44,9 @@ STATISTIC(NumLoadCopyConvertedToLoadBorrow,
 ///
 /// Semantically this implies that a value is never passed off as +1 to memory
 /// or another function implying it can be used everywhere at +0.
-static bool
-isDeadLiveRange(SILValue v, SmallVectorImpl<DestroyValueInst *> &destroys,
-                SmallVectorImpl<SILInstruction *> &forwardingInsts) {
+static bool isDeadLiveRange(
+    SILValue v, SmallVectorImpl<DestroyValueInst *> &destroys,
+    NullablePtr<SmallVectorImpl<SILInstruction *>> forwardingInsts = nullptr) {
   assert(v.getOwnershipKind() == ValueOwnershipKind::Owned);
   SmallVector<Operand *, 32> worklist(v->use_begin(), v->use_end());
   while (!worklist.empty()) {
@@ -84,7 +84,8 @@ isDeadLiveRange(SILValue v, SmallVectorImpl<DestroyValueInst *> &destroys,
       //
       // NOTE: Today we do not support TermInsts for simplicity... we /could/
       // support it though if we need to.
-      if (isa<TermInst>(user) || !isGuaranteedForwardingInst(user) ||
+      if (forwardingInsts.isNull() || isa<TermInst>(user) ||
+          !isGuaranteedForwardingInst(user) ||
           1 != count_if(user->getOperandValues(
                             true /*ignore type dependent operands*/),
                         [&](SILValue v) {
@@ -97,7 +98,7 @@ isDeadLiveRange(SILValue v, SmallVectorImpl<DestroyValueInst *> &destroys,
       // Ok, this is a forwarding instruction whose ownership we can flip from
       // owned -> guaranteed. Visit its users recursively to see if the the
       // users force the live range to be alive.
-      forwardingInsts.push_back(user);
+      forwardingInsts.get()->push_back(user);
       for (SILValue v : user->getResults()) {
         if (v.getOwnershipKind() != ValueOwnershipKind::Owned)
           continue;
@@ -297,49 +298,6 @@ static bool canHandleOperand(SILValue operand, SmallVectorImpl<SILValue> &out) {
   return all_of(out, canHandleValue);
 }
 
-static void convertForwardingInstsFromOwnedToGuaranteed(
-    SmallVectorImpl<SILInstruction *> &guaranteedForwardingInsts) {
-  // Then change all of our guaranteed forwarding insts to have guaranteed
-  // ownership kind instead of what ever they previously had (ignoring trivial
-  // results);
-  while (!guaranteedForwardingInsts.empty()) {
-    auto *i = guaranteedForwardingInsts.pop_back_val();
-    assert(i->hasResults());
-
-    for (SILValue result : i->getResults()) {
-      if (auto *svi = dyn_cast<OwnershipForwardingSingleValueInst>(result)) {
-        if (svi->getOwnershipKind() == ValueOwnershipKind::Owned) {
-          svi->setOwnershipKind(ValueOwnershipKind::Guaranteed);
-        }
-        continue;
-      }
-
-      if (auto *ofci = dyn_cast<OwnershipForwardingConversionInst>(result)) {
-        if (ofci->getOwnershipKind() == ValueOwnershipKind::Owned) {
-          ofci->setOwnershipKind(ValueOwnershipKind::Guaranteed);
-        }
-        continue;
-      }
-
-      if (auto *sei = dyn_cast<OwnershipForwardingSelectEnumInstBase>(result)) {
-        if (sei->getOwnershipKind() == ValueOwnershipKind::Owned) {
-          sei->setOwnershipKind(ValueOwnershipKind::Guaranteed);
-        }
-        continue;
-      }
-
-      if (auto *mvir = dyn_cast<MultipleValueInstructionResult>(result)) {
-        if (mvir->getOwnershipKind() == ValueOwnershipKind::Owned) {
-          mvir->setOwnershipKind(ValueOwnershipKind::Guaranteed);
-        }
-        continue;
-      }
-
-      llvm_unreachable("unhandled forwarding instruction?!");
-    }
-  }
-}
-
 // Eliminate a copy of a borrowed value, if:
 //
 // 1. All of the copies users do not consume the copy (and thus can accept a
@@ -387,7 +345,7 @@ bool SemanticARCOptVisitor::performGuaranteedCopyValueOptimization(CopyValueInst
   // value (e.x. storing into memory).
   SmallVector<DestroyValueInst *, 16> destroys;
   SmallVector<SILInstruction *, 16> guaranteedForwardingInsts;
-  if (!isDeadLiveRange(cvi, destroys, guaranteedForwardingInsts))
+  if (!isDeadLiveRange(cvi, destroys, &guaranteedForwardingInsts))
     return false;
 
   // Next check if we have any destroys at all of our copy_value and an operand
@@ -494,8 +452,47 @@ bool SemanticARCOptVisitor::performGuaranteedCopyValueOptimization(CopyValueInst
   }
 
   eraseAndRAUWSingleValueInstruction(cvi, cvi->getOperand());
-  convertForwardingInstsFromOwnedToGuaranteed(guaranteedForwardingInsts);
 
+  // Then change all of our guaranteed forwarding insts to have guaranteed
+  // ownership kind instead of what ever they previously had (ignoring trivial
+  // results);
+  while (!guaranteedForwardingInsts.empty()) {
+    auto *i = guaranteedForwardingInsts.pop_back_val();
+
+    assert(i->hasResults());
+
+    for (SILValue result : i->getResults()) {
+      if (auto *svi = dyn_cast<OwnershipForwardingSingleValueInst>(result)) {
+        if (svi->getOwnershipKind() == ValueOwnershipKind::Owned) {
+          svi->setOwnershipKind(ValueOwnershipKind::Guaranteed);
+        }
+        continue;
+      }
+
+      if (auto *ofci = dyn_cast<OwnershipForwardingConversionInst>(result)) {
+        if (ofci->getOwnershipKind() == ValueOwnershipKind::Owned) {
+          ofci->setOwnershipKind(ValueOwnershipKind::Guaranteed);
+        }
+        continue;
+      }
+
+      if (auto *sei = dyn_cast<OwnershipForwardingSelectEnumInstBase>(result)) {
+        if (sei->getOwnershipKind() == ValueOwnershipKind::Owned) {
+          sei->setOwnershipKind(ValueOwnershipKind::Guaranteed);
+        }
+        continue;
+      }
+
+      if (auto *mvir = dyn_cast<MultipleValueInstructionResult>(result)) {
+        if (mvir->getOwnershipKind() == ValueOwnershipKind::Owned) {
+          mvir->setOwnershipKind(ValueOwnershipKind::Guaranteed);
+        }
+        continue;
+      }
+
+      llvm_unreachable("unhandled forwarding instruction?!");
+    }
+  }
   ++NumEliminatedInsts;
   return true;
 }
@@ -738,8 +735,7 @@ bool SemanticARCOptVisitor::visitLoadInst(LoadInst *li) {
   // -> load_borrow if we can put a copy_value on a cold path and thus
   // eliminate RR traffic on a hot path.
   SmallVector<DestroyValueInst *, 32> destroyValues;
-  SmallVector<SILInstruction *, 16> guaranteedForwardingInsts;
-  if (!isDeadLiveRange(li, destroyValues, guaranteedForwardingInsts))
+  if (!isDeadLiveRange(li, destroyValues))
     return false;
 
   // Then check if our address is ever written to. If it is, then we
@@ -764,8 +760,6 @@ bool SemanticARCOptVisitor::visitLoadInst(LoadInst *li) {
   }
 
   eraseAndRAUWSingleValueInstruction(li, lbi);
-  convertForwardingInstsFromOwnedToGuaranteed(guaranteedForwardingInsts);
-
   ++NumEliminatedInsts;
   ++NumLoadCopyConvertedToLoadBorrow;
   return true;
diff --git a/test/SILOptimizer/semantic-arc-opts.sil b/test/SILOptimizer/semantic-arc-opts.sil
@@ -437,28 +437,6 @@ bb0(%x : @guaranteed $ClassLet):
   return undef : $()
 }
 
-// CHECK-LABEL: sil [ossa] @dont_copy_let_properties_with_guaranteed_base_and_forwarding_uses :
-// CHECK:         ref_element_addr
-// CHECK-NEXT:    load_borrow
-// CHECK-NEXT:    unchecked_ref_cast
-// CHECK-NEXT:    apply
-// CHECK-NEXT:    end_borrow
-// CHECK-NEXT:    return
-// CHECK: } // end sil function 'dont_copy_let_properties_with_guaranteed_base_and_forwarding_uses'
-sil [ossa] @dont_copy_let_properties_with_guaranteed_base_and_forwarding_uses : $@convention(thin) (@guaranteed ClassLet) -> () {
-bb0(%x : @guaranteed $ClassLet):
-  %f = function_ref @black_hole : $@convention(thin) (@guaranteed Klass) -> ()
-
-  %p = ref_element_addr %x : $ClassLet, #ClassLet.aLet
-  %v = load [copy] %p : $*Klass
-  %c = unchecked_ref_cast %v : $Klass to $Klass
-  %b = begin_borrow %c : $Klass
-  apply %f(%b) : $@convention(thin) (@guaranteed Klass) -> ()
-  end_borrow %b : $Klass
-  destroy_value %c : $Klass
-  return undef : $()
-}
-
 // CHECK-LABEL: sil [ossa] @dont_copy_let_properties_with_guaranteed_upcast_base
 // CHECK:         ref_element_addr
 // CHECK-NEXT:    load_borrow
diff --git a/test/api-digester/Inputs/stdlib-stable-abi.json b/test/api-digester/Inputs/stdlib-stable-abi.json
diff --git a/test/api-digester/Inputs/stdlib-stable.json b/test/api-digester/Inputs/stdlib-stable.json
diff --git a/test/api-digester/stability-stdlib-abi-with-asserts.swift b/test/api-digester/stability-stdlib-abi-with-asserts.swift
@@ -2,7 +2,8 @@
 // REQUIRES: swift_stdlib_asserts
 // RUN: %empty-directory(%t.tmp)
 // mkdir %t.tmp/module-cache && mkdir %t.tmp/dummy.sdk
-// RUN: %api-digester -diagnose-sdk -module Swift -o %t.tmp/changes.txt -module-cache-path %t.tmp/module-cache -sdk %t.tmp/dummy.sdk -abi -avoid-location
+// RUN: %api-digester -dump-sdk -module Swift -o %t.tmp/current-stdlib.json -module-cache-path %t.tmp/module-cache -sdk %t.tmp/dummy.sdk -abi -avoid-location
+// RUN: %api-digester -diagnose-sdk -input-paths %S/Inputs/stdlib-stable-abi.json -input-paths %t.tmp/current-stdlib.json -abi -o %t.tmp/changes.txt -v
 // RUN: %clang -E -P -x c %S/Outputs/stability-stdlib-abi.without.asserts.swift.expected -o - > %t.tmp/stability-stdlib-abi.swift.expected
 // RUN: %clang -E -P -x c %S/Outputs/stability-stdlib-abi.asserts.additional.swift.expected -o - >> %t.tmp/stability-stdlib-abi.swift.expected
 // RUN: %clang -E -P -x c %t.tmp/stability-stdlib-abi.swift.expected -o - | sed '/^\s*$/d' | sort > %t.tmp/stability-stdlib-abi.swift.expected.sorted
diff --git a/test/api-digester/stability-stdlib-source.swift b/test/api-digester/stability-stdlib-source.swift
@@ -1,7 +1,8 @@
 // REQUIRES: OS=macosx
 // RUN: %empty-directory(%t.tmp)
 // mkdir %t.tmp/module-cache && mkdir %t.tmp/dummy.sdk
-// RUN: %api-digester -diagnose-sdk -module Swift -o %t.tmp/changes.txt -module-cache-path %t.tmp/module-cache -sdk %t.tmp/dummy.sdk -avoid-location
+// RUN: %api-digester -dump-sdk -module Swift -o %t.tmp/current-stdlib.json -module-cache-path %t.tmp/module-cache -sdk %t.tmp/dummy.sdk -avoid-location
+// RUN: %api-digester -diagnose-sdk -input-paths %S/Inputs/stdlib-stable.json -input-paths %t.tmp/current-stdlib.json -o %t.tmp/changes.txt -v
 // RUN: %clang -E -P -x c %S/Outputs/stability-stdlib-source.swift.expected -o - | sed '/^\s*$/d' | sort > %t.tmp/stability-stdlib-source.swift.expected
 // RUN: %clang -E -P -x c %t.tmp/changes.txt -o - | sed '/^\s*$/d' | sort > %t.tmp/changes.txt.tmp
 // RUN: diff -u %t.tmp/stability-stdlib-source.swift.expected %t.tmp/changes.txt.tmp
diff --git a/tools/swift-api-digester/ModuleAnalyzerNodes.cpp b/tools/swift-api-digester/ModuleAnalyzerNodes.cpp
@@ -586,8 +586,8 @@ static StringRef getKeyContent(SDKContext &Ctx, KeyKind Kind) {
 SDKNode* SDKNode::constructSDKNode(SDKContext &Ctx,
                                    llvm::yaml::MappingNode *Node) {
   static auto GetScalarString = [&](llvm::yaml::Node *N) -> StringRef {
-    SmallString<64> Buffer;
-    return Ctx.buffer(cast<llvm::yaml::ScalarNode>(N)->getValue(Buffer));
+    auto WithQuote = cast<llvm::yaml::ScalarNode>(N)->getRawValue();
+    return WithQuote.substr(1, WithQuote.size() - 2);
   };
 
   static auto getAsInt = [&](llvm::yaml::Node *N) -> int {
@@ -2102,6 +2102,7 @@ static parseJsonEmit(SDKContext &Ctx, StringRef FileName) {
 // previously dumped.
 void SwiftDeclCollector::deSerialize(StringRef Filename) {
   auto Pair = parseJsonEmit(Ctx, Filename);
+  OwnedBuffers.push_back(std::move(Pair.first));
   RootNode = std::move(Pair.second);
 }
 
diff --git a/tools/swift-api-digester/ModuleAnalyzerNodes.h b/tools/swift-api-digester/ModuleAnalyzerNodes.h
@@ -230,6 +230,7 @@ class SDKContext {
     CIs.emplace_back(new CompilerInstance());
     return *CIs.back();
   }
+
   template<class YAMLNodeTy, typename ...ArgTypes>
   void diagnose(YAMLNodeTy node, Diag<ArgTypes...> ID,
                 typename detail::PassArgument<ArgTypes>::type... args) {
@@ -706,6 +707,7 @@ struct TypeInitInfo {
 
 class SwiftDeclCollector: public VisibleDeclConsumer {
   SDKContext &Ctx;
+  std::vector<std::unique_ptr<llvm::MemoryBuffer>> OwnedBuffers;
   SDKNode *RootNode;
   llvm::SetVector<Decl*> KnownDecls;
   // Collected and sorted after we get all of them.
diff --git a/tools/swift-api-digester/swift-api-digester.cpp b/tools/swift-api-digester/swift-api-digester.cpp
@@ -234,11 +234,6 @@ static llvm::cl::list<std::string>
 PreferInterfaceForModules("use-interface-for-module", llvm::cl::ZeroOrMore,
                           llvm::cl::desc("Prefer loading these modules via interface"),
                           llvm::cl::cat(Category));
-
-static llvm::cl::opt<std::string>
-BaselineFilePath("baseline-path",
-                 llvm::cl::desc("The path to the Json file that we should use as the baseline"),
-                 llvm::cl::cat(Category));
 } // namespace options
 
 namespace {
@@ -2534,77 +2529,6 @@ static bool hasBaselineInput() {
     !options::BaselineFrameworkPaths.empty() || !options::BaselineSDK.empty();
 }
 
-enum class ComparisonInputMode: uint8_t {
-  BothJson,
-  BaselineJson,
-  BothLoad,
-};
-
-static ComparisonInputMode checkComparisonInputMode() {
-  if (options::SDKJsonPaths.size() == 2)
-    return ComparisonInputMode::BothJson;
-  else if (hasBaselineInput())
-    return ComparisonInputMode::BothLoad;
-  else
-    return ComparisonInputMode::BaselineJson;
-}
-
-static SDKNodeRoot *getBaselineFromJson(const char *Main, SDKContext &Ctx) {
-  SwiftDeclCollector Collector(Ctx);
-  // If the baseline path has been given, honor that.
-  if (!options::BaselineFilePath.empty()) {
-    Collector.deSerialize(options::BaselineFilePath);
-    return Collector.getSDKRoot();
-  }
-  CompilerInvocation Invok;
-  llvm::StringSet<> Modules;
-  // We need to call prepareForDump to parse target triple.
-  if (prepareForDump(Main, Invok, Modules, true))
-    return nullptr;
-
-  assert(Modules.size() == 1 &&
-         "Cannot find builtin baseline for more than one module");
-  // The path of the swift-api-digester executable.
-  std::string ExePath = llvm::sys::fs::getMainExecutable(Main,
-    reinterpret_cast<void *>(&anchorForGetMainExecutable));
-  llvm::SmallString<128> BaselinePath(ExePath);
-  llvm::sys::path::remove_filename(BaselinePath); // Remove /swift-api-digester
-  llvm::sys::path::remove_filename(BaselinePath); // Remove /bin
-  llvm::sys::path::append(BaselinePath, "lib", "swift", "FrameworkABIBaseline",
-                          Modules.begin()->getKey());
-  // Look for ABI or API baseline
-  if (Ctx.checkingABI())
-    llvm::sys::path::append(BaselinePath, "ABI");
-  else
-    llvm::sys::path::append(BaselinePath, "API");
-  // Look for deployment target specific baseline files.
-  auto Triple = Invok.getLangOptions().Target;
-  if (Triple.isMacCatalystEnvironment())
-    llvm::sys::path::append(BaselinePath, "iosmac.json");
-  else if (Triple.isMacOSX())
-    llvm::sys::path::append(BaselinePath, "macos.json");
-  else if (Triple.isiOS())
-    llvm::sys::path::append(BaselinePath, "iphoneos.json");
-  else if (Triple.isTvOS())
-    llvm::sys::path::append(BaselinePath, "appletvos.json");
-  else if (Triple.isWatchOS())
-    llvm::sys::path::append(BaselinePath, "watchos.json");
-  else {
-    llvm::errs() << "Unsupported triple target\n";
-    exit(1);
-  }
-  StringRef Path = BaselinePath.str();
-  if (!fs::exists(Path)) {
-    llvm::errs() << "Baseline at " << Path << " does not exist\n";
-    exit(1);
-  }
-  if (options::Verbose) {
-    llvm::errs() << "Using baseline at " << Path << "\n";
-  }
-  Collector.deSerialize(Path);
-  return Collector.getSDKRoot();
-}
-
 int main(int argc, char *argv[]) {
   PROGRAM_START(argc, argv);
   INITIALIZE_LLVM();
@@ -2626,40 +2550,33 @@ int main(int argc, char *argv[]) {
       dumpSDKContent(InitInvok, Modules, options::OutputFile, Opts);
   case ActionType::MigratorGen:
   case ActionType::DiagnoseSDKs: {
-    ComparisonInputMode Mode = checkComparisonInputMode();
+    bool CompareJson = options::SDKJsonPaths.size() == 2;
+    if (!CompareJson && !hasBaselineInput()) {
+      llvm::errs() << "Only two SDK versions can be compared\n";
+      llvm::cl::PrintHelpMessage();
+      return 1;
+    }
     llvm::StringSet<> protocolWhitelist;
     if (!options::ProtReqWhiteList.empty()) {
       if (readFileLineByLine(options::ProtReqWhiteList, protocolWhitelist))
           return 1;
     }
-    if (options::Action == ActionType::MigratorGen) {
-      assert(Mode == ComparisonInputMode::BothJson && "Only BothJson mode is supported");
+    if (options::Action == ActionType::MigratorGen)
       return generateMigrationScript(options::SDKJsonPaths[0],
                                      options::SDKJsonPaths[1],
                                      options::OutputFile, IgnoredUsrs, Opts);
-    }
-    switch(Mode) {
-    case ComparisonInputMode::BothJson: {
+    else if (CompareJson)
       return diagnoseModuleChange(options::SDKJsonPaths[0],
                                   options::SDKJsonPaths[1],
                                   options::OutputFile, Opts,
                                   std::move(protocolWhitelist));
-    }
-    case ComparisonInputMode::BaselineJson: {
-      SDKContext Ctx(Opts);
-      return diagnoseModuleChange(Ctx, getBaselineFromJson(argv[0], Ctx),
-                                  getSDKRoot(argv[0], Ctx, false),
-                                  options::OutputFile,
-                                  std::move(protocolWhitelist));
-    }
-    case ComparisonInputMode::BothLoad: {
+    else {
       SDKContext Ctx(Opts);
       return diagnoseModuleChange(Ctx, getSDKRoot(argv[0], Ctx, true),
                                   getSDKRoot(argv[0], Ctx, false),
                                   options::OutputFile,
                                   std::move(protocolWhitelist));
     }
-    }
   }
   case ActionType::DeserializeSDK:
   case ActionType::DeserializeDiffItems: {
