Schedule MergeModules Incrementally

CodaFi · CodaFi · commit cecb7c8313be · 2020-10-07T16:57:14.000-07:00
Plumb the logic necessary to schedule merge-modules incrementally. This means that if any inputs jobs to merge-modules run, merge-modules is run. But, if they are all skipped, merge-modules will be skipped as well.

This requires some light special-casing of the legacy driver's incremental job handling because it assumes in a few places it can always extract a swiftdeps file. This invariant will be further broken when the precompile step for bridging headers is skipped as well.

rdar://65893400
diff --git a/lib/Driver/Compilation.cpp b/lib/Driver/Compilation.cpp
@@ -446,7 +446,7 @@ namespace driver {
 
     std::vector<const Job*>
     reloadAndRemarkDeps(const Job *FinishedCmd, int ReturnCode,
-                             const bool forRanges) {
+                        const bool forRanges) {
       const CommandOutput &Output = FinishedCmd->getOutput();
       StringRef DependenciesFile =
           Output.getAdditionalOutputForType(file_types::TY_SwiftDeps);
@@ -458,7 +458,8 @@ namespace driver {
         // coarse dependencies that always affect downstream nodes), but we're
         // not using either of those right now, and this logic should probably
         // be revisited when we are.
-        assert(FinishedCmd->getCondition() == Job::Condition::Always);
+        assert(isa<MergeModuleJobAction>(FinishedCmd->getSource()) ||
+               FinishedCmd->getCondition() == Job::Condition::Always);
         return {};
       }
       const bool compileExitedNormally =
@@ -907,6 +908,7 @@ namespace driver {
         return everyIncrementalJob;
       };
 
+      const Job *mergeModulesJob = nullptr;
       CommandSet jobsToSchedule;
       CommandSet initialCascadingCommands;
       for (const Job *cmd : Comp.getJobs()) {
@@ -915,6 +917,11 @@ namespace driver {
           continue;
         }
 
+        if (isa<MergeModuleJobAction>(cmd->getSource())) {
+          assert(!mergeModulesJob && "multiple scheduled merge-modules jobs?");
+          mergeModulesJob = cmd;
+        }
+
         const Optional<std::pair<bool, bool>> shouldSchedAndIsCascading =
             computeShouldInitiallyScheduleJobAndDependendents(cmd, forRanges);
         if (!shouldSchedAndIsCascading)
@@ -936,6 +943,15 @@ namespace driver {
            collectIncrementalExternallyDependentJobsFromDependencyGraph(
                forRanges))
         jobsToSchedule.insert(cmd);
+
+      // The merge-modules job is special: it *must* be scheduled if any other
+      // job has been scheduled because any other job can influence the
+      // structure of the resulting module. Additionally, the initial scheduling
+      // predicate above is only aware of intra-module changes. External
+      // dependencies changing *must* cause merge-modules to be scheduled.
+      if (!jobsToSchedule.empty() && mergeModulesJob) {
+        jobsToSchedule.insert(mergeModulesJob);
+      }
       return jobsToSchedule;
     }
 
@@ -1031,6 +1047,13 @@ namespace driver {
     /// But returns None if there was a dependency read error.
     Optional<std::pair<Job::Condition, bool>>
     loadDependenciesAndComputeCondition(const Job *const Cmd, bool forRanges) {
+      // merge-modules Jobs do not have .swiftdeps files associated with them,
+      // however, their compilation condition is computed as a function of their
+      // inputs, so their condition can be used as normal.
+      if (isa<MergeModuleJobAction>(Cmd->getSource())) {
+        return std::make_pair(Cmd->getCondition(), true);
+      }
+
       // Try to load the dependencies file for this job. If there isn't one, we
       // always have to run the job, but it doesn't affect any other jobs. If
       // there should be one but it's not present or can't be loaded, we have to
diff --git a/lib/Driver/Driver.cpp b/lib/Driver/Driver.cpp
@@ -2749,24 +2749,30 @@ static void addDiagFileOutputForPersistentPCHAction(
 
 /// If the file at \p input has not been modified since the last build (i.e. its
 /// mtime has not changed), adjust the Job's condition accordingly.
-static void
-handleCompileJobCondition(Job *J, CompileJobAction::InputInfo inputInfo,
-                          StringRef input, bool alwaysRebuildDependents) {
-  if (inputInfo.status == CompileJobAction::InputInfo::NewlyAdded) {
+static void handleCompileJobCondition(Job *J,
+                                      CompileJobAction::InputInfo inputInfo,
+                                      Optional<StringRef> input,
+                                      bool alwaysRebuildDependents) {
   using InputStatus = CompileJobAction::InputInfo::Status;
+
+  if (inputInfo.status == InputStatus::NewlyAdded) {
     J->setCondition(Job::Condition::NewlyAdded);
     return;
   }
 
+  auto output = J->getOutput().getPrimaryOutputFilename();
   bool hasValidModTime = false;
   llvm::sys::fs::file_status inputStatus;
-  if (!llvm::sys::fs::status(input, inputStatus)) {
+  if (input.hasValue() && !llvm::sys::fs::status(*input, inputStatus)) {
+    J->setInputModTime(inputStatus.getLastModificationTime());
+    hasValidModTime = J->getInputModTime() == inputInfo.previousModTime;
+  } else if (!llvm::sys::fs::status(output, inputStatus)) {
     J->setInputModTime(inputStatus.getLastModificationTime());
     hasValidModTime = true;
   }
 
   Job::Condition condition;
-  if (hasValidModTime && J->getInputModTime() == inputInfo.previousModTime) {
+  if (hasValidModTime) {
     switch (inputInfo.status) {
     case InputStatus::UpToDate:
       if (llvm::sys::fs::exists(output))
@@ -2942,14 +2948,18 @@ Job *Driver::buildJobsForAction(Compilation &C, const JobAction *JA,
   Job *J = C.addJob(std::move(ownedJob));
 
   // If we track dependencies for this job, we may be able to avoid running it.
-  if (!J->getOutput()
-           .getAdditionalOutputForType(file_types::TY_SwiftDeps)
-           .empty()) {
-    if (InputActions.size() == 1) {
-      auto compileJob = cast<CompileJobAction>(JA);
-      bool alwaysRebuildDependents =
-          C.getArgs().hasArg(options::OPT_driver_always_rebuild_dependents);
-      handleCompileJobCondition(J, compileJob->getInputInfo(), BaseInput,
+  if (auto incrementalJob = dyn_cast<IncrementalJobAction>(JA)) {
+    const bool alwaysRebuildDependents =
+        C.getArgs().hasArg(options::OPT_driver_always_rebuild_dependents);
+    if (!J->getOutput()
+             .getAdditionalOutputForType(file_types::TY_SwiftDeps)
+             .empty()) {
+      if (InputActions.size() == 1) {
+        handleCompileJobCondition(J, incrementalJob->getInputInfo(), BaseInput,
+                                  alwaysRebuildDependents);
+      }
+    } else if (isa<MergeModuleJobAction>(JA)) {
+      handleCompileJobCondition(J, incrementalJob->getInputInfo(), None,
                                 alwaysRebuildDependents);
     }
   }
diff --git a/test/Driver/Dependencies/one-way-merge-module-fine.swift b/test/Driver/Dependencies/one-way-merge-module-fine.swift
@@ -15,4 +15,4 @@
 
 // CHECK-SECOND-NOT: warning
 // CHECK-SECOND-NOT: Handled
-// CHECK-SECOND: Produced master.swiftmodule
+// CHECK-SECOND-NOT: Produced master.swiftmodule
diff --git a/test/Incremental/CrossModule/linear.swift b/test/Incremental/CrossModule/linear.swift
@@ -38,4 +38,21 @@
 // MODULE-B: Job finished: {merge-module: B.swiftmodule <= B.o}
 
 // MODULE-A: Job skipped: {compile: A.o <= A.swift}
-// MODULE-A: Job finished: {merge-module: A.swiftmodule <= A.o}
+// MODULE-A: Job skipped: {merge-module: A.swiftmodule <= A.o}
+
+//
+// And ensure that the null build really is null.
+//
+
+// RUN: cd %t && %swiftc_driver -c -incremental -emit-dependencies -emit-module -emit-module-path %t/C.swiftmodule -enable-experimental-cross-module-incremental-build -module-name C -I %t -output-file-map %t/C.json -working-directory %t -driver-show-incremental -driver-show-job-lifecycle -DNEW %t/C.swift 2>&1 | %FileCheck -check-prefix MODULE-C-NULL %s
+// RUN: cd %t && %swiftc_driver -c -incremental -emit-dependencies -emit-module -emit-module-path %t/B.swiftmodule -enable-experimental-cross-module-incremental-build -module-name B -I %t -output-file-map %t/B.json -working-directory %t -driver-show-incremental -driver-show-job-lifecycle %t/B.swift 2>&1 | %FileCheck -check-prefix MODULE-B-NULL %s
+// RUN: cd %t && %swiftc_driver -c -incremental -emit-dependencies -emit-module -emit-module-path %t/A.swiftmodule -enable-experimental-cross-module-incremental-build -module-name A -I %t -output-file-map %t/A.json -working-directory %t -driver-show-incremental -driver-show-job-lifecycle %t/A.swift 2>&1 | %FileCheck -check-prefix MODULE-A-NULL %s
+
+// MODULE-C-NULL: Job skipped: {compile: C.o <= C.swift}
+// MODULE-C-NULL: Job skipped: {merge-module: C.swiftmodule <= C.o}
+
+// MODULE-B-NULL: Job skipped: {compile: B.o <= B.swift}
+// MODULE-B-NULL: Job skipped: {merge-module: B.swiftmodule <= B.o}
+
+// MODULE-A-NULL: Job skipped: {compile: A.o <= A.swift}
+// MODULE-A-NULL: Job skipped: {merge-module: A.swiftmodule <= A.o}
diff --git a/test/Incremental/CrossModule/transitive.swift b/test/Incremental/CrossModule/transitive.swift
@@ -42,3 +42,20 @@
 // MODULE-A: Queuing because of incremental external dependencies: {compile: A.o <= A.swift}
 // MODULE-A: Job finished: {compile: A.o <= A.swift}
 // MODULE-A: Job finished: {merge-module: A.swiftmodule <= A.o}
+
+//
+// And ensure that the null build really is null.
+//
+
+// RUN: cd %t && %swiftc_driver -c -incremental -emit-dependencies -emit-module -emit-module-path %t/C.swiftmodule -enable-experimental-cross-module-incremental-build -module-name C -I %t -output-file-map %t/C.json -working-directory %t -driver-show-incremental -driver-show-job-lifecycle -DUSEC -DNEW %t/C.swift 2>&1 | %FileCheck -check-prefix MODULE-C-NULL %s
+// RUN: cd %t && %swiftc_driver -c -incremental -emit-dependencies -emit-module -emit-module-path %t/B.swiftmodule -enable-experimental-cross-module-incremental-build -module-name B -I %t -output-file-map %t/B.json -working-directory %t -driver-show-incremental -driver-show-job-lifecycle -DUSEC %t/B.swift 2>&1 | %FileCheck -check-prefix MODULE-B-NULL %s
+// RUN: cd %t && %swiftc_driver -c -incremental -emit-dependencies -emit-module -emit-module-path %t/A.swiftmodule -enable-experimental-cross-module-incremental-build -module-name A -I %t -output-file-map %t/A.json -working-directory %t -driver-show-incremental -driver-show-job-lifecycle -DUSEC %t/A.swift 2>&1 | %FileCheck -check-prefix MODULE-A-NULL %s
+
+// MODULE-C-NULL: Job skipped: {compile: C.o <= C.swift}
+// MODULE-C-NULL: Job skipped: {merge-module: C.swiftmodule <= C.o}
+
+// MODULE-B-NULL: Job skipped: {compile: B.o <= B.swift}
+// MODULE-B-NULL: Job skipped: {merge-module: B.swiftmodule <= B.o}
+
+// MODULE-A-NULL: Job skipped: {compile: A.o <= A.swift}
+// MODULE-A-NULL: Job skipped: {merge-module: A.swiftmodule <= A.o}
