Skip to content

Commit

Permalink
[RS4GC] Handle uses of extractelement for conversion from vector to s…
Browse files Browse the repository at this point in the history 
…calar base

As mentioned in the comments, extractelement is special
since we actually want a scalar base for that element we extracted from
the vector (i.e. not a vector base).
This same logic should apply to uses of the extractelement such as phis
and selects which have the same BDV as the extractelement.
Howeber, for these uses we conservatively mark the BDV state as
conflict, since setting the EE's new base BDV does not always dominate
these uses.

Added testcase showcases the problem where the BDV identification chokes
on the incorrect cast from vector to scalar for the phi use of
extractelement.

Tests-Run: make check, internal fuzzer testing

Reviewers: reames, skatkov, dantrushin
Reviewed-By: dantrushin

Differential Revision: https://reviews.llvm.org/D75704
  • Loading branch information
@annamthomas
annamthomas committed Mar 6, 2020
1 parent eb755df commit 59029b9
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 12 deletions.
46 changes: 34 additions & 12 deletions llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -926,8 +926,7 @@ static Value *findBasePointer(Value *I, DefiningValueMapTy &Cache) {
}
#endif

// Insert Phis for all conflicts
// TODO: adjust naming patterns to avoid this order of iteration dependency
// Handle extractelement instructions and their uses.
for (auto Pair : States) {
Instruction *I = cast<Instruction>(Pair.first);
BDVState State = Pair.second;
Expand All @@ -938,17 +937,40 @@ static Value *findBasePointer(Value *I, DefiningValueMapTy &Cache) {
// insert an extract even when we know an exact base for the instruction.
// The problem is that we need to convert from a vector base to a scalar
// base for the particular indice we're interested in.
if (State.isBase() && isa<ExtractElementInst>(I) &&
isa<VectorType>(State.getBaseValue()->getType())) {
auto *EE = cast<ExtractElementInst>(I);
// TODO: In many cases, the new instruction is just EE itself. We should
// exploit this, but can't do it here since it would break the invariant
// about the BDV not being known to be a base.
auto *BaseInst = ExtractElementInst::Create(
State.getBaseValue(), EE->getIndexOperand(), "base_ee", EE);
BaseInst->setMetadata("is_base_value", MDNode::get(I->getContext(), {}));
States[I] = BDVState(BDVState::Base, BaseInst);
if (!State.isBase() || !isa<ExtractElementInst>(I) ||
!isa<VectorType>(State.getBaseValue()->getType()))
continue;
auto *EE = cast<ExtractElementInst>(I);
// TODO: In many cases, the new instruction is just EE itself. We should
// exploit this, but can't do it here since it would break the invariant
// about the BDV not being known to be a base.
auto *BaseInst = ExtractElementInst::Create(
State.getBaseValue(), EE->getIndexOperand(), "base_ee", EE);
BaseInst->setMetadata("is_base_value", MDNode::get(I->getContext(), {}));
States[I] = BDVState(BDVState::Base, BaseInst);

// We need to handle uses of the extractelement that have the same vector
// base as well but the use is a scalar type. Since we cannot reuse the
// same BaseInst above (may not satisfy property that base pointer should
// always dominate derived pointer), we conservatively set this as conflict.
// Setting the base value for these conflicts is handled in the next loop
// which traverses States.
for (User *U : I->users()) {
auto *UseI = dyn_cast<Instruction>(U);
if (!UseI || !States.count(UseI))
continue;
if (!isa<VectorType>(UseI->getType()) && States[UseI] == State)
States[UseI] = BDVState(BDVState::Conflict);
}
}

// Insert Phis for all conflicts
// TODO: adjust naming patterns to avoid this order of iteration dependency
for (auto Pair : States) {
Instruction *I = cast<Instruction>(Pair.first);
BDVState State = Pair.second;
assert(!isKnownBaseResult(I) && "why did it get added?");
assert(!State.isUnknown() && "Optimistic algorithm didn't complete!");

// Since we're joining a vector and scalar base, they can never be the
// same. As a result, we should always see insert element having reached
Expand Down
53 changes: 53 additions & 0 deletions llvm/test/Transforms/RewriteStatepointsForGC/scalar-base-vector.ll
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,3 +141,56 @@ entry:
%ptr = extractelement <2 x i32 addrspace(1)*> %vec, i32 0
ret i32 addrspace(1)* %ptr
}

define void @test6() gc "statepoint-example" {
; CHECK-LABEL: @test6(
; CHECK-NEXT: bb:
; CHECK-NEXT: br label [[HEADER:%.*]]
; CHECK: header:
; CHECK-NEXT: [[TMP_BASE:%.*]] = phi i8 addrspace(1)* [ [[TMP6_BASE:%.*]], [[LATCH:%.*]] ], [ null, [[BB:%.*]] ], !is_base_value !0
; CHECK-NEXT: [[TMP:%.*]] = phi i8 addrspace(1)* [ [[TMP6:%.*]], [[LATCH]] ], [ undef, [[BB]] ]
; CHECK-NEXT: br label [[BB10:%.*]]
; CHECK: bb10:
; CHECK-NEXT: [[STATEPOINT_TOKEN:%.*]] = call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 2882400000, i32 0, void ()* @spam, i32 0, i32 0, i32 0, i32 1, i8 addrspace(1)* [[TMP]], i8 addrspace(1)* [[TMP]], i8 addrspace(1)* [[TMP_BASE]])
; CHECK-NEXT: [[TMP_RELOCATED:%.*]] = call coldcc i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token [[STATEPOINT_TOKEN]], i32 9, i32 8)
; CHECK-NEXT: [[TMP_BASE_RELOCATED:%.*]] = call coldcc i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token [[STATEPOINT_TOKEN]], i32 9, i32 9)
; CHECK-NEXT: br label [[BB25:%.*]]
; CHECK: bb25:
; CHECK-NEXT: [[STATEPOINT_TOKEN1:%.*]] = call token (i64, i32, <2 x i8 addrspace(1)*> ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_v2p1i8f(i64 2882400000, i32 0, <2 x i8 addrspace(1)*> ()* @baz, i32 0, i32 0, i32 0, i32 0)
; CHECK-NEXT: [[TMP262:%.*]] = call <2 x i8 addrspace(1)*> @llvm.experimental.gc.result.v2p1i8(token [[STATEPOINT_TOKEN1]])
; CHECK-NEXT: [[BASE_EE:%.*]] = extractelement <2 x i8 addrspace(1)*> [[TMP262]], i32 0, !is_base_value !0
; CHECK-NEXT: [[TMP27:%.*]] = extractelement <2 x i8 addrspace(1)*> [[TMP262]], i32 0
; CHECK-NEXT: br i1 undef, label [[BB7:%.*]], label [[LATCH]]
; CHECK: bb7:
; CHECK-NEXT: br label [[LATCH]]
; CHECK: latch:
; CHECK-NEXT: [[TMP6_BASE]] = phi i8 addrspace(1)* [ [[BASE_EE]], [[BB25]] ], [ [[BASE_EE]], [[BB7]] ], !is_base_value !0
; CHECK-NEXT: [[TMP6]] = phi i8 addrspace(1)* [ [[TMP27]], [[BB25]] ], [ [[TMP27]], [[BB7]] ]
; CHECK-NEXT: br label [[HEADER]]
;
bb:
br label %header

header: ; preds = %latch, %bb
%tmp = phi i8 addrspace(1)* [ %tmp6, %latch ], [ undef, %bb ]
br label %bb10

bb10: ; preds = %bb2
call void @spam() [ "deopt"(i8 addrspace(1)* %tmp) ]
br label %bb25

bb25: ; preds = %bb24
%tmp26 = call <2 x i8 addrspace(1)*> @baz()
%tmp27 = extractelement <2 x i8 addrspace(1)*> %tmp26, i32 0
br i1 undef, label %bb7, label %latch

bb7: ; preds = %bb25
br label %latch

latch: ; preds = %bb25, %bb7
%tmp6 = phi i8 addrspace(1)* [ %tmp27, %bb25 ], [ %tmp27, %bb7 ]
br label %header
}

declare void @spam()
declare <2 x i8 addrspace(1)*> @baz()

0 comments on commit 59029b9

Please sign in to comment.