================
@@ -905,6 +903,14 @@ bool SPIRVInstructionSelector::selectExtInst(Register
ResVReg,
const SPIRVType *ResType,
MachineInstr &I,
GL::GLSLExtInst GLInst) const {
+ if (!STI.canUseExtInstSet(
+ SPIRV::InstructionSet::InstructionSet::GLSL_std_450)) {
+ std::string DiagMsg;
+ raw_string_ostream OS(DiagMsg);
+ I.print(OS, true, false, false, false);
+ DiagMsg += " is only supported with the GLSL extended instruction set.\n";
+ report_fatal_error(DiagMsg.c_str(), false);
+ }
----------------
bogner wrote:
IMO this isn't the right way to do this kind of error handling.
`report_fatal_error` just causes the compiler to crash, but given that this can
happen given IR for the wrong target just crashing because of it isn't great.
We should probably be using `LLVMContext::diagnose` here instead so that we can
bubble the error up to the driver. Note that I do think we probably need to do
a little bit of refactoring to use `diagnose` here, as this will probably just
crash anyway with a `Cannot select` error if we just call diagnose and return
from this function.
I'll comment on #123847 to this effect.
In the meantime, I also don't really like that we're adding this error handling
only for the `GLSLExtInst` overload and not all of the extremely related cases.
I think we should treat this as out of scope for the `reflect` function change,
remove the `report_fatal_error`, and drop or XFAIL the `reflect-error.ll` test
for now - we can reintroduce that test when we fix #123847.
https://github.com/llvm/llvm-project/pull/125599
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
