This is an archive of the discontinued LLVM Phabricator instance.

Differential D131146

[RS4GC] Handle vectors of pointers in non-live clobbering
ClosedPublic

Authored by danilaml on Aug 4 2022, 12:10 AM.

Details

Reviewers
reames
mkazantsev
apilipenko
skatkov
Commits
rG451497a030d7: [RS4GC] Handle vectors of pointers in non-live clobbering
Summary

Fix crash when trying to unconditionally cast alloca type to PointerType

Diff Detail

Event Timeline

danilaml created this revision.Aug 4 2022, 12:10 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 4 2022, 12:10 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
danilaml requested review of this revision.Aug 4 2022, 12:10 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 4 2022, 12:10 AM
Herald added a subscriber: llvm-commits. · View Herald Transcript
danilaml added inline comments.Aug 4 2022, 12:14 AM
llvm/test/Transforms/RewriteStatepointsForGC/vector-nonlive-clobber.ll
4

will fix typo before landing

danilaml added reviewers: reames, mkazantsev, apilipenko, skatkov.Aug 4 2022, 12:15 AM
Harbormaster completed remote builds in B179211: Diff 449889.Aug 4 2022, 1:11 AM
apilipenko accepted this revision.Aug 15 2022, 10:45 AM
apilipenko added inline comments.
llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
2086–2090
This revision is now accepted and ready to land.Aug 15 2022, 10:45 AM
This revision was landed with ongoing or failed builds.Aug 16 2022, 7:47 AM
Closed by commit rG451497a030d7: [RS4GC] Handle vectors of pointers in non-live clobbering (authored by danilaml). · Explain Why
This revision was automatically updated to reflect the committed changes.
danilaml added a commit: rG451497a030d7: [RS4GC] Handle vectors of pointers in non-live clobbering.
danilaml marked an inline comment as done.Aug 16 2022, 7:50 AM
danilaml added inline comments.
llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
2086–2090

Ternary operator is not able to implicitly cast ConstantAggregateZero and ConstantPointerNull to the common base type Constant, so this version would need explicit static_casts at which point I feel like the if wins out in terseness/readability.

Revision Contents

PathSize
llvm/
lib/
Transforms/
Scalar/
8 lines
test/
Transforms/
RewriteStatepointsForGC/
44 lines

Diff 453006

llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp

