-
Notifications
You must be signed in to change notification settings - Fork 17.9k
[SPIRV] Add support for SPV_KHR_abort extension #193037
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
eab1954
31414b8
d0da520
4936a11
5ac1874
6bacaf1
f4c0835
dd33160
f65c7f7
57c64dd
b5c48c4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2253,6 +2253,29 @@ Instruction *SPIRVEmitIntrinsics::visitAtomicCmpXchgInst(AtomicCmpXchgInst &I) { | |
| Instruction *SPIRVEmitIntrinsics::visitUnreachableInst(UnreachableInst &I) { | ||
| IRBuilder<> B(I.getParent()); | ||
| B.SetInsertPoint(&I); | ||
|
vmustya marked this conversation as resolved.
Outdated
|
||
| // OpAbortKHR is itself a SPIR-V block terminator. If the immediately | ||
| // preceding instruction is a call to llvm.spv.abort, do not emit an | ||
| // additional OpUnreachable, which would leave the SPIR-V block with two | ||
| // terminators and produce invalid SPIR-V. The check is intentionally limited | ||
| // to the directly-preceding non-debug instruction: any real instruction | ||
| // sitting between `llvm.spv.abort` and `unreachable` would also be invalid | ||
| // SPIR-V (nothing can follow OpAbortKHR in the same block), so assert that | ||
| // shape if we see an `spv_abort` anywhere earlier in the block. | ||
| Instruction *Prev = I.getPrevNode(); | ||
| while (Prev && Prev->isDebugOrPseudoInst()) | ||
| Prev = Prev->getPrevNode(); | ||
| if (auto *CI = dyn_cast_or_null<CallInst>(Prev); | ||
| CI && CI->getIntrinsicID() == Intrinsic::spv_abort) | ||
| return &I; | ||
| #ifndef NDEBUG | ||
| for (Instruction *P = I.getPrevNode(); P; P = P->getPrevNode()) { | ||
| auto *CI = dyn_cast<CallInst>(P); | ||
| if (CI && CI->getIntrinsicID() == Intrinsic::spv_abort) | ||
| llvm_unreachable("llvm.spv.abort must be the last non-debug instruction " | ||
| "before its block's `unreachable`; OpAbortKHR is itself " | ||
| "a SPIR-V block terminator"); | ||
| } | ||
| #endif | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This kind of check is for the verifier and should not be in here. You should be able to assume that the IR is well formed. Or if it's only meant for debug builds, like an assertion, then use an actual assertion. PS: I'll do the later. On a totally different aspect, instead of iteration backwards, you can iterate from the beginning of the block until
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, I'm not sure the check is correct. Look at the example I posted in the translator PR: KhronosGroup/SPIRV-LLVM-Translator#3691 (comment) Why would that be invalid? I think we should be able to gracefully handle that instead of asserting.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've implemented the handling on the stage, when the |
||
| B.CreateIntrinsic(Intrinsic::spv_unreachable, {}); | ||
| return &I; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -350,6 +350,7 @@ class SPIRVInstructionSelector : public InstructionSelector { | |
|
|
||
| bool diagnoseUnsupported(const MachineInstr &I, const Twine &Msg) const; | ||
|
|
||
| bool selectAbort(MachineInstr &I) const; | ||
| bool selectFrameIndex(Register ResVReg, SPIRVTypeInst ResType, | ||
| MachineInstr &I) const; | ||
| bool selectAllocaArray(Register ResVReg, SPIRVTypeInst ResType, | ||
|
|
@@ -4635,6 +4636,8 @@ bool SPIRVInstructionSelector::selectIntrinsic(Register ResVReg, | |
| BuildMI(BB, I, I.getDebugLoc(), TII.get(SPIRV::OpUnreachable)) | ||
| .constrainAllUses(TII, TRI, RBI); | ||
| return true; | ||
| case Intrinsic::spv_abort: | ||
| return selectAbort(I); | ||
| case Intrinsic::spv_alloca: | ||
| return selectFrameIndex(ResVReg, ResType, I); | ||
| case Intrinsic::spv_alloca_array: | ||
|
|
@@ -6250,6 +6253,63 @@ bool SPIRVInstructionSelector::selectAllocaArray(Register ResVReg, | |
| return true; | ||
| } | ||
|
|
||
| bool SPIRVInstructionSelector::selectAbort(MachineInstr &I) const { | ||
| if (!STI.canUseExtension(SPIRV::Extension::SPV_KHR_abort)) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we also check for/enable implicitly
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The extension formally requires the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think you should be doing this check in here. It is done already in
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are similar checks already implemented for the |
||
| report_fatal_error("OpAbortKHR instruction requires the following " | ||
| "SPIR-V extension: SPV_KHR_abort", | ||
| false); | ||
| // The intrinsic is declared as variadic so it can carry composite message | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This shouldn't hit a fatal error. This should call
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done: 6bacaf1
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe the problem is that it was declared as variadic in the first place. Why do we need that? |
||
| // types (vectors and structs) without LLVM IR mangling restrictions, but | ||
| // OpAbortKHR takes exactly one Message operand. | ||
| if (I.getNumExplicitOperands() != 2) | ||
| report_fatal_error("llvm.spv.abort must be called with exactly one " | ||
| "message argument", | ||
| false); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This kind of check should never happen in well-formed LLVM-IR right? You could do it in the verifier. Or put an |
||
| Register MsgReg = I.getOperand(1).getReg(); | ||
| SPIRVTypeInst MsgType = GR.getSPIRVTypeForVReg(MsgReg); | ||
| assert(MsgType && "Message argument of llvm.spv.abort has no SPIR-V type"); | ||
| // SPV_KHR_abort requires Message Type to be a concrete type. Per the | ||
| // SPIR-V "Concrete Type" definition, that means a numerical scalar | ||
| // (int/float), a (physical) pointer, a vector, matrix, or any aggregate | ||
| // (array/struct) recursively containing only such types. OpTypeBool, | ||
| // OpTypeVoid, opaque handles and similar abstract/non-concrete types are | ||
| // rejected up front rather than emitting invalid SPIR-V. Validate | ||
| // recursively so that e.g. a struct containing a bool is also rejected. | ||
| SmallVector<SPIRVTypeInst, 4> Worklist{MsgType}; | ||
| while (!Worklist.empty()) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this deserves its own function. |
||
| SPIRVTypeInst Ty = Worklist.pop_back_val(); | ||
| switch (Ty->getOpcode()) { | ||
| case SPIRV::OpTypeInt: | ||
| case SPIRV::OpTypeFloat: | ||
| case SPIRV::OpTypePointer: | ||
| break; | ||
| case SPIRV::OpTypeVector: | ||
| case SPIRV::OpTypeMatrix: | ||
| case SPIRV::OpTypeArray: | ||
| // Operand 1 holds the element/component type id. | ||
| Worklist.push_back(GR.getSPIRVTypeForVReg(Ty->getOperand(1).getReg())); | ||
| break; | ||
| case SPIRV::OpTypeStruct: | ||
| // Operands 1..N hold the field type ids. | ||
| for (unsigned Idx = 1, E = Ty->getNumOperands(); Idx < E; ++Idx) | ||
| Worklist.push_back( | ||
| GR.getSPIRVTypeForVReg(Ty->getOperand(Idx).getReg())); | ||
| break; | ||
| default: | ||
|
Comment on lines
+6343
to
+6364
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could this also be a physical pointer (since it can be any concrete type)?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Implemented: d0da520 |
||
| report_fatal_error("llvm.spv.abort message type must be a concrete " | ||
| "SPIR-V type (numerical scalar, pointer, vector, " | ||
| "matrix, or aggregate of such types)", | ||
| false); | ||
| } | ||
| } | ||
| MachineBasicBlock &BB = *I.getParent(); | ||
| BuildMI(BB, I, I.getDebugLoc(), TII.get(SPIRV::OpAbortKHR)) | ||
| .addUse(GR.getSPIRVTypeID(MsgType)) | ||
| .addUse(MsgReg) | ||
| .constrainAllUses(TII, TRI, RBI); | ||
| return true; | ||
| } | ||
|
|
||
| bool SPIRVInstructionSelector::selectFrameIndex(Register ResVReg, | ||
| SPIRVTypeInst ResType, | ||
| MachineInstr &I) const { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,7 @@ | |
| //===----------------------------------------------------------------------===// | ||
|
|
||
| #include "SPIRV.h" | ||
| #include "SPIRVBuiltins.h" | ||
| #include "SPIRVSubtarget.h" | ||
| #include "SPIRVTargetMachine.h" | ||
| #include "SPIRVUtils.h" | ||
|
|
@@ -33,6 +34,7 @@ | |
| #include "llvm/IR/Intrinsics.h" | ||
| #include "llvm/IR/IntrinsicsSPIRV.h" | ||
| #include "llvm/Transforms/Utils/Cloning.h" | ||
| #include "llvm/Transforms/Utils/Local.h" | ||
| #include "llvm/Transforms/Utils/LowerMemIntrinsics.h" | ||
| #include <regex> | ||
|
|
||
|
|
@@ -43,6 +45,7 @@ namespace { | |
| class SPIRVPrepareFunctions : public ModulePass { | ||
| const SPIRVTargetMachine &TM; | ||
| bool substituteIntrinsicCalls(Function *F); | ||
| bool substituteAbortKHRCalls(Function *F); | ||
| Function *removeAggregateTypesFromSignature(Function *F); | ||
| bool removeAggregateTypesFromCalls(Function *F); | ||
|
|
||
|
|
@@ -595,6 +598,62 @@ SPIRVPrepareFunctions::removeAggregateTypesFromSignature(Function *F) { | |
| return NewF; | ||
| } | ||
|
|
||
| // Replace OpenCL/SPIR-V style calls to `__spirv_AbortKHR(message)` with calls | ||
| // to the `llvm.spv.abort` target intrinsic, so that they go through the same | ||
| // instruction-selection path as the intrinsic and get lowered to OpAbortKHR. | ||
| bool SPIRVPrepareFunctions::substituteAbortKHRCalls(Function *F) { | ||
| if (F->isDeclaration()) | ||
| return false; | ||
|
|
||
| SmallVector<CallInst *> Calls; | ||
| for (Instruction &I : instructions(F)) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think calls to
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 to this. We can leave it as todo for now, but ultimately we should be lowering llvm.trap to spirv.abort. The Q would be: "what kind of message we should pass in this case?", may be something like: "abort is caused by llvm.trap with no debug string" is good enough.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would like the translation to be implemented in this very patch, if possible. It's tightly related, so we may as well have it all together. Regarding the message, we could try and be a little more verbose and pass maybe something more meaningful. I'd say maybe a combination of the instruction, basic block, function and module, so that at least it's possible to know where the abort happened. I'd be happy to have that in a follow up, though. Something basic is good to start with.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd prefer to emit something like
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That works for me as a starting point. We can follow up in the future.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good to me. If anything, we can change it to something else later. PS: I'll be on vacations so I won't be able to go further with the review. But don't wait for my approval to land this change.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Implemented both
See the details: 6bacaf1 |
||
| auto *CI = dyn_cast<CallInst>(&I); | ||
| if (!CI) | ||
| continue; | ||
| Function *Callee = CI->getCalledFunction(); | ||
| if (!Callee || Callee->isIntrinsic()) | ||
| continue; | ||
| StringRef Demangled = Callee->getName(); | ||
| std::string DemangledStr = getOclOrSpirvBuiltinDemangledName(Demangled); | ||
| if (DemangledStr.empty()) | ||
| continue; | ||
| std::string BuiltinName = SPIRV::lookupBuiltinNameHelper(DemangledStr); | ||
| if (StringRef(BuiltinName) != "__spirv_AbortKHR") | ||
| continue; | ||
| if (CI->arg_size() != 1) | ||
| continue; | ||
| Calls.push_back(CI); | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should do this the other way around, scan the functions in the module looking for Otherwise we scan the whole module even if
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done: 6bacaf1 |
||
|
|
||
| if (Calls.empty()) | ||
| return false; | ||
|
|
||
| for (CallInst *CI : Calls) { | ||
| IRBuilder<> B(CI); | ||
| Value *Msg = CI->getArgOperand(0); | ||
| // The OpenCL C ABI may pass aggregate arguments by pointer (byval). In | ||
| // that case load the underlying value so that OpAbortKHR receives the | ||
| // composite itself, as required by the SPV_KHR_abort spec ("Message Type | ||
| // must be a concrete type"). | ||
| if (CI->isByValArgument(0)) { | ||
| Type *AggTy = CI->getParamByValType(0); | ||
| Msg = B.CreateLoad(AggTy, Msg); | ||
| } | ||
| B.CreateIntrinsic(Intrinsic::spv_abort, {}, {Msg}); | ||
| // OpAbortKHR is itself a SPIR-V function-termination instruction and must | ||
| // be the last instruction in its block. Drop the original call and | ||
| // everything that follows it (the OpenCL ABI typically appends stores into | ||
| // the return slot and a `ret`), and re-terminate the block with | ||
| // `unreachable`. We use changeToUnreachable so that any successor PHI | ||
| // nodes have the now-removed predecessor edge cleaned up; otherwise the | ||
| // IR verifier would reject mismatched PHI incoming entries. The matching | ||
| // suppression in SPIRVEmitIntrinsics::visitUnreachableInst ensures no | ||
| // extra OpUnreachable is emitted after OpAbortKHR. | ||
| changeToUnreachable(CI); | ||
| } | ||
| return true; | ||
| } | ||
|
|
||
| // Mutates indirect callsites iff if aggregate argument/return types are present | ||
| // with the types replaced by i32 types. The change in types is noted in | ||
| // 'spv.mutated_callsites' metadata for later restoration. | ||
|
|
@@ -684,6 +743,7 @@ bool SPIRVPrepareFunctions::runOnModule(Module &M) { | |
|
|
||
| for (Function &F : M) { | ||
| Changed |= substituteIntrinsicCalls(&F); | ||
| Changed |= substituteAbortKHRCalls(&F); | ||
| Changed |= sortBlocks(F); | ||
| Changed |= removeAggregateTypesFromCalls(&F); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| ; RUN: llc -O0 -mtriple=spirv32-unknown-unknown --spirv-ext=+SPV_KHR_abort %s -o - | FileCheck %s | ||
| ; RUN: llc -O0 -mtriple=spirv64-unknown-unknown --spirv-ext=+SPV_KHR_abort %s -o - | FileCheck %s | ||
| ; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv64-unknown-unknown --spirv-ext=+SPV_KHR_abort %s -o - -filetype=obj | spirv-val %} | ||
|
|
||
| ;; Verify that OpAbortKHR can consume a composite produced by | ||
| ;; OpCompositeConstruct, mirroring the SPV_KHR_abort spec example which builds | ||
| ;; the message via OpCompositeConstruct. | ||
|
|
||
| ; CHECK-DAG: OpCapability AbortKHR | ||
| ; CHECK-DAG: OpExtension "SPV_KHR_abort" | ||
| ; CHECK-DAG: %[[#I32:]] = OpTypeInt 32 0 | ||
| ; CHECK-DAG: %[[#V2:]] = OpTypeVector %[[#I32]] 2 | ||
|
|
||
| ; CHECK: %[[#CC:]] = OpCompositeConstruct %[[#V2]] %{{[0-9]+}} %{{[0-9]+}} | ||
| ; CHECK: OpAbortKHR %[[#V2]] %[[#CC]] | ||
| ; CHECK-NOT: OpUnreachable | ||
|
|
||
| declare void @llvm.spv.abort(...) #0 | ||
|
|
||
| define spir_kernel void @abort_composite_construct(i32 %a, i32 %b) { | ||
| entry: | ||
| %va = insertelement <1 x i32> poison, i32 %a, i32 0 | ||
| %vb = insertelement <1 x i32> poison, i32 %b, i32 0 | ||
| %v = shufflevector <1 x i32> %va, <1 x i32> %vb, | ||
| <2 x i32> <i32 0, i32 1> | ||
| call void (...) @llvm.spv.abort(<2 x i32> %v) | ||
| unreachable | ||
| } | ||
|
|
||
| attributes #0 = { noreturn } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| ; RUN: llc -O0 -mtriple=spirv32-unknown-unknown --spirv-ext=+SPV_KHR_abort %s -o - | FileCheck %s | ||
| ; RUN: llc -O0 -mtriple=spirv64-unknown-unknown --spirv-ext=+SPV_KHR_abort %s -o - | FileCheck %s | ||
| ; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv64-unknown-unknown --spirv-ext=+SPV_KHR_abort %s -o - -filetype=obj | spirv-val %} | ||
|
|
||
| ; Verify that OpAbortKHR can consume a composite (struct or vector) message, | ||
| ; built up via OpCompositeInsert, mirroring the example in the SPV_KHR_abort | ||
| ; specification. | ||
|
|
||
| ; CHECK-DAG: OpCapability AbortKHR | ||
| ; CHECK-DAG: OpExtension "SPV_KHR_abort" | ||
|
|
||
| ; CHECK-DAG: %[[#I32:]] = OpTypeInt 32 0 | ||
| ; CHECK-DAG: %[[#F32:]] = OpTypeFloat 32 | ||
| ; CHECK-DAG: %[[#STRUCT:]] = OpTypeStruct %[[#I32]] %[[#I32]] %[[#F32]] | ||
| ; CHECK-DAG: %[[#VEC:]] = OpTypeVector %[[#I32]] 4 | ||
|
|
||
| ; CHECK: %[[#S0:]] = OpCompositeInsert %[[#STRUCT]] %{{[0-9]+}} %{{[0-9]+}} 0 | ||
| ; CHECK: %[[#S1:]] = OpCompositeInsert %[[#STRUCT]] %{{[0-9]+}} %[[#S0]] 1 | ||
| ; CHECK: %[[#S2:]] = OpCompositeInsert %[[#STRUCT]] %{{[0-9]+}} %[[#S1]] 2 | ||
| ; CHECK: OpAbortKHR %[[#STRUCT]] %[[#S2]] | ||
|
|
||
| ; CHECK: %[[#V0:]] = OpCompositeInsert %[[#VEC]] %{{[0-9]+}} %{{[0-9]+}} 0 | ||
| ; CHECK: %[[#V1:]] = OpCompositeInsert %[[#VEC]] %{{[0-9]+}} %[[#V0]] 1 | ||
| ; CHECK: %[[#V2:]] = OpCompositeInsert %[[#VEC]] %{{[0-9]+}} %[[#V1]] 2 | ||
| ; CHECK: %[[#V3:]] = OpCompositeInsert %[[#VEC]] %{{[0-9]+}} %[[#V2]] 3 | ||
| ; CHECK: OpAbortKHR %[[#VEC]] %[[#V3]] | ||
|
|
||
| %struct.Msg = type { i32, i32, float } | ||
|
|
||
| declare void @llvm.spv.abort(...) #0 | ||
|
|
||
| define spir_kernel void @abort_with_struct(i32 %x, i32 %y, float %z) { | ||
| entry: | ||
| %m0 = insertvalue %struct.Msg poison, i32 %x, 0 | ||
| %m1 = insertvalue %struct.Msg %m0, i32 %y, 1 | ||
| %m2 = insertvalue %struct.Msg %m1, float %z, 2 | ||
| call void (...) @llvm.spv.abort(%struct.Msg %m2) | ||
| unreachable | ||
| } | ||
|
|
||
| define spir_kernel void @abort_with_vector(i32 %a, i32 %b, i32 %c, i32 %d) { | ||
| entry: | ||
| %v0 = insertelement <4 x i32> poison, i32 %a, i32 0 | ||
| %v1 = insertelement <4 x i32> %v0, i32 %b, i32 1 | ||
| %v2 = insertelement <4 x i32> %v1, i32 %c, i32 2 | ||
| %v3 = insertelement <4 x i32> %v2, i32 %d, i32 3 | ||
| call void (...) @llvm.spv.abort(<4 x i32> %v3) | ||
| unreachable | ||
| } | ||
|
|
||
| attributes #0 = { noreturn } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| ; RUN: split-file %s %t | ||
| ; RUN: not llc -O0 -mtriple=spirv64-unknown-unknown --spirv-ext=+SPV_KHR_abort %t/top-level-bool.ll -o /dev/null 2>&1 | FileCheck %s | ||
| ; RUN: not llc -O0 -mtriple=spirv64-unknown-unknown --spirv-ext=+SPV_KHR_abort %t/nested-bool.ll -o /dev/null 2>&1 | FileCheck %s | ||
|
|
||
| ; CHECK: llvm.spv.abort message type must be a concrete SPIR-V type | ||
|
|
||
| ;--- top-level-bool.ll | ||
| declare void @llvm.spv.abort(...) #0 | ||
| define void @abort_with_bool(i1 %b) { | ||
| entry: | ||
| call void (...) @llvm.spv.abort(i1 %b) | ||
| unreachable | ||
| } | ||
| attributes #0 = { noreturn } | ||
|
|
||
| ;--- nested-bool.ll | ||
| %B = type { i32, i1 } | ||
| declare void @llvm.spv.abort(...) #0 | ||
| define void @abort_with_struct_of_bool(i1 %b) { | ||
| entry: | ||
| %s0 = insertvalue %B poison, i32 0, 0 | ||
| %s1 = insertvalue %B %s0, i1 %b, 1 | ||
| call void (...) @llvm.spv.abort(%B %s1) | ||
| unreachable | ||
| } | ||
| attributes #0 = { noreturn } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if we parametrize the intrinsic by type, instead of taking a vararg?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do believe that we need to implement constant data extension before abort so we could use constant data in the arg.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On a second thought, not sure I agree. If instead of a constant string we would like to pass simply a constant int as message, I think that would be valid for OpAbortKHR, wouldn't it? So the constant data extension wouldn't be required for that, would it? Also, I'm concerned about the fact that this accepts any number of arguments, while OpAbortKHR only accepts one: the message --the type of that one should be implicit. Should we use
llvm_any_tyinstead?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I withdraw my comment requiring a constant data. Lets have some implementation experience to may be provide feedback to the specification.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've converted the intrinsic operand type from the vararg to the
llvm_any_ty: 6bacaf1