[clang] [llvm] Reland "[HLSL] Implement the reflect HLSL function" (PR #125599)

[Justin Bogner via cfe-commits]

================
@@ -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