Change Projection to support signed indices.

atrick · atrick · commit 2767b51d615d · 2020-10-16T15:00:09.000-07:00
Change ProjectionIndex for ref_tail_addr to std::numeric_limits&lt;int&gt;::max();
This is necessary to disambiguate the tail elements from
ref_element_addr field zero.
diff --git a/include/swift/SIL/Projection.h b/include/swift/SIL/Projection.h
@@ -67,9 +67,9 @@ inline bool isStrictSubSeqRelation(SubSeqRelation_t Seq) {
 
 /// Extract an integer index from a SILValue.
 ///
-/// Return true if IndexVal is a constant index representable as unsigned
+/// Return true if IndexVal is a constant index representable as an
 /// int. We do not support symbolic projections yet.
-bool getIntegerIndex(SILValue IndexVal, unsigned &IndexConst);
+bool getIntegerIndex(SILValue IndexVal, int &IndexConst);
 
 /// The kind of projection that we are representing.
 ///
@@ -136,11 +136,18 @@ static inline bool isCastProjectionKind(ProjectionKind Kind) {
 /// that immediately contains it.
 ///
 /// This lightweight utility maps a SIL address projection to an index.
+///
+/// project_box does not have a projection index. At the SIL level, the box
+/// storage is considered part of the same object as the. The box projection is
+/// does not affect access path so that box projections can occur on distinct
+/// phi paths in the address def-use chain.
 struct ProjectionIndex {
+  static constexpr int TailIndex = std::numeric_limits<int>::max();
+
   SILValue Aggregate;
-  unsigned Index;
+  int Index = std::numeric_limits<int>::min();
 
-  explicit ProjectionIndex(SILValue V) : Index(~0U) {
+  explicit ProjectionIndex(SILValue V) {
     switch (V->getKind()) {
     default:
       break;
@@ -163,16 +170,9 @@ struct ProjectionIndex {
       break;
     }
     case ValueKind::RefTailAddrInst: {
-      RefTailAddrInst *REA = cast<RefTailAddrInst>(V);
-      Index = 0;
-      Aggregate = REA->getOperand();
-      break;
-    }
-    case ValueKind::ProjectBoxInst: {
-      ProjectBoxInst *PBI = cast<ProjectBoxInst>(V);
-      // A box has only a single payload.
-      Index = 0;
-      Aggregate = PBI->getOperand();
+      RefTailAddrInst *RTA = cast<RefTailAddrInst>(V);
+      Index = TailIndex;
+      Aggregate = RTA->getOperand();
       break;
     }
     case ValueKind::TupleElementAddrInst: {
@@ -233,8 +233,7 @@ class Projection {
       : Projection(dyn_cast<SingleValueInstruction>(I)) {}
   explicit Projection(SingleValueInstruction *I);
 
-  Projection(ProjectionKind Kind, unsigned NewIndex)
-      : Value(Kind, NewIndex) {}
+  Projection(ProjectionKind Kind, int NewIndex) : Value(Kind, NewIndex) {}
 
   Projection(ProjectionKind Kind, TypeBase *Ptr)
       : Value(Kind, Ptr) {}
@@ -252,10 +251,8 @@ class Projection {
 
   /// Convenience method for getting the underlying index. Assumes that this
   /// projection is valid. Otherwise it asserts.
-  unsigned getIndex() const {
-    return Value.getIndex();
-  }
-  
+  int getIndex() const { return Value.getIndex(); }
+
   unsigned getHash() const { return (unsigned)Value.getStorage(); }
 
   /// Determine if I is a value projection instruction whose corresponding
@@ -359,7 +356,7 @@ class Projection {
       return nullptr;
     case ValueKind::IndexAddrInst: {
       auto *i = cast<IndexAddrInst>(v);
-      unsigned scalar;
+      int scalar;
       if (getIntegerIndex(i->getIndex(), scalar))
         return i;
       return nullptr;
diff --git a/lib/SIL/Utils/Projection.cpp b/lib/SIL/Utils/Projection.cpp
@@ -12,10 +12,11 @@
 
 #define DEBUG_TYPE "sil-projection"
 #include "swift/SIL/Projection.h"
+#include "swift/Basic/IndexTrie.h"
 #include "swift/Basic/NullablePtr.h"
-#include "swift/SIL/SILBuilder.h"
-#include "swift/SIL/InstructionUtils.h"
 #include "swift/SIL/DebugUtils.h"
+#include "swift/SIL/InstructionUtils.h"
+#include "swift/SIL/SILBuilder.h"
 #include "swift/SIL/SILUndef.h"
 #include "llvm/ADT/None.h"
 #include "llvm/Support/Debug.h"
@@ -42,22 +43,27 @@ static_assert(std::is_standard_layout<Projection>::value,
 /// Return true if IndexVal is a constant index representable as unsigned
 /// int. We do not support symbolic projections yet, only 32-bit unsigned
 /// integers.
-bool swift::getIntegerIndex(SILValue IndexVal, unsigned &IndexConst) {
-  if (auto *IndexLiteral = dyn_cast<IntegerLiteralInst>(IndexVal)) {
-    APInt ConstInt = IndexLiteral->getValue();
-    // IntegerLiterals are signed.
-    if (ConstInt.isIntN(32) && ConstInt.isNonNegative()) {
-      IndexConst = (unsigned)ConstInt.getSExtValue();
-      return true;
-    }    
-  }
-  return false;
+bool swift::getIntegerIndex(SILValue IndexVal, int &IndexConst) {
+  auto *IndexLiteral = dyn_cast<IntegerLiteralInst>(IndexVal);
+  if (!IndexLiteral)
+    return false;
+
+  APInt ConstInt = IndexLiteral->getValue();
+  // Reserve 1 bit for encoding. See AccessPath::Index.
+  if (!ConstInt.isSignedIntN(31))
+    return false;
+
+  IndexConst = ConstInt.getSExtValue();
+  assert(((IndexConst << 1) >> 1) == IndexConst);
+  return true;
 }
 
 //===----------------------------------------------------------------------===//
 //                               Projection
 //===----------------------------------------------------------------------===//
 
+constexpr int ProjectionIndex::TailIndex;
+
 Projection::Projection(SingleValueInstruction *I) : Value() {
   if (!I)
     return;
@@ -72,21 +78,21 @@ Projection::Projection(SingleValueInstruction *I) : Value() {
     auto *SEAI = cast<StructElementAddrInst>(I);
     Value = ValueTy(ProjectionKind::Struct, SEAI->getFieldIndex());
     assert(getKind() == ProjectionKind::Struct);
-    assert(getIndex() == SEAI->getFieldIndex());
+    assert(getIndex() == int(SEAI->getFieldIndex()));
     break;
   }
   case SILInstructionKind::StructExtractInst: {
     auto *SEI = cast<StructExtractInst>(I);
     Value = ValueTy(ProjectionKind::Struct, SEI->getFieldIndex());
     assert(getKind() == ProjectionKind::Struct);
-    assert(getIndex() == SEI->getFieldIndex());
+    assert(getIndex() == int(SEI->getFieldIndex()));
     break;
   }
   case SILInstructionKind::RefElementAddrInst: {
     auto *REAI = cast<RefElementAddrInst>(I);
     Value = ValueTy(ProjectionKind::Class, REAI->getFieldIndex());
     assert(getKind() == ProjectionKind::Class);
-    assert(getIndex() == REAI->getFieldIndex());
+    assert(getIndex() == int(REAI->getFieldIndex()));
     break;
   }
   case SILInstructionKind::RefTailAddrInst: {
@@ -108,28 +114,28 @@ Projection::Projection(SingleValueInstruction *I) : Value() {
     auto *TEI = cast<TupleExtractInst>(I);
     Value = ValueTy(ProjectionKind::Tuple, TEI->getFieldIndex());
     assert(getKind() == ProjectionKind::Tuple);
-    assert(getIndex() == TEI->getFieldIndex());
+    assert(getIndex() == int(TEI->getFieldIndex()));
     break;
   }
   case SILInstructionKind::TupleElementAddrInst: {
     auto *TEAI = cast<TupleElementAddrInst>(I);
     Value = ValueTy(ProjectionKind::Tuple, TEAI->getFieldIndex());
     assert(getKind() == ProjectionKind::Tuple);
-    assert(getIndex() == TEAI->getFieldIndex());
+    assert(getIndex() == int(TEAI->getFieldIndex()));
     break;
   }
   case SILInstructionKind::UncheckedEnumDataInst: {
     auto *UEDI = cast<UncheckedEnumDataInst>(I);
     Value = ValueTy(ProjectionKind::Enum, UEDI->getElementNo());
     assert(getKind() == ProjectionKind::Enum);
-    assert(getIndex() == UEDI->getElementNo());
+    assert(getIndex() == int(UEDI->getElementNo()));
     break;
   }
   case SILInstructionKind::UncheckedTakeEnumDataAddrInst: {
     auto *UTEDAI = cast<UncheckedTakeEnumDataAddrInst>(I);
     Value = ValueTy(ProjectionKind::Enum, UTEDAI->getElementNo());
     assert(getKind() == ProjectionKind::Enum);
-    assert(getIndex() == UTEDAI->getElementNo());
+    assert(getIndex() == int(UTEDAI->getElementNo()));
     break;
   }
   case SILInstructionKind::IndexAddrInst: {
@@ -138,9 +144,10 @@ Projection::Projection(SingleValueInstruction *I) : Value() {
     // updated and a MaxLargeIndex will need to be used here. Currently we
     // represent large Indexes using a 64 bit integer, so we don't need to mess
     // with anything.
-    unsigned NewIndex = 0;
+    int NewIndex = 0;
     auto *IAI = cast<IndexAddrInst>(I);
-    if (getIntegerIndex(IAI->getIndex(), NewIndex)) {
+    // TODO: handle negative indices
+    if (getIntegerIndex(IAI->getIndex(), NewIndex) && NewIndex >= 0) {
       Value = ValueTy(ProjectionKind::Index, NewIndex);
       assert(getKind() == ProjectionKind::Index);
       assert(getIndex() == NewIndex);
@@ -321,10 +328,6 @@ void Projection::getFirstLevelProjections(
 
   if (auto *C = Ty.getClassOrBoundGenericClass()) {
     unsigned Count = 0;
-    for (auto *superDecl = C->getSuperclassDecl(); superDecl != nullptr;
-         superDecl = superDecl->getSuperclassDecl()) {
-      Count += superDecl->getStoredProperties().size();
-    }
     for (auto *VDecl : C->getStoredProperties()) {
       (void) VDecl;
       Projection P(ProjectionKind::Class, Count++);
diff --git a/lib/SILOptimizer/LoopTransforms/ArrayOpt.h b/lib/SILOptimizer/LoopTransforms/ArrayOpt.h
@@ -63,7 +63,7 @@ class StructUseCollector {
   /// Do not form a path with an IndexAddrInst because we have no way to
   /// distinguish between indexing and subelement access. The same index could
   /// either refer to the next element (indexed) or a subelement.
-  static SILValue getAccessPath(SILValue V, SmallVectorImpl<unsigned>& Path) {
+  static SILValue getAccessPath(SILValue V, SmallVectorImpl<int> &Path) {
     V = stripCasts(V);
     if (auto *IA = dyn_cast<IndexAddrInst>(V)) {
       // Don't include index_addr projections in the access path. We could if
@@ -89,7 +89,7 @@ class StructUseCollector {
   VisitedSet Visited;
 
   /// Collect all uses of the value at the given address.
-  void collectUses(ValueBase *V, ArrayRef<unsigned> AccessPath) {
+  void collectUses(ValueBase *V, ArrayRef<int> AccessPath) {
     // Save our old indent and increment.
     // Collect all users of the address and loads.
     collectAddressUses(V, AccessPath, nullptr);
@@ -142,7 +142,7 @@ class StructUseCollector {
   /// StructVal is invalid, then the value is the address of the Struct. If
   /// StructVal is valid, the value is the address of an element within the
   /// Struct.
-  void collectAddressUses(ValueBase *V, ArrayRef<unsigned> AccessPathSuffix,
+  void collectAddressUses(ValueBase *V, ArrayRef<int> AccessPathSuffix,
                           Operand *StructVal) {
     for (auto *UI : V->getUses()) {
       // Keep the operand, not the instruction in the visited set. The same
diff --git a/lib/SILOptimizer/LoopTransforms/ArrayPropertyOpt.cpp b/lib/SILOptimizer/LoopTransforms/ArrayPropertyOpt.cpp
@@ -402,7 +402,7 @@ class ArrayPropertiesAnalysis {
     if (!Call.canHoist(Preheader->getTerminator(), DomTree))
       return false;
 
-    SmallVector<unsigned, 4> AccessPath;
+    SmallVector<int, 4> AccessPath;
     SILValue ArrayContainer =
       StructUseCollector::getAccessPath(Arr, AccessPath);
 
diff --git a/lib/SILOptimizer/LoopTransforms/COWArrayOpt.cpp b/lib/SILOptimizer/LoopTransforms/COWArrayOpt.cpp
@@ -843,7 +843,7 @@ bool COWArrayOpt::hoistMakeMutable(ArraySemanticsCall MakeMutable,
     return false;
   }
 
-  SmallVector<unsigned, 4> AccessPath;
+  SmallVector<int, 4> AccessPath;
   SILValue ArrayContainer =
     StructUseCollector::getAccessPath(CurrentArrayAddr, AccessPath);
   bool arrayContainerIsUnique = checkUniqueArrayContainer(ArrayContainer);

Original file line number	Diff line number	Diff line change
`@@ -843,7 +843,7 @@ bool COWArrayOpt::hoistMakeMutable(ArraySemanticsCall MakeMutable,`
`843`	`843`	`return false;`
`844`	`844`	`}`
`845`	`845`
`846`		`- SmallVector<unsigned, 4> AccessPath;`
	`846`	`+ SmallVector<int, 4> AccessPath;`
`847`	`847`	`SILValue ArrayContainer =`
`848`	`848`	`StructUseCollector::getAccessPath(CurrentArrayAddr, AccessPath);`
`849`	`849`	`bool arrayContainerIsUnique = checkUniqueArrayContainer(ArrayContainer);`