[llvm-commits] [llvm] r156442 - in /llvm/trunk: lib/Transforms/Scalar/ObjCARC.cpp test/Transforms/ObjCARC/contract-storestrong.ll

[Dan Gohman]

Author: djg
Date: Tue May  8 18:34:08 2012
New Revision: 156442

URL: http://llvm.org/viewvc/llvm-project?rev=156442&view=rev
Log:
Fix objc_storeStrong pattern matching to catch a potential use of the
old value after the store but before it is released.
This fixes rdar:/11116986.

Modified:
    llvm/trunk/lib/Transforms/Scalar/ObjCARC.cpp
    llvm/trunk/test/Transforms/ObjCARC/contract-storestrong.ll

Modified: llvm/trunk/lib/Transforms/Scalar/ObjCARC.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/ObjCARC.cpp?rev=156442&r1=156441&r2=156442&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/ObjCARC.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/ObjCARC.cpp Tue May  8 18:34:08 2012
@@ -3925,18 +3925,38 @@
   BasicBlock *BB = Release->getParent();
   if (Load->getParent() != BB) return;
 
-  // Walk down to find the store.
+  // Walk down to find the store and the release, which may be in either order.
   BasicBlock::iterator I = Load, End = BB->end();
   ++I;
   AliasAnalysis::Location Loc = AA->getLocation(Load);
-  while (I != End &&
-         (&*I == Release ||
-          IsRetain(GetBasicInstructionClass(I)) ||
-          !(AA->getModRefInfo(I, Loc) & AliasAnalysis::Mod)))
-    ++I;
-  StoreInst *Store = dyn_cast<StoreInst>(I);
-  if (!Store || !Store->isSimple()) return;
-  if (Store->getPointerOperand() != Loc.Ptr) return;
+  StoreInst *Store = 0;
+  bool SawRelease = false;
+  for (; !Store || !SawRelease; ++I) {
+    Instruction *Inst = I;
+    if (Inst == Release) {
+      SawRelease = true;
+      continue;
+    }
+
+    InstructionClass Class = GetBasicInstructionClass(Inst);
+
+    // Unrelated retains are harmless.
+    if (IsRetain(Class))
+      continue;
+
+    if (Store) {
+      // The store is the point where we're going to put the objc_storeStrong,
+      // so make sure there are no uses after it.
+      if (CanUse(Inst, Load, PA, Class))
+        return;
+    } else if (AA->getModRefInfo(Inst, Loc) & AliasAnalysis::Mod) {
+      // We are moving the load down to the store, so check for anything
+      // else which writes to the memory between the load and the store.
+      Store = dyn_cast<StoreInst>(Inst);
+      if (!Store || !Store->isSimple()) return;
+      if (Store->getPointerOperand() != Loc.Ptr) return;
+    }
+  }
 
   Value *New = StripPointerCastsAndObjCCalls(Store->getValueOperand());
 

Modified: llvm/trunk/test/Transforms/ObjCARC/contract-storestrong.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/ObjCARC/contract-storestrong.ll?rev=156442&r1=156441&r2=156442&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/ObjCARC/contract-storestrong.ll (original)
+++ llvm/trunk/test/Transforms/ObjCARC/contract-storestrong.ll Tue May  8 18:34:08 2012
@@ -4,6 +4,7 @@
 
 declare i8* @objc_retain(i8*)
 declare void @objc_release(i8*)
+declare void @use_pointer(i8*)
 
 @x = external global i8*
 
@@ -57,3 +58,78 @@
   tail call void @objc_release(i8* %tmp) nounwind
   ret void
 }
+
+; Don't do this if there's a use of the old pointer vlaue between the store
+; and the release.
+
+; CHECK:      define void @test3(i8* %newValue) {
+; CHECK-NEXT: entry:
+; CHECK-NEXT:   %x0 = tail call i8* @objc_retain(i8* %newValue) nounwind
+; CHECK-NEXT:   %x1 = load i8** @x, align 8
+; CHECK-NEXT:   store i8* %x0, i8** @x, align 8
+; CHECK-NEXT:   tail call void @use_pointer(i8* %x1), !clang.arc.no_objc_arc_exceptions !0
+; CHECK-NEXT:   tail call void @objc_release(i8* %x1) nounwind, !clang.imprecise_release !0
+; CHECK-NEXT:   ret void
+; CHECK-NEXT: }
+define void @test3(i8* %newValue) {
+entry:
+  %x0 = tail call i8* @objc_retain(i8* %newValue) nounwind
+  %x1 = load i8** @x, align 8
+  store i8* %newValue, i8** @x, align 8
+  tail call void @use_pointer(i8* %x1), !clang.arc.no_objc_arc_exceptions !0
+  tail call void @objc_release(i8* %x1) nounwind, !clang.imprecise_release !0
+  ret void
+}
+
+; Like test3, but with an icmp use instead of a call, for good measure.
+
+; CHECK:      define i1 @test4(i8* %newValue, i8* %foo) {
+; CHECK-NEXT: entry:
+; CHECK-NEXT:   %x0 = tail call i8* @objc_retain(i8* %newValue) nounwind
+; CHECK-NEXT:   %x1 = load i8** @x, align 8
+; CHECK-NEXT:   store i8* %x0, i8** @x, align 8
+; CHECK-NEXT:   %t = icmp eq i8* %x1, %foo
+; CHECK-NEXT:   tail call void @objc_release(i8* %x1) nounwind, !clang.imprecise_release !0
+; CHECK-NEXT:   ret i1 %t
+; CHECK-NEXT: }
+define i1 @test4(i8* %newValue, i8* %foo) {
+entry:
+  %x0 = tail call i8* @objc_retain(i8* %newValue) nounwind
+  %x1 = load i8** @x, align 8
+  store i8* %newValue, i8** @x, align 8
+  %t = icmp eq i8* %x1, %foo
+  tail call void @objc_release(i8* %x1) nounwind, !clang.imprecise_release !0
+  ret i1 %t
+}
+
+; Do form an objc_storeStrong here, because the use is before the store.
+
+; CHECK: define i1 @test5(i8* %newValue, i8* %foo) {
+; CHECK: %t = icmp eq i8* %x1, %foo
+; CHECK: tail call void @objc_storeStrong(i8** @x, i8* %newValue) nounwind
+define i1 @test5(i8* %newValue, i8* %foo) {
+entry:
+  %x0 = tail call i8* @objc_retain(i8* %newValue) nounwind
+  %x1 = load i8** @x, align 8
+  %t = icmp eq i8* %x1, %foo
+  store i8* %newValue, i8** @x, align 8
+  tail call void @objc_release(i8* %x1) nounwind, !clang.imprecise_release !0
+  ret i1 %t
+}
+
+; Like test5, but the release is before the store.
+
+; CHECK: define i1 @test6(i8* %newValue, i8* %foo) {
+; CHECK: %t = icmp eq i8* %x1, %foo
+; CHECK: tail call void @objc_storeStrong(i8** @x, i8* %newValue) nounwind
+define i1 @test6(i8* %newValue, i8* %foo) {
+entry:
+  %x0 = tail call i8* @objc_retain(i8* %newValue) nounwind
+  %x1 = load i8** @x, align 8
+  tail call void @objc_release(i8* %x1) nounwind, !clang.imprecise_release !0
+  %t = icmp eq i8* %x1, %foo
+  store i8* %newValue, i8** @x, align 8
+  ret i1 %t
+}
+
+!0 = metadata !{}