Add 'IsDebuggerPresent' and detail DebugBreak more#752
Conversation
llvm-beanz
left a comment
llvm-beanz
left a comment
There was a problem hiding this comment.
I think we need to work on the abort semantics (comment inline).
| - Trigger a debugger break before termination (implementation-defined) | ||
|
|
||
| The exact behavior is implementation-defined, but the shader must not continue | ||
| execution past this point. This matches the semantics of C's `abort()` function. |
There was a problem hiding this comment.
I think that a proper abort should force termination of the thread immediately, and should be specified to terminate all threads in a dispatch "eventually". The tricky thing about "eventually" being that it would be legal to terminate the thread that calls abort and allow all other threads to terminate naturally by completing execution.
There was a problem hiding this comment.
At the language level terminating the thread (wave?) is about all we can do right? What happens after that is a runtime issue? i.e. if it should always result in a TDR for example.
|
|
||
| ```hlsl | ||
| void DebugBreak(); // Trigger a breakpoint if debugger attached | ||
| void Abort(); // Terminate execution abnormally |
There was a problem hiding this comment.
While most HLSL functions are CamelCase the ones that have direct C equivalents (like the math functions) we tend to match the C spellings. So I think abort() would be preferable.
@tex3d thoughts?
There was a problem hiding this comment.
FastFail might be alternative as well. I don't think that the actual semantics we'll end up with will really exactly match those of C's abort().
There was a problem hiding this comment.
It would be nice if we can repurpose the existing FXC abort since it matches the C function in name at least
| 4. **Per-Pipeline Control**: Enable assertions only for specific shaders that | ||
| are suspected of containing bugs, minimizing performance impact. | ||
|
|
||
| #### Backend Compiler Optimization |
There was a problem hiding this comment.
I don't think this should ever be allowed.
There was a problem hiding this comment.
For some more context, we've gotten feedback from game developers that it'll be nice to have a way to enable and disable asserts/aborts at PSO creation time so that they can enable validation without needing to recompile all their shaders. That way, they can ship with aborts in their shader disabled by default, and if they see a spike in GPU crashes they can temporarily enable the aborts for a small percentage of users and trigger a dump or additional logging for offline analysis
There was a problem hiding this comment.
I'm not sure that complexity is warranted at the DXIL level. A root constant could be used to enable or disable custom assert macros if a specific software vendor needs such a case. Ideally this is a use case for specialization constants, but since we don't have those in DXIL it isn't really viable, but increasing the complexity and testing matrix for a DXIL operation by allowing it to behave differently under different runtime state is sub-optimal and should be avoided.
There are additional reasons why I'm opposed to this which cannot be discussed publicly at this time, but I'm happy to discuss privately.
| ### D3D12 Runtime Behavior for Abort | ||
|
|
||
| The SPIRV usage will utilize the following instructions: | ||
| #### Default Behavior: Abort as No-Op |
There was a problem hiding this comment.
Per my other comment, I'm pretty strongly opposed to this being allowed.
JoeCitizen
changed the title
| The value returned is uniform across all threads in a dispatch/draw and remains | ||
| constant for the duration of shader execution. | ||
|
|
||
| #### Enabling Semi-Optimized Development Builds |
There was a problem hiding this comment.
I don't know that this section really needs to be here. HLSL specs aren't user documentation, they're for compiler authors to implement language features.
| #### `IsDebuggerPresent()` | ||
|
|
||
| ```hlsl | ||
| bool IsDebuggerPresent(); |
There was a problem hiding this comment.
This will need to be in the dx namespace as there is no Vulkan equivalent that we can lower to for SPIRV generation.
| no debugger is present, they are required supported features and do not require | ||
| capability bits. | ||
|
|
||
| ### D3D12 Runtime Behavior for DebugBreak |
There was a problem hiding this comment.
I would prefer to remove D3D-focused language from the HLSL specifications. This isn't where we want users looking for documentation.
| - Lower to a specialization constant that can be set by the debugger | ||
| - Use a vendor-specific extension | ||
| - Return a conservative value (e.g., always `false` in release, always `true` | ||
| in debug builds) |
There was a problem hiding this comment.
I commented this above, but it seems more appropriate that we put this function in the dx namespace and make it unavailable to Vulkan targets if it has no lowering. There is no vendor extension equivalent that I'm aware of, and using a specialization constant isn't really something the compiler can just do it would require user involvement.
| By default, when no debugger is attached, `DebugBreak()` is treated as a no-op | ||
| i.e the `dx.op.debugBreak` operation has no effect at runtime. | ||
|
|
||
| The allows for driver back end compilers to optimize away these instructions |
There was a problem hiding this comment.
The D3D spec currently has the default as no-op such that the backend compiler can optimize away instructions that lead up to the debug break and even if a debugger was attached after PSO creation the debugbreak would still be a no-op, we still want the three debugbreak configuration options (no-op, debugbreak if debugger else no-op, and abort/trigger TDR), is that right?
JoeCitizen
changed the title
| void main(uint GI : SV_GroupIndex) { | ||
| assert(GI < 8); | ||
| // Conditional expensive debug checks | ||
| if (dx::IsDebuggerPresent()) { |
There was a problem hiding this comment.
nit:
| if (dx::IsDebuggerPresent()) { | |
| if (dx::IsDebuggerPresent()) { |
5 participants
HLSL Debugging Intrinsics
Broadened Proposal Scope: The proposal now covers two new debugging intrinsics:
DebugBreak(): Triggers a breakpoint when a debugger is attached.dx::IsDebuggerPresent(): Returns whether a graphics debugger is currently attached (DirectX only).Motivation Expanded: Highlights the need for conditional debug checks and selective breakpoints, allowing shader authors to perform expensive debugging operations only when needed.
Detailed API & Usage Examples:
Implementation Details Added:
dx.op.debugBreak,dx.op.isDebuggerPresent).SPIR-V Targeting:
NonSemantic.DebugBreakforDebugBreak()in SPIR-V.dx::IsDebuggerPresent()has no SPIR-V equivalent.Testing Section: Introduces compiler, validation, and execution testing requirements to ensure correct integration and behavior.