[llvm] r262095 - [InstCombine] Be more conservative about removing stackrestore

[Reid Kleckner via llvm-commits]

Author: rnk
Date: Fri Feb 26 18:53:54 2016
New Revision: 262095

URL: http://llvm.org/viewvc/llvm-project?rev=262095&view=rev
Log:
[InstCombine] Be more conservative about removing stackrestore

We ended up removing a save/restore pair around an inalloca call,
leading to a miscompile in Chromium.

Modified:
    llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp
    llvm/trunk/test/Transforms/InstCombine/stacksaverestore.ll

Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp?rev=262095&r1=262094&r2=262095&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp Fri Feb 26 18:53:54 2016
@@ -1821,7 +1821,13 @@ Instruction *InstCombiner::visitCallInst
           // If there is a stackrestore below this one, remove this one.
           if (II->getIntrinsicID() == Intrinsic::stackrestore)
             return eraseInstFromFunction(CI);
-          // Otherwise, ignore the intrinsic.
+
+          // Bail if we cross over an intrinsic with side effects, such as
+          // llvm.stacksave, llvm.read_register, or llvm.setjmp.
+          if (II->mayHaveSideEffects()) {
+            CannotRemove = true;
+            break;
+          }
         } else {
           // If we found a non-intrinsic call, we can't remove the stack
           // restore.

Modified: llvm/trunk/test/Transforms/InstCombine/stacksaverestore.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/stacksaverestore.ll?rev=262095&r1=262094&r2=262095&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/stacksaverestore.ll (original)
+++ llvm/trunk/test/Transforms/InstCombine/stacksaverestore.ll Fri Feb 26 18:53:54 2016
@@ -1,4 +1,6 @@
-; RUN: opt < %s -instcombine -S | grep "call.*stackrestore" | count 1
+; RUN: opt < %s -instcombine -S | FileCheck %s
+
+ at glob = global i32 0
 
 declare i8* @llvm.stacksave()
 declare void @llvm.stackrestore(i8*)
@@ -11,11 +13,19 @@ define i32* @test1(i32 %P) {
 	ret i32* %A
 }
 
+; CHECK-LABEL: define i32* @test1(
+; CHECK-NOT: call void @llvm.stackrestore
+; CHECK: ret i32*
+
 define void @test2(i8* %X) {
 	call void @llvm.stackrestore( i8* %X )  ;; no allocas before return.
 	ret void
 }
 
+; CHECK-LABEL: define void @test2(
+; CHECK-NOT: call void @llvm.stackrestore
+; CHECK: ret void
+
 define void @foo(i32 %size) nounwind  {
 entry:
 	%tmp118124 = icmp sgt i32 %size, 0		; <i1> [#uses=1]
@@ -52,5 +62,51 @@ return:		; preds = %bb, %entry
 	ret void
 }
 
+; CHECK-LABEL: define void @foo(
+; CHECK: %tmp = call i8* @llvm.stacksave()
+; CHECK: alloca i8
+; CHECK-NOT: stacksave
+; CHECK: call void @bar(
+; CHECK-NEXT: call void @llvm.stackrestore(i8* %tmp)
+; CHECK: ret void
+
 declare void @bar(i32, i8*, i8*, i8*, i8*, i32)
 
+declare void @inalloca_callee(i32* inalloca)
+
+define void @test3(i32 %c) {
+entry:
+  br label %loop
+
+loop:
+  %i = phi i32 [0, %entry], [%i1, %loop]
+  %save1 = call i8* @llvm.stacksave()
+  %argmem = alloca inalloca i32
+  store i32 0, i32* %argmem
+  call void @inalloca_callee(i32* inalloca %argmem)
+
+  ; This restore cannot be deleted, the restore below does not make it dead.
+  call void @llvm.stackrestore(i8* %save1)
+
+  ; FIXME: We should be able to remove this save/restore pair, but we don't.
+  %save2 = call i8* @llvm.stacksave()
+  store i32 0, i32* @glob
+  call void @llvm.stackrestore(i8* %save2)
+  %i1 = add i32 1, %i
+  %done = icmp eq i32 %i1, %c
+  br i1 %done, label %loop, label %return
+
+return:
+  ret void
+}
+
+; CHECK-LABEL: define void @test3(
+; CHECK: loop:
+; CHECK: %i = phi i32 [ 0, %entry ], [ %i1, %loop ]
+; CHECK: %save1 = call i8* @llvm.stacksave()
+; CHECK: %argmem = alloca inalloca i32
+; CHECK: store i32 0, i32* %argmem
+; CHECK: call void @inalloca_callee(i32* inalloca {{.*}} %argmem)
+; CHECK: call void @llvm.stackrestore(i8* %save1)
+; CHECK: br i1 %done, label %loop, label %return
+; CHECK: ret void