SILCombine: remove a problematic cast peephole optimization.

This optimization inserted bit casts based on structural properties of types.
It caused a miscompile in case of imported C structs. Also, by inspection, I found that checks for resilient types are missing.
As this optimization does not have any noticeable impact on the benchmarks it's better to remove it at all, together with the complexity for checking the types.

rdar://problem/40074362

Author: eeckstein

include/swift/SIL/SILType.h

@@ -367,17 +367,6 @@ class SILType {
   /// pointer.
   bool isPointerSizeAndAligned();
 
-  /// Return true if the layout of `toType` is an ABI compatible prefix of
-  /// `fromType` ignoring reference types. `fromType` may be larger than
-  /// `toType` and still be unsafe castable. `fromType` may contain references
-  /// in positions where `toType` does not contain references and still be
-  /// unsafe castable. This is used solely to determine whether an address cast
-  /// can be promoted to a cast between aggregates of scalar values without
-  /// confusing IRGen.
-  static bool canPerformABICompatibleUnsafeCastValue(SILType fromType,
-                                                     SILType toType,
-                                                     SILModule &M);
-
   /// True if `operTy` can be cast by single-reference value into `resultTy`.
   static bool canRefCast(SILType operTy, SILType resultTy, SILModule &M);
 

lib/SIL/SILType.cpp

@@ -110,193 +110,6 @@ bool SILType::isPointerSizeAndAligned() {
   return false;
 }
 
-// Allow casting a struct by value when all elements in toType correspond to
-// an element of the same size or larger laid out in the same order in
-// fromType. The assumption is that if fromType has larger elements, or
-// additional elements, their presence cannot induce a more compact layout of
-// the overlapping elements.
-//
-// struct {A, B} -> A is castable
-// struct {A, B, C} -> struct {A, B} is castable
-// struct { struct {A, B}, C} -> struct {A, B} is castable
-// struct { A, B, C} -> struct { struct {A, B}, C} is NOT castable
-//
-// FIXME: This is unnecessarily conservative given the current ABI
-// (TypeLayout.rst). It would be simpler to flatten both `from` and `to` types,
-// exploding all structs and tuples, then trivially check if `to` is a prefix.
-static bool canUnsafeCastStruct(SILType fromType, StructDecl *fromStruct,
-                                SILType toType, SILModule &M) {
-  auto fromRange = fromStruct->getStoredProperties();
-  if (fromRange.begin() == fromRange.end())
-    return false;
-
-  // Can the first element of fromStruct be cast by value into toType?
-  SILType fromEltTy = fromType.getFieldType(*fromRange.begin(), M);
-  if (SILType::canPerformABICompatibleUnsafeCastValue(fromEltTy, toType, M))
-    return true;
-  
-  // Otherwise, flatten one level of struct elements on each side.
-  StructDecl *toStruct = toType.getStructOrBoundGenericStruct();
-  if (!toStruct)
-    return false;
-
-  auto toRange = toStruct->getStoredProperties();
-  for (auto toI = toRange.begin(), toE = toRange.end(),
-         fromI = fromRange.begin(), fromE = fromRange.end();
-       toI != toE; ++toI, ++fromI) {
-
-    if (fromI == fromE)
-      return false; // fromType is a struct with fewer elements.
-      
-    SILType fromEltTy = fromType.getFieldType(*fromI, M);
-    SILType toEltTy = toType.getFieldType(*toI, M);
-    if (!SILType::canPerformABICompatibleUnsafeCastValue(fromEltTy, toEltTy, M))
-      return false;
-  }
-  // fromType's overlapping elements are compatible.
-  return true;
-}
-
-// Allow casting a tuple by value when all elements in toType correspond to an
-// element of the same size or larger in fromType in the same order.
-static bool canUnsafeCastTuple(SILType fromType, CanTupleType fromTupleTy,
-                               SILType toType, SILModule &M) {
-  unsigned numFromElts = fromTupleTy->getNumElements();
-  // Can the first element of fromTupleTy be cast by value into toType?
-  if (numFromElts != 0
-      && SILType::canPerformABICompatibleUnsafeCastValue(
-             fromType.getTupleElementType(0), toType, M)) {
-    return true;
-  }
-  // Otherwise, flatten one level of tuple elements on each side.
-  auto toTupleTy = dyn_cast<TupleType>(toType.getSwiftRValueType());
-  if (!toTupleTy)
-    return false;
-
-  unsigned numToElts = toTupleTy->getNumElements();
-  if (numFromElts < numToElts)
-    return false;
-
-  for (unsigned i = 0; i != numToElts; ++i) {
-    if (!SILType::canPerformABICompatibleUnsafeCastValue(
-            fromType.getTupleElementType(i), toType.getTupleElementType(i),
-            M)) {
-      return false;
-    }
-  }
-  return true;
-}
-
-// Allow casting an enum by value when toType is an enum and each elements is
-// individually castable to toType. An enum cannot be smaller than its payload.
-static bool canUnsafeCastEnum(SILType fromType, EnumDecl *fromEnum,
-                              SILType toType, SILModule &M) {
-  unsigned numToElements = 0;
-  SILType toElementTy;
-  if (EnumDecl *toEnum = toType.getEnumOrBoundGenericEnum()) {
-    for (auto toElement : toEnum->getAllElements()) {
-      ++numToElements;
-      if (!toElement->hasAssociatedValues())
-        continue;
-      // Bail on multiple payloads.
-      if (!toElementTy.isNull())
-        return false;
-      toElementTy = toType.getEnumElementType(toElement, M);
-    }
-  } else {
-    // If toType is not an enum, handle it like a singleton
-    numToElements = 1;
-    toElementTy = toType;
-  }
-  // If toType has more elements, it may be larger.
-  auto fromElements = fromEnum->getAllElements();
-  if (static_cast<ptrdiff_t>(numToElements) >
-      std::distance(fromElements.begin(), fromElements.end()))
-    return false;
-
-  if (toElementTy.isNull())
-    return true;
-
-  // If any of the fromElements can be cast by value to the singleton toElement,
-  // then the overall enum can be cast by value.
-  for (auto fromElement : fromElements) {
-    if (!fromElement->hasAssociatedValues())
-      continue;
-
-    auto fromElementTy = fromType.getEnumElementType(fromElement, M);
-    if (SILType::canPerformABICompatibleUnsafeCastValue(fromElementTy,
-                                                        toElementTy, M))
-      return true;
-  }
-  return false;
-}
-
-static bool canUnsafeCastScalars(SILType fromType, SILType toType,
-                                 SILModule &M) {
-  CanType fromCanTy = fromType.getSwiftRValueType();
-  bool isToPointer = toType.isPointerSizeAndAligned();
-
-  unsigned LeastFromWidth = 0;
-  // Like UnsafeRefBitCast, allow class existentials to be truncated to
-  // single-pointer references. Unlike UnsafeRefBitCast, this also supports raw
-  // pointers and words.
-  if (fromType.isPointerSizeAndAligned()
-      || fromCanTy.isAnyClassReferenceType()) {
-
-    // Allow casting from a value that contains an aligned pointer into another
-    // pointer value regardless of the fixed width.
-    if (isToPointer)
-      return true;
-
-    LeastFromWidth = BuiltinIntegerWidth::pointer().getLeastWidth();
-
-  } else if (auto fromIntTy = dyn_cast<BuiltinIntegerType>(fromCanTy)) {
-    if (fromIntTy->isFixedWidth())
-      LeastFromWidth = fromIntTy->getFixedWidth();
-  }
-
-  unsigned GreatestToWidth = UINT_MAX;
-  if (isToPointer) {
-    GreatestToWidth = BuiltinIntegerWidth::pointer().getGreatestWidth();
-
-  } else if (auto toIntTy = dyn_cast<BuiltinIntegerType>(
-               toType.getSwiftRValueType())) {
-    if (toIntTy->isFixedWidth())
-      GreatestToWidth = toIntTy->getFixedWidth();
-  }
-  return LeastFromWidth >= GreatestToWidth;
-}
-
-bool SILType::canPerformABICompatibleUnsafeCastValue(SILType fromType,
-                                                     SILType toType,
-                                                     SILModule &M) {
-  if (fromType == toType)
-    return true;
-
-  // Unwrap single element structs.
-  if (StructDecl *toStruct = toType.getStructOrBoundGenericStruct()) {
-    auto toRange = toStruct->getStoredProperties();
-    if (toRange.begin() != toRange.end()
-        && std::next(toRange.begin()) == toRange.end()) {
-      toType = toType.getFieldType(*toRange.begin(), M);
-    }
-  }
-  if (canUnsafeCastScalars(fromType, toType, M))
-    return true;
-
-  if (StructDecl *fromStruct = fromType.getStructOrBoundGenericStruct())
-    return canUnsafeCastStruct(fromType, fromStruct, toType, M);
-
-  if (CanTupleType fromTupleTy =
-      dyn_cast<TupleType>(fromType.getSwiftRValueType())) {
-    return canUnsafeCastTuple(fromType, fromTupleTy, toType, M);
-  }
-  if (EnumDecl *fromEnum = fromType.getEnumOrBoundGenericEnum())
-    return canUnsafeCastEnum(fromType, fromEnum, toType, M);
-  
-  return false;
-}
-
 // Reference cast from representations with single pointer low bits.
 // Only reference cast to simple single pointer representations.
 //

lib/SILOptimizer/SILCombiner/SILCombinerCastVisitors.cpp

@@ -208,68 +208,7 @@ SILCombiner::visitUncheckedAddrCastInst(UncheckedAddrCastInst *UADCI) {
     return Builder.createUpcast(UADCI->getLoc(), UADCI->getOperand(),
                                 UADCI->getType());
 
-  // See if we have all loads from this unchecked_addr_cast. If we do, load the
-  // original type and create the appropriate bitcast.
-
-  // First if our UADCI has not users, bail. This will be eliminated by DCE.
-  if (UADCI->use_empty())
-    return nullptr;
-
-  SILType InputTy = UADCI->getOperand()->getType();
-  SILType OutputTy = UADCI->getType();
-
-  // If either type is address only, do not do anything here.
-  if (InputTy.isAddressOnly(Mod) || OutputTy.isAddressOnly(Mod))
-    return nullptr;
-
-  bool InputIsTrivial = InputTy.isTrivial(Mod);
-  bool OutputIsTrivial = OutputTy.isTrivial(Mod);
-
-  // If our input is trivial and our output type is not, do not do
-  // anything. This is to ensure that we do not change any types reference
-  // semantics from trivial -> reference counted.
-  if (InputIsTrivial && !OutputIsTrivial)
-    return nullptr;
-
-  // Check that the input type can be value cast to the output type.  It is
-  // possible to cast the address of a smaller InputType to the address of a
-  // larger OutputType (the actual memory object must be large enough to hold
-  // both types). However, such address casts cannot be converted to value
-  // casts.
-  if (!SILType::canPerformABICompatibleUnsafeCastValue(InputTy, OutputTy,
-                                                       UADCI->getModule())) {
-    return nullptr;
-  }
-  // For each user U of the unchecked_addr_cast...
-  for (auto U : getNonDebugUses(UADCI))
-    // Check if it is load. If it is not a load, bail...
-    if (!isa<LoadInst>(U->getUser()))
-      return nullptr;
-
-  SILValue Op = UADCI->getOperand();
-  SILLocation Loc = UADCI->getLoc();
-
-  // Ok, we have all loads. Lets simplify this. Go back through the loads a
-  // second time, rewriting them into a load + bitcast from our source.
-  auto UsesRange = getNonDebugUses(UADCI);
-  for (auto UI = UsesRange.begin(), E = UsesRange.end(); UI != E;) {
-    // Grab the original load.
-    LoadInst *L = cast<LoadInst>(UI->getUser());
-    UI++;
-
-    // Insert a new load from our source and bitcast that as appropriate.
-    LoadInst *NewLoad =
-        Builder.createLoad(Loc, Op, LoadOwnershipQualifier::Unqualified);
-    auto *BitCast = Builder.createUncheckedBitCast(Loc, NewLoad,
-                                                    OutputTy.getObjectType());
-    // Replace all uses of the old load with the new bitcasted result and erase
-    // the old load.
-    replaceInstUsesWith(*L, BitCast);
-    eraseInstFromFunction(*L);
-  }
-
-  // Delete the old cast.
-  return eraseInstFromFunction(*UADCI);
+  return nullptr;
 }
 
 SILInstruction *