[ObjC][ARC] Don't form a StoreStrong call if it is unsafe to move the

release call

findSafeStoreForStoreStrongContraction checks whether it's safe to move
the release call to the store by inspecting all instructions between the
two, but was ignoring retain instructions. This was causing objects to
be released and deallocated before they were retained.

rdar://81668577

Author: ahatanaka

llvm/lib/Transforms/ObjCARC/ObjCARCContract.cpp

@@ -226,13 +226,6 @@ static StoreInst *findSafeStoreForStoreStrongContraction(LoadInst *Load,
     // of Inst.
     ARCInstKind Class = GetBasicARCInstKind(Inst);
 
-    // If Inst is an unrelated retain, we don't care about it.
-    //
-    // TODO: This is one area where the optimization could be made more
-    // aggressive.
-    if (IsRetain(Class))
-      continue;
-
     // If we have seen the store, but not the release...
     if (Store) {
       // We need to make sure that it is safe to move the release from its
@@ -248,8 +241,18 @@ static StoreInst *findSafeStoreForStoreStrongContraction(LoadInst *Load,
       return nullptr;
     }
 
-    // Ok, now we know we have not seen a store yet. See if Inst can write to
-    // our load location, if it can not, just ignore the instruction.
+    // Ok, now we know we have not seen a store yet.
+
+    // If Inst is a retain, we don't care about it as it doesn't prevent moving
+    // the load to the store.
+    //
+    // TODO: This is one area where the optimization could be made more
+    // aggressive.
+    if (IsRetain(Class))
+      continue;
+
+    // See if Inst can write to our load location, if it can not, just ignore
+    // the instruction.
     if (!isModSet(AA->getModRefInfo(Inst, Loc)))
       continue;
 

llvm/test/Transforms/ObjCARC/contract-storestrong.ll

@@ -256,6 +256,25 @@ define i8* @test13(i8* %a0, i8* %a1, i8** %addr, i8* %new) {
   ret i8* %retained
 }
 
+; Cannot form a storeStrong call because it's unsafe to move the release call to
+; the store.
+
+; CHECK-LABEL: define void @test14(
+; CHECK: %[[V0:.*]] = load i8*, i8** %a
+; CHECK: %[[V1:.*]] = call i8* @llvm.objc.retain(i8* %p)
+; CHECK: store i8* %[[V1]], i8** %a
+; CHECK: %[[V2:.*]] = call i8* @llvm.objc.retain(i8* %[[V0]])
+; CHECK: call void @llvm.objc.release(i8* %[[V2]])
+
+define void @test14(i8** %a, i8* %p) {
+  %v0 = load i8*, i8** %a, align 8
+  %v1 = call i8* @llvm.objc.retain(i8* %p)
+  store i8* %p, i8** %a, align 8
+  %v2  = call i8* @llvm.objc.retain(i8* %v0)
+  call void @llvm.objc.release(i8* %v0)
+  ret void
+}
+
 !0 = !{}
 
 ; CHECK: attributes [[NUW]] = { nounwind }