Show First 20 Lines • Show All 2,076 Lines • ▼ Show 20 Lines if (ClobberNonLive) {
if (VisitedLiveValues.count(Def)) { if (VisitedLiveValues.count(Def)) {
continue; continue;
} }
ToClobber.push_back(Alloca); ToClobber.push_back(Alloca);
} }
auto InsertClobbersAt = [&](Instruction *IP) { auto InsertClobbersAt = [&](Instruction *IP) {
for (auto *AI : ToClobber) { for (auto *AI : ToClobber) {
auto PT = cast<PointerType>(AI->getAllocatedType()); auto AT = AI->getAllocatedType();
Constant *CPN = ConstantPointerNull::get(PT); Constant *CPN;
if (AT->isVectorTy())
CPN = ConstantAggregateZero::get(AT);
else
CPN = ConstantPointerNull::get(cast<PointerType>(AT));
apilipenkoUnsubmitted
Not Done
ReplyInline Actions
auto PT = AI->getAllocatedType();
- Constant *CPN;
- if (PT->isVectorTy())
- CPN = ConstantAggregateZero::get(PT);
- else
- CPN = ConstantPointerNull::get(cast<PointerType>(PT));
+ Constant *CPN = PT->isVectorTy() ?
+ ConstantAggregateZero::get(PT) :
+ ConstantPointerNull::get(cast<PointerType>(PT));
new StoreInst(CPN, AI, IP);
apilipenko:
danilamlAuthorUnsubmitted
Done
ReplyInline Actions

Ternary operator is not able to implicitly cast ConstantAggregateZero and ConstantPointerNull to the common base type Constant, so this version would need explicit static_casts at which point I feel like the if wins out in terseness/readability.

danilaml: Ternary operator is not able to implicitly cast `ConstantAggregateZero` and…
new StoreInst(CPN, AI, IP); new StoreInst(CPN, AI, IP);
} }
}; };
// Insert the clobbering stores. These may get intermixed with the // Insert the clobbering stores. These may get intermixed with the
// gc.results and gc.relocates, but that's fine. // gc.results and gc.relocates, but that's fine.
if (auto II = dyn_cast<InvokeInst>(Statepoint)) { if (auto II = dyn_cast<InvokeInst>(Statepoint)) {
InsertClobbersAt(&*II->getNormalDest()->getFirstInsertionPt()); InsertClobbersAt(&*II->getNormalDest()->getFirstInsertionPt());
▲ Show 20 Lines • Show All 1,179 Lines • Show Last 20 Lines

llvm/test/Transforms/RewriteStatepointsForGC/vector-nonlive-clobber.ll

  • This file was added.
PropertyOld ValueNew Value
File Modenull100755
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
; RUN: opt -S -passes=rewrite-statepoints-for-gc -rs4gc-clobber-non-live %s | FileCheck %s
; Make sure that clobber-non-live correctly handles vector types
danilamlAuthorUnsubmitted
Done
ReplyInline Actions

will fix typo before landing

danilaml: will fix typo before landing
define void @test_vector_clobber(i8 addrspace(1)* %ptr) gc "statepoint-example" {
; CHECK-LABEL: @test_vector_clobber(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[PTR_CAST:%.*]] = bitcast i8 addrspace(1)* [[PTR:%.*]] to float addrspace(1)*
; CHECK-NEXT: [[STATEPOINT_TOKEN:%.*]] = call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 2882400000, i32 0, void ()* elementtype(void ()) @foo, i32 0, i32 0, i32 0, i32 0) [ "deopt"(i32 0, i32 2, i32 0, i32 62, i32 0, i32 13, i32 0, i32 7, i8* null, i32 7, i8* null, i32 7, i8* null, i32 3, i32 14, i32 3, i32 -2406, i32 3, i32 28963, i32 3, i32 30401, i32 3, i32 -11, i32 3, i32 -5, i32 3, i32 1, i32 0, i8 addrspace(1)* [[PTR]], i32 0, i8 addrspace(1)* [[PTR]], i32 7, i8* null), "gc-live"(i8 addrspace(1)* [[PTR]]) ]
; CHECK-NEXT: [[PTR_RELOCATED:%.*]] = call coldcc i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token [[STATEPOINT_TOKEN]], i32 0, i32 0)
; CHECK-NEXT: [[PTR_CAST_REMAT:%.*]] = bitcast i8 addrspace(1)* [[PTR_RELOCATED]] to float addrspace(1)*
; CHECK-NEXT: [[CAST:%.*]] = bitcast i8 addrspace(1)* [[PTR_RELOCATED]] to float addrspace(1)*
; CHECK-NEXT: [[DOTSPLATINSERT_BASE:%.*]] = insertelement <8 x float addrspace(1)*> zeroinitializer, float addrspace(1)* [[CAST]], i32 0, !is_base_value !0
; CHECK-NEXT: [[DOTSPLATINSERT:%.*]] = insertelement <8 x float addrspace(1)*> poison, float addrspace(1)* [[PTR_CAST_REMAT]], i32 0
; CHECK-NEXT: [[DOTSPLAT_BASE:%.*]] = shufflevector <8 x float addrspace(1)*> [[DOTSPLATINSERT_BASE]], <8 x float addrspace(1)*> undef, <8 x i32> zeroinitializer, !is_base_value !0
; CHECK-NEXT: [[DOTSPLAT:%.*]] = shufflevector <8 x float addrspace(1)*> [[DOTSPLATINSERT]], <8 x float addrspace(1)*> poison, <8 x i32> zeroinitializer
; CHECK-NEXT: [[GEP:%.*]] = getelementptr inbounds float, <8 x float addrspace(1)*> [[DOTSPLAT]], <8 x i64> <i64 83, i64 81, i64 79, i64 77, i64 75, i64 73, i64 71, i64 69>
; CHECK-NEXT: [[STATEPOINT_TOKEN1:%.*]] = call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 2882400000, i32 0, void ()* elementtype(void ()) @bar, i32 0, i32 0, i32 0, i32 0) [ "deopt"(i32 0, i32 1, i32 0, i32 112, i32 0, i32 13, i32 0, i32 7, i8* null, i32 7, i8* null, i32 3, i32 undef, i32 3, i32 14, i32 3, i32 -2406, i32 3, i32 28963, i32 3, i32 30401, i32 3, i32 -11, i32 3, i32 -5, i32 3, i32 1, i32 0, i8 addrspace(1)* [[PTR_RELOCATED]], i32 0, i8 addrspace(1)* [[PTR_RELOCATED]], i32 7, i8* null), "gc-live"(<8 x float addrspace(1)*> [[GEP]], i8 addrspace(1)* [[PTR_RELOCATED]], <8 x float addrspace(1)*> [[DOTSPLAT_BASE]]) ]
; CHECK-NEXT: [[GEP_RELOCATED:%.*]] = call coldcc <8 x i8 addrspace(1)*> @llvm.experimental.gc.relocate.v8p1i8(token [[STATEPOINT_TOKEN1]], i32 2, i32 0)
; CHECK-NEXT: [[GEP_RELOCATED_CASTED:%.*]] = bitcast <8 x i8 addrspace(1)*> [[GEP_RELOCATED]] to <8 x float addrspace(1)*>
; CHECK-NEXT: [[PTR_RELOCATED2:%.*]] = call coldcc i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token [[STATEPOINT_TOKEN1]], i32 1, i32 1)
; CHECK-NEXT: [[DOTSPLAT_BASE_RELOCATED:%.*]] = call coldcc <8 x i8 addrspace(1)*> @llvm.experimental.gc.relocate.v8p1i8(token [[STATEPOINT_TOKEN1]], i32 2, i32 2)
; CHECK-NEXT: [[DOTSPLAT_BASE_RELOCATED_CASTED:%.*]] = bitcast <8 x i8 addrspace(1)*> [[DOTSPLAT_BASE_RELOCATED]] to <8 x float addrspace(1)*>
; CHECK-NEXT: [[RES:%.*]] = call <8 x float> @llvm.masked.gather.v8f32.v8p1f32(<8 x float addrspace(1)*> [[GEP_RELOCATED_CASTED]], i32 4, <8 x i1> <i1 true, i1 true, i1 true, i1 true, i1 true, i1 true, i1 true, i1 true>, <8 x float> undef)
; CHECK-NEXT: unreachable
;
entry:
%ptr.cast = bitcast i8 addrspace(1)* %ptr to float addrspace(1)*
call void @foo() [ "deopt"(i32 0, i32 2, i32 0, i32 62, i32 0, i32 13, i32 0, i32 7, i8* null, i32 7, i8* null, i32 7, i8* null, i32 3, i32 14, i32 3, i32 -2406, i32 3, i32 28963, i32 3, i32 30401, i32 3, i32 -11, i32 3, i32 -5, i32 3, i32 1, i32 0, i8 addrspace(1)* %ptr, i32 0, i8 addrspace(1)* %ptr, i32 7, i8* null) ]
%gep = getelementptr inbounds float, float addrspace(1)* %ptr.cast, <8 x i64> <i64 83, i64 81, i64 79, i64 77, i64 75, i64 73, i64 71, i64 69>
call void @bar() [ "deopt"(i32 0, i32 1, i32 0, i32 112, i32 0, i32 13, i32 0, i32 7, i8* null, i32 7, i8* null, i32 3, i32 undef, i32 3, i32 14, i32 3, i32 -2406, i32 3, i32 28963, i32 3, i32 30401, i32 3, i32 -11, i32 3, i32 -5, i32 3, i32 1, i32 0, i8 addrspace(1)* %ptr, i32 0, i8 addrspace(1)* %ptr, i32 7, i8* null) ]
%res = call <8 x float> @llvm.masked.gather.v8f32.v8p1f32(<8 x float addrspace(1)*> %gep, i32 4, <8 x i1> <i1 true, i1 true, i1 true, i1 true, i1 true, i1 true, i1 true, i1 true>, <8 x float> undef)
unreachable
}
declare void @foo() gc "statepoint-example"
; Function Attrs: nocallback nofree nosync nounwind readonly willreturn
declare <8 x float> @llvm.masked.gather.v8f32.v8p1f32(<8 x float addrspace(1)*>, i32 immarg, <8 x i1>, <8 x float>) #1
declare void @bar()
attributes #1 = { nocallback nofree nosync nounwind readonly willreturn }