search

microsoft / OpenCLOn12

The OpenCL-on-D3D12 mapping layer
MIT License
104 stars 13 forks source link

DebugBreak is enabled in (store) release version #17

Closed MarkIngramUK closed 3 years ago

MarkIngramUK commented 3 years ago

I have the latest store version installed at the time of writing (version 0.2102.8.0), and when I make use of the OpenCLOn12 provider, I encounter wil::details::DebugBreak during debug sessions.

e.g. here is a call to clBuildProgram (the call itself succeeds):

image

Then after that, a non-application thread continually asserts:

image

jenatali commented 3 years ago

That would be this one: https://github.com/microsoft/OpenCLOn12/blob/master/src/program.cpp#L73, which means that we're producing DXIL that doesn't validate.

Any chance you'd be able to share the program(s) that trigger this so we can debug?

MarkIngramUK commented 3 years ago

I'd need to share it privately, is that possible?

jenatali commented 3 years ago

Yes, you can send it to me at jenatali@microsoft.com.

MarkIngramUK commented 3 years ago

Looking at the kernel, I imagine it's because we're making use of cl_khr_byte_addressable_store, as I know OpenCLOn12 doesn't support extensions right now. I'll send the kernel regardless, in case there is more to it than that.

jenatali commented 3 years ago

That one might actually be free for us to flip on, so yes please still send the kernel.

As a requirement for supporting packed structures, we already have to support unaligned reads/writes, so as long as Clang/LLVM is able to figure out that they're unaligned and embed that information in the resulting SPIR-V, our compiler stack should emit the necessary code to handle unaligned reads/writes.

jenatali commented 3 years ago

Oh, nevermind. https://www.khronos.org/registry/OpenCL/specs/3.0-unified/html/OpenCL_Ext.html#cl_khr_byte_addressable_store says that this is a core 1.1 feature, so yes we do support it, since we've implemented 1.2.

MarkIngramUK commented 3 years ago

Ah OK - I missed that. We take a hard dependency on OpenCL 1.1, so that's good. I'm continuing to trim the kernel down to a minimum repro that triggers the asserts. As soon as I'm there I'll send it over.

jenatali commented 3 years ago

Hm... I'm not reproducing on the latest compiler, which could very well mean that it's already been fixed. Let me see if I can reproduce using the compiler from your exact package.

MarkIngramUK commented 3 years ago

Forgot to mention that we also send the following compiler flags: -cl-single-precision-constant -cl-fast-relaxed-math

jenatali commented 3 years ago

Nope, that's not tripping it up either...

jenatali commented 3 years ago

Ok, I found it. This was fixed by https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9464. I'll see about pushing out a new application package with that fix.

jenatali commented 3 years ago

A new 0.2104.2.0 package should be live any minute now. Please close the issue if that resolves it for you, and continue to report any further issues you find.

MarkIngramUK commented 3 years ago

A new 0.2104.2.0 package should be live any minute now. Please close the issue if that resolves it for you, and continue to report any further issues you find.

I've just updated to 0.2104.2.0 and I can confirm that I'm no longer hitting the DebugBreak (and release builds are no longer crashing).