SILOptimizer: fix a phase ordering problem, which prevented array optimizations to work in some cases

Introduce an "early redundant load elimination", which does not optimize loads from arrays.
Later array optimizations, like ABCOpt, get confused if an array load in a loop is converted to a pattern with a phi argument.

This problem was introduced with accessors.

rdar://problem/44184763

Author: eeckstein

include/swift/SILOptimizer/PassManager/Passes.def

@@ -154,6 +154,8 @@ PASS(ARCSequenceOpts, "arc-sequence-opts",
      "ARC Sequence Optimization")
 PASS(ARCLoopOpts, "arc-loop-opts",
      "ARC Loop Optimization")
+PASS(EarlyRedundantLoadElimination, "early-redundant-load-elim",
+     "Early Redundant Load Elimination")
 PASS(RedundantLoadElimination, "redundant-load-elim",
      "Redundant Load Elimination")
 PASS(DeadStoreElimination, "dead-store-elim",

lib/SILOptimizer/PassManager/PassPipeline.cpp

@@ -305,7 +305,14 @@ void addSSAPasses(SILPassPipelinePlan &P, OptimizationLevelKind OpLevel) {
   P.addSimplifyCFG();
 
   P.addCSE();
-  P.addRedundantLoadElimination();
+  if (OpLevel == OptimizationLevelKind::HighLevel) {
+    // Early RLE does not touch loads from Arrays. This is important because
+    // later array optimizations, like ABCOpt, get confused if an array load in
+    // a loop is converted to a pattern with a phi argument.
+    P.addEarlyRedundantLoadElimination();
+  } else {
+    P.addRedundantLoadElimination();
+  }
 
   P.addPerformanceConstantPropagation();
   P.addCSE();

lib/SILOptimizer/Transforms/RedundantLoadElimination.cpp

@@ -481,14 +481,17 @@ class RLEContext {
   /// walked, i.e. when the we generate the genset and killset.
   llvm::DenseSet<SILBasicBlock *> BBWithLoads;
 
+  /// If set, RLE ignores loads from that array type.
+  NominalTypeDecl *ArrayType;
+
 #ifndef NDEBUG
   SILPrintContext printCtx;
 #endif
 
 public:
   RLEContext(SILFunction *F, SILPassManager *PM, AliasAnalysis *AA,
              TypeExpansionAnalysis *TE, PostOrderFunctionInfo *PO,
-             EpilogueARCFunctionInfo *EAFI);
+             EpilogueARCFunctionInfo *EAFI, bool disableArrayLoads);
 
   RLEContext(const RLEContext &) = delete;
   RLEContext(RLEContext &&) = default;
@@ -555,6 +558,17 @@ class RLEContext {
   /// Transitively collect all the values that make up this location and
   /// create a SILArgument out of them.
   SILValue computePredecessorLocationValue(SILBasicBlock *BB, LSLocation &L);
+
+  /// Returns the LoadInst if \p Inst is a load inst we want to handle.
+  LoadInst *isLoadInstToHandle(SILInstruction *Inst) {
+    if (auto *LI = dyn_cast<LoadInst>(Inst)) {
+      if (!ArrayType ||
+          LI->getType().getNominalOrBoundGenericNominal() != ArrayType) {
+        return LI;
+      }
+    }
+    return nullptr;
+  }
 };
 
 } // end anonymous namespace
@@ -1054,7 +1068,7 @@ void BlockState::processInstructionWithKind(RLEContext &Ctx,
 
   // This is a LoadInst. Let's see if we can find a previous loaded, stored
   // value to use instead of this load.
-  if (auto *LI = dyn_cast<LoadInst>(Inst)) {
+  if (auto *LI = Ctx.isLoadInstToHandle(Inst)) {
     processLoadInst(Ctx, LI, Kind);
     return;
   }
@@ -1174,8 +1188,10 @@ void BlockState::dump(RLEContext &Ctx) {
 
 RLEContext::RLEContext(SILFunction *F, SILPassManager *PM, AliasAnalysis *AA,
                        TypeExpansionAnalysis *TE, PostOrderFunctionInfo *PO,
-                       EpilogueARCFunctionInfo *EAFI)
-    : Fn(F), PM(PM), AA(AA), TE(TE), PO(PO), EAFI(EAFI)
+                       EpilogueARCFunctionInfo *EAFI, bool disableArrayLoads)
+    : Fn(F), PM(PM), AA(AA), TE(TE), PO(PO), EAFI(EAFI),
+      ArrayType(disableArrayLoads ?
+                F->getModule().getASTContext().getArrayDecl() : nullptr)
 #ifndef NDEBUG
       ,
       printCtx(llvm::dbgs(), /*Verbose=*/false, /*Sorted=*/true)
@@ -1612,6 +1628,14 @@ namespace {
 
 class RedundantLoadElimination : public SILFunctionTransform {
 
+private:
+  bool disableArrayLoads;
+
+public:
+
+  RedundantLoadElimination(bool disableArrayLoads)
+    : disableArrayLoads(disableArrayLoads) { }
+
   /// The entry point to the transformation.
   void run() override {
     SILFunction *F = getFunction();
@@ -1623,7 +1647,7 @@ class RedundantLoadElimination : public SILFunctionTransform {
     auto *PO = PM->getAnalysis<PostOrderAnalysis>()->get(F);
     auto *EAFI = PM->getAnalysis<EpilogueARCAnalysis>()->get(F);
 
-    RLEContext RLE(F, PM, AA, TE, PO, EAFI);
+    RLEContext RLE(F, PM, AA, TE, PO, EAFI, disableArrayLoads);
     if (RLE.run()) {
       invalidateAnalysis(SILAnalysis::InvalidationKind::Instructions);
     }
@@ -1632,6 +1656,10 @@ class RedundantLoadElimination : public SILFunctionTransform {
 
 } // end anonymous namespace
 
+SILTransform *swift::createEarlyRedundantLoadElimination() {
+  return new RedundantLoadElimination(/*disableArrayLoads=*/true);
+}
+
 SILTransform *swift::createRedundantLoadElimination() {
-  return new RedundantLoadElimination();
+  return new RedundantLoadElimination(/*disableArrayLoads=*/false);
 }

test/SILOptimizer/early-rle.sil

@@ -0,0 +1,36 @@
+// RUN: %target-sil-opt -assume-parsing-unqualified-ownership-sil -enable-sil-verify-all %s -early-redundant-load-elim | %FileCheck %s
+
+sil_stage canonical
+
+import Builtin
+import Swift
+import SwiftShims
+
+
+// Check that earlyl redundant load elimination ignores arrays.
+
+
+// CHECK-LABEL: test_array
+// CHECK: %1 = load
+// CHECK: %2 = load
+// CHECK: %3 = tuple (%1 {{.*}}, %2 {{.*}})
+sil @test_array : $@convention(thin) (@inout Array<Int>) -> (Array<Int>, Array<Int>) {
+bb0(%0 : $*Array<Int>):
+  %1 = load %0 : $*Array<Int>
+  %2 = load %0 : $*Array<Int>
+  %3 = tuple (%1: $Array<Int>, %2: $Array<Int>)
+  return %3 : $(Array<Int>, Array<Int>)
+}
+
+// CHECK-LABEL: test_non_array
+// CHECK: %1 = load
+// CHECK: %2 = tuple (%1 {{.*}}, %1 {{.*}})
+sil @test_non_array : $@convention(thin) (@inout Int) -> (Int, Int) {
+bb0(%0 : $*Int):
+  %1 = load %0 : $*Int
+  %2 = load %0 : $*Int
+  %3 = tuple (%1: $Int, %2: $Int)
+  return %3 : $(Int, Int)
+}
+
+