[PATCH] D97670: [AMDGPU] New intrinsic void llvm.amdgcn.s.sethalt(i32)

[Jay Foad via Phabricator via llvm-commits]

foad added a comment.

In D97670#2599025 <https://reviews.llvm.org/D97670#2599025>, @t-tye wrote:

> What other debuggers do you have in mind?

Windbg and umr.

> If non-HSA targets would like to support a debugger it would make sense that the also add support for llvm.debugtrap as well:-)

Makes sense to me. It seems like the lowering of llvm.debugtrap to s_trap is deliberately restricted to HSA, but I don't know the history of that.

> Also, does this patch account for the hardware hazard that you must not have an s_endpgm following an s_halt on asics that do not support it? One approach to avoid the hazard is to always put a s_nop after an s_halt.

No I was not aware of that. I'll see what I can do...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97670/new/

https://reviews.llvm.org/D97670