[Statepoint] Return undef value for the statepoint of the none token #72552
Conversation
Helps avoid the crash in verifier when it tries to print the error. `none` token might be produced by llvm-reduce, since it's a default value, so by extension this also fixes llvm-reduce crash, allowing it to just discard invalid IR.
|
@llvm/pr-subscribers-llvm-ir Author: Danila Malyutin (danilaml) ChangesHelps avoid the crash in verifier when it tries to print the error. We might special-case tokens in ReduceArguments and to use some other value, other than Full diff: https://github.com/llvm/llvm-project/pull/72552.diff 2 Files Affected:
diff --git a/llvm/lib/IR/IntrinsicInst.cpp b/llvm/lib/IR/IntrinsicInst.cpp
index a24ca8d100527d5..77c3bbbb8baecf5 100644
--- a/llvm/lib/IR/IntrinsicInst.cpp
+++ b/llvm/lib/IR/IntrinsicInst.cpp
@@ -870,6 +870,11 @@ const Value *GCProjectionInst::getStatepoint() const {
if (isa<UndefValue>(Token))
return Token;
+ // Treat none token as if it was undef here
+ if (isa<ConstantTokenNone>(Token)) {
+ return UndefValue::get(Token->getType());
+ }
+
// This takes care both of relocates for call statepoints and relocates
// on normal path of invoke statepoint.
if (!isa<LandingPadInst>(Token))
diff --git a/llvm/test/Verifier/gc_none_token.ll b/llvm/test/Verifier/gc_none_token.ll
new file mode 100644
index 000000000000000..3847f625c4869f8
--- /dev/null
+++ b/llvm/test/Verifier/gc_none_token.ll
@@ -0,0 +1,18 @@
+; RUN: not opt -passes=verify -S %s 2>&1 | FileCheck %s
+; Check that verifier doesn't crash on relocate with none token
+
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @check_verify_none_token() gc "statepoint-example" {
+
+entry:
+ ret i32 0
+
+unreach:
+ ; CHECK: gc relocate is incorrectly tied to the statepoint
+ ; CHECK: (undef, undef)
+ %token_call = call i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(token none, i32 0, i32 0)
+ ret i32 1
+}
+
+declare i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(token, i32, i32)
|
Co-authored-by: arpilipe <[email protected]>
|
Can you use poison here? |
|
@nunoplopes possibly. Not sure what semantics of this would be though. And this would need to be adapted to poison as well (otherwise there is no point): d693fd2 |
Right. It seems all of those could be converted to poison. It's being used just as a placeholder, so anything works. |
…lvm#72552) Helps avoid the crash in verifier when it tries to print the error. `none` token might be produced by llvm-reduce, since it's a default value, so by extension this also fixes llvm-reduce crash, allowing it to just discard invalid IR. --------- Co-authored-by: arpilipe <[email protected]>
…lvm#72552) Helps avoid the crash in verifier when it tries to print the error. `none` token might be produced by llvm-reduce, since it's a default value, so by extension this also fixes llvm-reduce crash, allowing it to just discard invalid IR. --------- Co-authored-by: arpilipe <[email protected]>
Helps avoid the crash in verifier when it tries to print the error.
nonetoken might be produced by llvm-reduce, since it's a default value, so by extension this also fixes llvm-reduce crash, allowing it to just discard invalid IR.We might special-case tokens in ReduceArguments and to use some other value, other than
getDefaultValue(or change thegetDefaultValueto use something other thangetNullValuefor token), but it still wouldn't fix the fact that verifier would crash trying to print an error message if it ever encounters such relocation.I wasn't able to come up with any potential issues with handling
nonetoken as if it wasundef, but it doesn't mean that there isn't any =)