[llvm] f845076 - [StatepointLowering] Move statepoint correctness checks to Verifier. NFC.
Denis Antrushin via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 18 07:16:28 PST 2023
Author: Denis Antrushin
Date: 2023-01-18T18:14:58+03:00
New Revision: f8450767384b2a929bad250b4942289598e89612
URL: https://github.com/llvm/llvm-project/commit/f8450767384b2a929bad250b4942289598e89612
DIFF: https://github.com/llvm/llvm-project/commit/f8450767384b2a929bad250b4942289598e89612.diff
LOG: [StatepointLowering] Move statepoint correctness checks to Verifier. NFC.
Since D140504, GCStrategy is available for use in opt.
Now we can move statepoint correctness checks from
StatepointLowering.cpp to Verifier.
Reviewed By: reames
Differential Revision: https://reviews.llvm.org/D141948
Added:
Modified:
llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
llvm/lib/IR/Verifier.cpp
llvm/test/Transforms/InstCombine/statepoint-iter.ll
llvm/test/Verifier/gc_relocate_operand.ll
Removed:
################################################################################
diff --git a/llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp b/llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
index 5d3307257ba59..57bfe344dbab3 100644
--- a/llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
@@ -524,32 +524,6 @@ lowerStatepointMetaArgs(SmallVectorImpl<SDValue> &Ops,
SelectionDAGBuilder &Builder) {
// Lower the deopt and gc arguments for this statepoint. Layout will be:
// deopt argument length, deopt arguments.., gc arguments...
-#ifndef NDEBUG
- if (auto *GFI = Builder.GFI) {
- // Check that each of the gc pointer and bases we've gotten out of the
- // safepoint is something the strategy thinks might be a pointer (or vector
- // of pointers) into the GC heap. This is basically just here to help catch
- // errors during statepoint insertion. TODO: This should actually be in the
- // Verifier, but we can't get to the GCStrategy from there (yet).
- GCStrategy &S = GFI->getStrategy();
- for (const Value *V : SI.Bases) {
- auto Opt = S.isGCManagedPointer(V->getType()->getScalarType());
- if (Opt) {
- assert(*Opt && "non gc managed base pointer found in statepoint");
- }
- }
- for (const Value *V : SI.Ptrs) {
- auto Opt = S.isGCManagedPointer(V->getType()->getScalarType());
- if (Opt) {
- assert(*Opt && "non gc managed derived pointer found in statepoint");
- }
- }
- assert(SI.Bases.size() == SI.Ptrs.size() && "Pointer without base!");
- } else {
- assert(SI.Bases.empty() && "No gc specified, so cannot relocate pointers!");
- assert(SI.Ptrs.empty() && "No gc specified, so cannot relocate pointers!");
- }
-#endif
// Figure out what lowering strategy we're going to use for each part
// Note: Is is conservatively correct to lower both "live-in" and "live-out"
@@ -738,7 +712,9 @@ SDValue SelectionDAGBuilder::LowerAsSTATEPOINT(
NumOfStatepoints++;
// Clear state
StatepointLowering.startNewStatepoint(*this);
- assert(SI.Bases.size() == SI.Ptrs.size());
+ assert(SI.Bases.size() == SI.Ptrs.size() && "Pointer without base!");
+ assert((GFI || SI.Bases.empty()) &&
+ "No gc specified, so cannot relocate pointers!");
LLVM_DEBUG(dbgs() << "Lowering statepoint " << *SI.StatepointInstr << "\n");
#ifndef NDEBUG
@@ -1234,10 +1210,6 @@ void SelectionDAGBuilder::visitGCRelocate(const GCRelocateInst &Relocate) {
if (cast<GCStatepointInst>(Statepoint)->getParent() == Relocate.getParent())
StatepointLowering.relocCallVisited(Relocate);
-
- auto *Ty = Relocate.getType()->getScalarType();
- if (auto IsManaged = GFI->getStrategy().isGCManagedPointer(Ty))
- assert(*IsManaged && "Non gc managed pointer relocated!");
#endif
const Value *DerivedPtr = Relocate.getDerivedPtr();
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 6ce9ccfd068aa..233e10bb223af 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -73,6 +73,7 @@
#include "llvm/IR/DerivedTypes.h"
#include "llvm/IR/Dominators.h"
#include "llvm/IR/Function.h"
+#include "llvm/IR/GCStrategy.h"
#include "llvm/IR/GlobalAlias.h"
#include "llvm/IR/GlobalValue.h"
#include "llvm/IR/GlobalVariable.h"
@@ -5361,11 +5362,15 @@ void Verifier::visitIntrinsicCall(Intrinsic::ID ID, CallBase &Call) {
// relocated pointer. It can be casted to the correct type later if it's
// desired. However, they must have the same address space and 'vectorness'
GCRelocateInst &Relocate = cast<GCRelocateInst>(Call);
- Check(Relocate.getDerivedPtr()->getType()->isPtrOrPtrVectorTy(),
- "gc.relocate: relocated value must be a gc pointer", Call);
+ auto *ResultType = Call.getType();
+ auto *DerivedType = Relocate.getDerivedPtr()->getType();
+ auto *BaseType = Relocate.getBasePtr()->getType();
+
+ Check(BaseType->isPtrOrPtrVectorTy(),
+ "gc.relocate: relocated value must be a pointer", Call);
+ Check(DerivedType->isPtrOrPtrVectorTy(),
+ "gc.relocate: relocated value must be a pointer", Call);
- auto ResultType = Call.getType();
- auto DerivedType = Relocate.getDerivedPtr()->getType();
Check(ResultType->isVectorTy() == DerivedType->isVectorTy(),
"gc.relocate: vector relocates to vector and pointer to pointer",
Call);
@@ -5374,6 +5379,20 @@ void Verifier::visitIntrinsicCall(Intrinsic::ID ID, CallBase &Call) {
DerivedType->getPointerAddressSpace(),
"gc.relocate: relocating a pointer shouldn't change its address space",
Call);
+
+ auto GC = llvm::getGCStrategy(Relocate.getFunction()->getGC());
+ Check(GC, "gc.relocate: calling function must have GCStrategy",
+ Call.getFunction());
+ if (GC) {
+ auto isGCPtr = [&GC](Type *PTy) {
+ return GC->isGCManagedPointer(PTy->getScalarType()).value_or(true);
+ };
+ Check(isGCPtr(ResultType), "gc.relocate: must return gc pointer", Call);
+ Check(isGCPtr(BaseType),
+ "gc.relocate: relocated value must be a gc pointer", Call);
+ Check(isGCPtr(DerivedType),
+ "gc.relocate: relocated value must be a gc pointer", Call);
+ }
break;
}
case Intrinsic::eh_exceptioncode:
diff --git a/llvm/test/Transforms/InstCombine/statepoint-iter.ll b/llvm/test/Transforms/InstCombine/statepoint-iter.ll
index 2dff4ee2e4f21..91f61366a5d6a 100644
--- a/llvm/test/Transforms/InstCombine/statepoint-iter.ll
+++ b/llvm/test/Transforms/InstCombine/statepoint-iter.ll
@@ -26,19 +26,19 @@ right:
br label %merge
left:
- %safepoint_token = tail call token (i64, i32, ptr, i32, i32, ...) @llvm.experimental.gc.statepoint.p0(i64 0, i32 0, ptr elementtype(void ()) @func, i32 0, i32 0, i32 0, i32 0) ["gc-live" (ptr null)]
- %pnew = call ptr @llvm.experimental.gc.relocate.p1(token %safepoint_token, i32 0, i32 0)
+ %safepoint_token = tail call token (i64, i32, ptr, i32, i32, ...) @llvm.experimental.gc.statepoint.p0(i64 0, i32 0, ptr elementtype(void ()) @func, i32 0, i32 0, i32 0, i32 0) ["gc-live" (ptr addrspace(1) null)]
+ %pnew = call ptr addrspace(1) @llvm.experimental.gc.relocate.p1(token %safepoint_token, i32 0, i32 0)
br label %merge
merge:
- %pnew_phi = phi ptr [null, %right], [%pnew, %left]
- %safepoint_token2 = tail call token (i64, i32, ptr, i32, i32, ...) @llvm.experimental.gc.statepoint.p0(i64 0, i32 0, ptr elementtype(void ()) @func, i32 0, i32 0, i32 0, i32 0) ["gc-live" (ptr %pnew_phi)]
- %pnew2 = call ptr @llvm.experimental.gc.relocate.p1(token %safepoint_token2, i32 0, i32 0)
- %cmp = icmp eq ptr %pnew2, null
+ %pnew_phi = phi ptr addrspace(1) [null, %right], [%pnew, %left]
+ %safepoint_token2 = tail call token (i64, i32, ptr, i32, i32, ...) @llvm.experimental.gc.statepoint.p0(i64 0, i32 0, ptr elementtype(void ()) @func, i32 0, i32 0, i32 0, i32 0) ["gc-live" (ptr addrspace(1) %pnew_phi)]
+ %pnew2 = call ptr addrspace(1) @llvm.experimental.gc.relocate.p1(token %safepoint_token2, i32 0, i32 0)
+ %cmp = icmp eq ptr addrspace(1) %pnew2, null
ret i1 %cmp
}
-define ptr @test_undef(i1 %cond) gc "statepoint-example" {
+define ptr addrspace(1) @test_undef(i1 %cond) gc "statepoint-example" {
; CHECK-LABEL: @test_undef(
; CHECK-NEXT: entry:
; CHECK-NEXT: br i1 [[COND:%.*]], label [[LEFT:%.*]], label [[RIGHT:%.*]]
@@ -49,7 +49,7 @@ define ptr @test_undef(i1 %cond) gc "statepoint-example" {
; CHECK-NEXT: br label [[MERGE]]
; CHECK: merge:
; CHECK-NEXT: [[SAFEPOINT_TOKEN2:%.*]] = tail call token (i64, i32, ptr, i32, i32, ...) @llvm.experimental.gc.statepoint.p0(i64 0, i32 0, ptr nonnull elementtype(void ()) @func, i32 0, i32 0, i32 0, i32 0) [ "gc-live"() ]
-; CHECK-NEXT: ret ptr undef
+; CHECK-NEXT: ret ptr addrspace(1) undef
;
entry:
br i1 %cond, label %left, label %right
@@ -58,16 +58,16 @@ right:
br label %merge
left:
- %safepoint_token = tail call token (i64, i32, ptr, i32, i32, ...) @llvm.experimental.gc.statepoint.p0(i64 0, i32 0, ptr elementtype(void ()) @func, i32 0, i32 0, i32 0, i32 0) ["gc-live" (ptr undef)]
- %pnew = call ptr @llvm.experimental.gc.relocate.p1(token %safepoint_token, i32 0, i32 0)
+ %safepoint_token = tail call token (i64, i32, ptr, i32, i32, ...) @llvm.experimental.gc.statepoint.p0(i64 0, i32 0, ptr elementtype(void ()) @func, i32 0, i32 0, i32 0, i32 0) ["gc-live" (ptr addrspace(1) undef)]
+ %pnew = call ptr addrspace(1) @llvm.experimental.gc.relocate.p1(token %safepoint_token, i32 0, i32 0)
br label %merge
merge:
- %pnew_phi = phi ptr [undef, %right], [%pnew, %left]
- %safepoint_token2 = tail call token (i64, i32, ptr, i32, i32, ...) @llvm.experimental.gc.statepoint.p0(i64 0, i32 0, ptr elementtype(void ()) @func, i32 0, i32 0, i32 0, i32 0) ["gc-live" (ptr %pnew_phi)]
- %pnew2 = call ptr @llvm.experimental.gc.relocate.p1(token %safepoint_token2, i32 0, i32 0)
- ret ptr %pnew2
+ %pnew_phi = phi ptr addrspace(1) [undef, %right], [%pnew, %left]
+ %safepoint_token2 = tail call token (i64, i32, ptr, i32, i32, ...) @llvm.experimental.gc.statepoint.p0(i64 0, i32 0, ptr elementtype(void ()) @func, i32 0, i32 0, i32 0, i32 0) ["gc-live" (ptr addrspace(1) %pnew_phi)]
+ %pnew2 = call ptr addrspace(1) @llvm.experimental.gc.relocate.p1(token %safepoint_token2, i32 0, i32 0)
+ ret ptr addrspace(1) %pnew2
}
declare token @llvm.experimental.gc.statepoint.p0(i64, i32, ptr, i32, i32, ...)
-declare ptr @llvm.experimental.gc.relocate.p1(token, i32, i32)
+declare ptr addrspace(1) @llvm.experimental.gc.relocate.p1(token, i32, i32)
diff --git a/llvm/test/Verifier/gc_relocate_operand.ll b/llvm/test/Verifier/gc_relocate_operand.ll
index 4a89ca2efcbf3..615731b6afacd 100644
--- a/llvm/test/Verifier/gc_relocate_operand.ll
+++ b/llvm/test/Verifier/gc_relocate_operand.ll
@@ -1,7 +1,7 @@
; RUN: not llvm-as -disable-output <%s 2>&1 | FileCheck %s
; This is to verify that the relocated value by gc_relocate must be a pointer type.
-; CHECK: gc.relocate: relocated value must be a gc pointer
+; CHECK: gc.relocate: relocated value must be a pointer
declare void @foo()
More information about the llvm-commits
mailing list