[llvm-branch-commits] [DirectX] Lower `@llvm.dx.typedBufferStore` to DXIL ops (PR #104253)

[Justin Bogner via llvm-branch-commits]

================
@@ -94,6 +95,7 @@ class OpLowerer {
         DiagnosticInfoUnsupported Diag(*CI->getFunction(), Message,
                                        CI->getDebugLoc());
         M.getContext().diagnose(Diag);
+        HasErrors = true;
----------------
bogner wrote:

Even if we did parallel codegen we'd surely have different instances of the `OpLowerer` in different threads, no? In any case, I do like this suggestion for readability reasons since updating a member variable like this is harder to follow than control flow.

One thing that makes it tricky is that it really is easiest to turn the error into a diagnostic in `replaceFunction` - that's where we have the context of which call couldn't be replaced and access to its `DebugInfo`. So there are a couple of options:
1. Create a custom `ErrorInfo` subclass so that we can propagate info about the call instruction up. This would need to live somewhere shared between `DXILOpLowering` and `DXILOpBuilder`.
2. Keep handling the error in `replaceFunction` but also return a `bool` to indicate success/failure
3. Handle the error in `replaceFunction` but then return an Error anyway. This would probably need a different `ErrorInfo` subclass so we could differentiate between errors we handled and one's we didn't.

Option (3) just seems too messy, and option (1) adds quite a bit of code without a lot of advantage. Option (2) is very simple but `bool Success` return values make me sad.

Thoughts?

https://github.com/llvm/llvm-project/pull/104253