[region-isolation] Make load/load_borrow look through instructions.

This ensures that when we process, we consider load/load_borrow's result to be
equivalent to its operand. This ensures that a load/load_borrow cannot act as a
use of its operand preventing spurious diagnostics.

(cherry picked from commit eed51e7)

Author: gottesmm

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

@@ -371,14 +371,23 @@ static SILValue getUnderlyingTrackedObjectValue(SILValue value) {
 
 static UnderlyingTrackedValueInfo getUnderlyingTrackedValue(SILValue value) {
   if (!value->getType().isAddress()) {
-    return UnderlyingTrackedValueInfo(getUnderlyingTrackedObjectValue(value));
+    SILValue underlyingValue = getUnderlyingTrackedObjectValue(value);
+
+    if (!isa<LoadInst, LoadBorrowInst>(underlyingValue)) {
+      return UnderlyingTrackedValueInfo(underlyingValue);
+    }
+
+    // If we got an address, lets see if we can do even better by looking at the
+    // address.
+    value = cast<SingleValueInstruction>(underlyingValue)->getOperand(0);
   }
+  assert(value->getType().isAddress());
 
   UseDefChainVisitor visitor;
   SILValue base = visitor.visitAll(value);
   assert(base);
   if (base->getType().isObject())
-    return {getUnderlyingObject(base), visitor.actorIsolation};
+    return {getUnderlyingTrackedValue(base).value, visitor.actorIsolation};
   return {base, visitor.actorIsolation};
 }
 
@@ -2393,8 +2402,6 @@ CONSTANT_TRANSLATION(ObjCExistentialMetatypeToObjectInst, AssignFresh)
 // example, a cast would be inappropriate here. This is implemented by
 // propagating the operand's region into the result's region and by
 // requiring all operands.
-CONSTANT_TRANSLATION(LoadInst, Assign)
-CONSTANT_TRANSLATION(LoadBorrowInst, Assign)
 CONSTANT_TRANSLATION(LoadWeakInst, Assign)
 CONSTANT_TRANSLATION(StrongCopyUnownedValueInst, Assign)
 CONSTANT_TRANSLATION(ClassMethodInst, Assign)
@@ -2736,6 +2743,23 @@ PartitionOpTranslator::visitBeginBorrowInst(BeginBorrowInst *bbi) {
   return TranslationSemantics::LookThrough;
 }
 
+/// LoadInst is technically a statically look through instruction, but we want
+/// to handle it especially in the infrastructure, so we cannot mark it as
+/// such. This makes marking it as a normal lookthrough instruction impossible
+/// since the routine checks that invariant.
+TranslationSemantics PartitionOpTranslator::visitLoadInst(LoadInst *limvi) {
+  return TranslationSemantics::Special;
+}
+
+/// LoadBorrowInst is technically a statically look through instruction, but we
+/// want to handle it especially in the infrastructure, so we cannot mark it as
+/// such. This makes marking it as a normal lookthrough instruction impossible
+/// since the routine checks that invariant.
+TranslationSemantics
+PartitionOpTranslator::visitLoadBorrowInst(LoadBorrowInst *lbi) {
+  return TranslationSemantics::Special;
+}
+
 TranslationSemantics PartitionOpTranslator::visitReturnInst(ReturnInst *ri) {
   if (ri->getFunction()->getLoweredFunctionType()->hasTransferringResult()) {
     return TranslationSemantics::TransferringNoResult;

test/ClangImporter/transferring.swift

@@ -27,12 +27,14 @@ func funcTestTransferringResult() async {
   // Just to show that without the transferring param, we generate diagnostics.
   let x2 = NonSendableCStruct()
   let y2 = returnUserDefinedFromGlobalFunction(x2)
-  await transferToMain(x2) // expected-error {{transferring value of non-Sendable type 'NonSendableCStruct' from nonisolated context to main actor-isolated context}}
+  await transferToMain(x2) // expected-error {{transferring 'x2' may cause a data race}}
+  // expected-note @-1 {{transferring disconnected 'x2' to main actor-isolated callee could cause races in between callee main actor-isolated and local nonisolated uses}}
   useValue(y2) // expected-note {{use here could race}}
 }
 
 func funcTestTransferringArg() async {
   let x = NonSendableCStruct()
-  transferUserDefinedIntoGlobalFunction(x) // expected-error {{value of non-Sendable type 'NonSendableCStruct' accessed after being transferred; later accesses could race}}
+  transferUserDefinedIntoGlobalFunction(x) // expected-error {{transferring 'x' may cause a data race}}
+  // expected-note @-1 {{'x' used after being passed as a transferring parameter}}
   useValue(x) // expected-note {{use here could race}}
 }

test/Concurrency/transfernonsendable_instruction_matching.sil

@@ -24,6 +24,7 @@ class NonSendableKlass {}
 sil @transferNonSendableKlass : $@convention(thin) @async (@guaranteed NonSendableKlass) -> ()
 sil @useNonSendableKlass : $@convention(thin) (@guaranteed NonSendableKlass) -> ()
 sil @constructNonSendableKlass : $@convention(thin) () -> @owned NonSendableKlass
+sil @constructNonSendableKlassIndirect : $@convention(thin) () -> @out NonSendableKlass
 sil @useUnmanagedNonSendableKlass : $@convention(thin) (@guaranteed @sil_unmanaged NonSendableKlass) -> ()
 
 final class SendableKlass : Sendable {}
@@ -1678,6 +1679,47 @@ bb0(%arg : $Builtin.Word):
   return %9999 : $()
 }
 
+sil [ossa] @load_is_lookthrough_test : $@convention(thin) @async () -> () {
+bb0:
+  %0 = alloc_stack $NonSendableKlass
+  %indirectConstruct = function_ref @constructNonSendableKlassIndirect : $@convention(thin) () -> @out NonSendableKlass
+  apply %indirectConstruct(%0) : $@convention(thin) () -> @out NonSendableKlass
+
+  %1 = load_borrow %0 : $*NonSendableKlass
+  %transferNonSendableKlass = function_ref @transferNonSendableKlass : $@convention(thin) @async (@guaranteed NonSendableKlass) -> ()
+  apply [caller_isolation=nonisolated] [callee_isolation=global_actor] %transferNonSendableKlass(%1) : $@convention(thin) @async (@guaranteed NonSendableKlass) -> ()
+  end_borrow %1 : $NonSendableKlass
+
+  %2 = load [copy] %0 : $*NonSendableKlass
+  destroy_value %2 : $NonSendableKlass
+  %3 = load [take] %0 : $*NonSendableKlass
+  destroy_value %3 : $NonSendableKlass
+  dealloc_stack %0 : $*NonSendableKlass
+
+  %9999 = tuple ()
+  return %9999 : $()
+}
+
+sil [ossa] @loadborrow_is_lookthrough_test : $@convention(thin) @async () -> () {
+bb0:
+  %0 = alloc_stack $NonSendableKlass
+  %indirectConstruct = function_ref @constructNonSendableKlassIndirect : $@convention(thin) () -> @out NonSendableKlass
+  apply %indirectConstruct(%0) : $@convention(thin) () -> @out NonSendableKlass
+
+  %1 = load_borrow %0 : $*NonSendableKlass
+  %transferNonSendableKlass = function_ref @transferNonSendableKlass : $@convention(thin) @async (@guaranteed NonSendableKlass) -> ()
+  apply [caller_isolation=nonisolated] [callee_isolation=global_actor] %transferNonSendableKlass(%1) : $@convention(thin) @async (@guaranteed NonSendableKlass) -> ()
+  end_borrow %1 : $NonSendableKlass
+
+  %2 = load_borrow %0 : $*NonSendableKlass
+  end_borrow %2 : $NonSendableKlass
+  destroy_addr %0 : $*NonSendableKlass
+  dealloc_stack %0 : $*NonSendableKlass
+
+  %9999 = tuple ()
+  return %9999 : $()
+}
+
 sil hidden [ossa] @test_unconditional_checked_cast : $@convention(method) (@guaranteed ChildClass) -> @owned ChildClass {
 bb0(%0 : @guaranteed $ChildClass):
   debug_value %0 : $ChildClass, let, name "self", argno 1

test/Concurrency/transfernonsendable_strong_transferring_params.swift

@@ -185,14 +185,35 @@ func canTransferAssigningIntoLocal(_ x: transferring Klass) async {
 }
 
 func canTransferAssigningIntoLocal2(_ x: transferring Klass) async {
+  let _ = x
+  await transferToMain(x)
+  // We do not error here since we just load the value and do not do anything
+  // with it.
+  //
+  // TODO: We should change let _ = x so that it has a move_value '_' or
+  // something like that. It will also help move checking as well.
+  let _ = x
+}
 
+func canTransferAssigningIntoLocal2a(_ x: transferring Klass) async {
+  let _ = x
+  await transferToMain(x)
+  // We do not error here since we just load the value and do not do anything
+  // with it.
+  //
+  // TODO: We should change let _ = x so that it has a move_value '_' or
+  // something like that. It will also help move checking as well.
+  _ = x
+}
+
+func canTransferAssigningIntoLocal3(_ x: transferring Klass) async {
   let _ = x
   await transferToMain(x) // expected-warning {{transferring 'x' may cause a data race}}
   // expected-note @-1 {{transferring disconnected 'x' to main actor-isolated callee could cause races in between callee main actor-isolated and local nonisolated uses}}
-  let _ = x // expected-note {{use here could race}}
+  let y = x // expected-note {{use here could race}}
+  _ = y
 }
 
-
 //////////////////////////////////////
 // MARK: Transferring is "var" like //
 //////////////////////////////////////