From ceb0f307b3ac11aad61242d75215db1234b4b3b7 Mon Sep 17 00:00:00 2001 From: Igor Laevsky Date: Fri, 27 May 2016 13:13:59 +0000 Subject: [PATCH] [RewriteStatepointsForGC] All constant should have null base pointer Currently we consider that each constant has itself as a base value. I.e "base(const) = const". This introduces couple of problems when we are trying to avoid reporting constants in statepoint live sets: 1. When querying "base( phi(const1, const2) )" we will get "phi(const1, const2)" as a base pointer. Since it's not a constant we will record it in a stack map. However on practice we don't want this to happen (constant are never relocated). 2. base( phi(const, gc ptr) ) = phi( const, base(gc ptr) ). This particular case imposes challenge on our runtime - we don't expect to see constant base pointers other than null. This problems can be avoided by treating all constant as if they were derived from null pointer base. I.e in a first case we will not include constant pointer in a stack map at all. In a second case we will get "phi(null, base(gc ptr))" as a base pointer which is a lot more convenient. Differential Revision: http://reviews.llvm.org/D20584 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@270993 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Transforms/Scalar/RewriteStatepointsForGC.cpp | 24 ++-- .../RewriteStatepointsForGC/base-pointers-12.ll | 2 +- .../RewriteStatepointsForGC/base-pointers-13.ll | 2 +- .../RewriteStatepointsForGC/base-vector.ll | 2 +- .../RewriteStatepointsForGC/constants.ll | 144 +++++++++++++++++++++ 5 files changed, 163 insertions(+), 11 deletions(-) diff --git a/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp b/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp index 991147aeb04..218a52bb3b1 100644 --- a/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp +++ b/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp @@ -343,8 +343,10 @@ findBaseDefiningValueOfVector(Value *I) { return BaseDefiningValueResult(I, true); if (isa(I)) - // Constant vectors consist only of constant pointers. - return BaseDefiningValueResult(I, true); + // Base of constant vector consists only of constant null pointers. + // For reasoning see similar case inside 'findBaseDefiningValue' function. + return BaseDefiningValueResult(ConstantAggregateZero::get(I->getType()), + true); if (isa(I)) return BaseDefiningValueResult(I, true); @@ -386,14 +388,20 @@ static BaseDefiningValueResult findBaseDefiningValue(Value *I) { // We should have never reached here if this argument isn't an gc value return BaseDefiningValueResult(I, true); - if (isa(I)) + if (isa(I)) { // We assume that objects with a constant base (e.g. a global) can't move // and don't need to be reported to the collector because they are always - // live. All constants have constant bases. Besides global references, all - // kinds of constants (e.g. undef, constant expressions, null pointers) can - // be introduced by the inliner or the optimizer, especially on dynamically - // dead paths. See e.g. test4 in constants.ll. - return BaseDefiningValueResult(I, true); + // live. Besides global references, all kinds of constants (e.g. undef, + // constant expressions, null pointers) can be introduced by the inliner or + // the optimizer, especially on dynamically dead paths. + // Here we treat all of them as having single null base. By doing this we + // trying to avoid problems reporting various conflicts in a form of + // "phi (const1, const2)" or "phi (const, regular gc ptr)". + // See constant.ll file for relevant test cases. + + return BaseDefiningValueResult( + ConstantPointerNull::get(cast(I->getType())), true); + } if (CastInst *CI = dyn_cast(I)) { Value *Def = CI->stripPointerCasts(); diff --git a/test/Transforms/RewriteStatepointsForGC/base-pointers-12.ll b/test/Transforms/RewriteStatepointsForGC/base-pointers-12.ll index d2373f3a98d..4706ce70df1 100644 --- a/test/Transforms/RewriteStatepointsForGC/base-pointers-12.ll +++ b/test/Transforms/RewriteStatepointsForGC/base-pointers-12.ll @@ -1,6 +1,6 @@ ; RUN: opt < %s -rewrite-statepoints-for-gc -spp-print-base-pointers -S 2>&1 | FileCheck %s -; CHECK: derived %select base @global +; CHECK: derived %select base null @global = external addrspace(1) global i8 diff --git a/test/Transforms/RewriteStatepointsForGC/base-pointers-13.ll b/test/Transforms/RewriteStatepointsForGC/base-pointers-13.ll index 5d291af29df..d01c771349e 100644 --- a/test/Transforms/RewriteStatepointsForGC/base-pointers-13.ll +++ b/test/Transforms/RewriteStatepointsForGC/base-pointers-13.ll @@ -1,6 +1,6 @@ ; RUN: opt < %s -rewrite-statepoints-for-gc -spp-print-base-pointers -S 2>&1 | FileCheck %s -; CHECK: derived %derived base @global +; CHECK: derived %derived base null @global = external addrspace(1) global i8 diff --git a/test/Transforms/RewriteStatepointsForGC/base-vector.ll b/test/Transforms/RewriteStatepointsForGC/base-vector.ll index 3f507c0a77c..39a78d72597 100644 --- a/test/Transforms/RewriteStatepointsForGC/base-vector.ll +++ b/test/Transforms/RewriteStatepointsForGC/base-vector.ll @@ -106,7 +106,7 @@ entry: define void @test6(i1 %cnd, i64 addrspace(1)* %obj, i64 %idx) gc "statepoint-example" { ; CHECK-LABEL: @test6 ; CHECK: %gep = getelementptr i64, i64 addrspace(1)* %obj, i64 1 -; CHECK: %vec.base = insertelement <2 x i64 addrspace(1)*> undef, i64 addrspace(1)* %obj, i32 0, !is_base_value !0 +; CHECK: %vec.base = insertelement <2 x i64 addrspace(1)*> zeroinitializer, i64 addrspace(1)* %obj, i32 0, !is_base_value !0 ; CHECK: %vec = insertelement <2 x i64 addrspace(1)*> undef, i64 addrspace(1)* %gep, i32 0 ; CHECK: %bdv.base = extractelement <2 x i64 addrspace(1)*> %vec.base, i64 %idx, !is_base_value !0 ; CHECK: %bdv = extractelement <2 x i64 addrspace(1)*> %vec, i64 %idx diff --git a/test/Transforms/RewriteStatepointsForGC/constants.ll b/test/Transforms/RewriteStatepointsForGC/constants.ll index 68d6fd13934..0a16f38f136 100644 --- a/test/Transforms/RewriteStatepointsForGC/constants.ll +++ b/test/Transforms/RewriteStatepointsForGC/constants.ll @@ -117,3 +117,147 @@ entry: call void @foo() [ "deopt"() ] ret i8 addrspace(1)* %load_addr } + +define i8 @test8(i8 addrspace(1)* %p) gc "statepoint-example" { +; Checks that base( phi(gep null, oop) ) = phi(null, base(oop)) and that we +; correctly relocate this value +; CHECK-LABEL: @test8 +entry: + %is_null = icmp eq i8 addrspace(1)* %p, null + br i1 %is_null, label %null.crit-edge, label %not-null + +not-null: + %load_addr = getelementptr inbounds i8, i8 addrspace(1)* %p, i64 8 + br label %join + +null.crit-edge: + %load_addr.const = getelementptr inbounds i8, i8 addrspace(1)* null, i64 8 + br label %join + +join: + %addr = phi i8 addrspace(1)* [ %load_addr, %not-null ], [%load_addr.const, %null.crit-edge] + ; CHECK: %addr.base = phi i8 addrspace(1)* + ; CHECK-DAG: [ %p, %not-null ] + ; CHECK-DAG: [ null, %null.crit-edge ] + ; CHECK: gc.statepoint + call void @foo() [ "deopt"() ] + ; CHECK-DAG: call {{.*}}gc.relocate{{.*}}(%addr.base, %addr.base) + ; CHECK-DAG: call {{.*}}gc.relocate{{.*}}(%addr.base, %addr) + br i1 %is_null, label %early-exit, label %use + +early-exit: + ret i8 0 + +use: + %res = load i8, i8 addrspace(1)* %addr, align 1 + ret i8 %res +} + +define i8 @test9(i8 addrspace(1)* %p) gc "statepoint-example" { +; Checks that base( phi(inttoptr, oop) ) = phi(null, base(oop)) and that we +; correctly relocate this value +; CHECK-LABEL: @test9 +entry: + %is_null = icmp eq i8 addrspace(1)* %p, null + br i1 %is_null, label %null.crit-edge, label %not-null + +not-null: + %load_addr = getelementptr inbounds i8, i8 addrspace(1)* %p, i64 8 + br label %join + +null.crit-edge: + br label %join + +join: + %addr = phi i8 addrspace(1)* [ %load_addr, %not-null ], [inttoptr (i64 8 to i8 addrspace(1)*), %null.crit-edge] + ; CHECK: %addr.base = phi i8 addrspace(1)* + ; CHECK-DAG: [ %p, %not-null ] + ; CHECK-DAG: [ null, %null.crit-edge ] + ; CHECK: gc.statepoint + call void @foo() [ "deopt"() ] + ; CHECK-DAG: call {{.*}}gc.relocate{{.*}}(%addr.base, %addr.base) + ; CHECK-DAG: call {{.*}}gc.relocate{{.*}}(%addr.base, %addr) + br i1 %is_null, label %early-exit, label %use + +early-exit: + ret i8 0 + +use: + %res = load i8, i8 addrspace(1)* %addr, align 1 + ret i8 %res +} + +define i8 @test10(i8 addrspace(1)* %p) gc "statepoint-example" { +; Checks that base( phi(const gep, oop) ) = phi(null, base(oop)) and that we +; correctly relocate this value +; CHECK-LABEL: @test10 +entry: + %is_null = icmp eq i8 addrspace(1)* %p, null + br i1 %is_null, label %null.crit-edge, label %not-null + +not-null: + %load_addr = getelementptr inbounds i8, i8 addrspace(1)* %p, i64 8 + br label %join + +null.crit-edge: + br label %join + +join: + %addr = phi i8 addrspace(1)* [ %load_addr, %not-null ], [getelementptr (i8, i8 addrspace(1)* null, i64 8), %null.crit-edge] + ; CHECK: %addr.base = phi i8 addrspace(1)* + ; CHECK-DAG: [ %p, %not-null ] + ; CHECK-DAG: [ null, %null.crit-edge ] + ; CHECK: gc.statepoint + call void @foo() [ "deopt"() ] + ; CHECK-DAG: call {{.*}}gc.relocate{{.*}}(%addr.base, %addr.base) + ; CHECK-DAG: call {{.*}}gc.relocate{{.*}}(%addr.base, %addr) + br i1 %is_null, label %early-exit, label %use + +early-exit: + ret i8 0 + +use: + %res = load i8, i8 addrspace(1)* %addr, align 1 + ret i8 %res +} + +define i32 addrspace(1)* @test11(i1 %c) gc "statepoint-example" { +; CHECK-LABEL: @test11 +; Checks that base( select(const1, const2) ) == null and that we don't record +; such value in the oop map +entry: + %val = select i1 %c, i32 addrspace(1)* inttoptr (i64 8 to i32 addrspace(1)*), i32 addrspace(1)* inttoptr (i64 15 to i32 addrspace(1)*) + ; CHECK: gc.statepoint + ; CHECK-NOT: call {{.*}}gc.relocate + call void @foo() [ "deopt"() ] + ret i32 addrspace(1)* %val +} + + +define <2 x i32 addrspace(1)*> @test12(i1 %c) gc "statepoint-example" { +; CHECK-LABEL: @test12 +; Same as test11 but with vectors +entry: + %val = select i1 %c, <2 x i32 addrspace(1)*> , + <2 x i32 addrspace(1)*> + ; CHECK: gc.statepoint + ; CHECK-NOT: call {{.*}}gc.relocate + call void @foo() [ "deopt"() ] + ret <2 x i32 addrspace(1)*> %val +} + +define <2 x i32 addrspace(1)*> @test13(i1 %c, <2 x i32 addrspace(1)*> %ptr) gc "statepoint-example" { +; CHECK-LABEL: @test13 +; Similar to test8, test9 and test10 but with vectors +entry: + %val = select i1 %c, <2 x i32 addrspace(1)*> %ptr, + <2 x i32 addrspace(1)*> + ; CHECK: %val.base = select i1 %c, <2 x i32 addrspace(1)*> %ptr, <2 x i32 addrspace(1)*> zeroinitializer, !is_base_value !0 + ; CHECK: gc.statepoint + call void @foo() [ "deopt"() ] + ; CHECK-DAG: call {{.*}}gc.relocate{{.*}}(%val.base, %val.base) + ; CHECK-DAG: call {{.*}}gc.relocate{{.*}}(%val.base, %val) + ret <2 x i32 addrspace(1)*> %val +} -- 2.11.0