Metal Shader Converter thread safety

Hello Apple!

We've got offline shader compilation from HLSL -> Metallib using DXC -> SPIR-V -> metal.exe. This works okay for the most part, but it requires the creation of intermediate files to pass to/from the metal.exe process and we've had some issues with metal.exe sometimes not launching (probably our fault).

Then we noticed Metal Shader Converter (MSC) exists and has a DLL - this looks way better since there's no need to launch processes or store intermediate files. However, upon trying to replace metal.exe with it I quickly ran into rampant heap corruption. I was surprised because the docs claim this:

Each thread in your program needs to create its own instance of IRCompiler to avoid race conditions.

But once I start calling IRCompilerAllocCompileAndLink in parallel all hell breaks loose, whether or not each thread has its own IRCompiler.

I figured I must be doing something wrong, so I removed my attempt and compiled DXC locally with the MSC integration and encountered the exact same heap corruption. So I'm inclined to think the library isn't actually thread safe, but I'm wondering if there's something I'm missing?

I tried all 3 versions of MSC just in case it was a problem with 3.0, but I got the same result each time. The only way to make it work was to surround compilation with a mutex, which makes its use pointless in our case.

Answered by DTS Engineer in 901525022

The paragraph you are quoting constrains one object. It says the IRCompiler instance is not reentrant and that each thread needs its own. It also opens by stating that multithreading the IR translation process is supported. So your reading is right, and doing what it asks should have been enough.

You have already eliminated a great deal, across three versions and with a compiler per thread, so the question becomes what else your threads share. The C interface sample on the Metal shader converter page (https://developer.apple.com/metal/shader-converter/) creates every object per invocation. That covers the compiler, the input IRObject, the output IRObject, and the IRMetalLibBinary. If each of your threads does the same, one thing is still potentially shared:

IRObject* pDXIL = IRObjectCreateFromDXIL(bytecode, size, IRBytecodeOwnershipNone);

IRBytecodeOwnershipNone means the library does not take ownership of bytecode, so it holds a pointer into your memory for the lifetime of that IRObject. If several threads supply the same bytecode pointer, that buffer is the one piece of shared state the documented pattern leaves.

One thing worth trying before you commit to the mutex: give each thread its own copy of the input bytecode. If the corruption stops, you have a fix that costs a memcpy rather than serializing your whole shader build. If it persists, you have eliminated the only shared state in the documented pattern. That is worth putting into a Feedback Assistant report (https://feedbackassistant.apple.com), along with your threading setup and the corruption stack. Noting whether each thread had its own compiler, and whether the input buffers were shared or copied, avoids a follow-up question.

The version history on that page also lists a 3.1 between the 3 and the 4.0 beta. You mentioned versions 1, 2, and 3, so there is a released version you may not have tried yet.

The paragraph you are quoting constrains one object. It says the IRCompiler instance is not reentrant and that each thread needs its own. It also opens by stating that multithreading the IR translation process is supported. So your reading is right, and doing what it asks should have been enough.

You have already eliminated a great deal, across three versions and with a compiler per thread, so the question becomes what else your threads share. The C interface sample on the Metal shader converter page (https://developer.apple.com/metal/shader-converter/) creates every object per invocation. That covers the compiler, the input IRObject, the output IRObject, and the IRMetalLibBinary. If each of your threads does the same, one thing is still potentially shared:

IRObject* pDXIL = IRObjectCreateFromDXIL(bytecode, size, IRBytecodeOwnershipNone);

IRBytecodeOwnershipNone means the library does not take ownership of bytecode, so it holds a pointer into your memory for the lifetime of that IRObject. If several threads supply the same bytecode pointer, that buffer is the one piece of shared state the documented pattern leaves.

One thing worth trying before you commit to the mutex: give each thread its own copy of the input bytecode. If the corruption stops, you have a fix that costs a memcpy rather than serializing your whole shader build. If it persists, you have eliminated the only shared state in the documented pattern. That is worth putting into a Feedback Assistant report (https://feedbackassistant.apple.com), along with your threading setup and the corruption stack. Noting whether each thread had its own compiler, and whether the input buffers were shared or copied, avoids a follow-up question.

The version history on that page also lists a 3.1 between the 3 and the 4.0 beta. You mentioned versions 1, 2, and 3, so there is a released version you may not have tried yet.

Thanks for replying!

I've used that C example as my guide, which means I create every object per invocation. As for the bytecode, DXC compiles the HLSL in each invocation, so the DXIL bytecode fed to IRObjectCreateFromDXIL isn't shared either. The DXC implementation of this also looks nearly identical to the example C code in the manual.

The crashes I encounter tend to originate from irmetalirconverter.dll in some of the allocation functions like RtlFreeHeap() or RtlAllocateHeap(), so maybe there's an unsafe allocator or something.

Interestingly, I am able to catch the issue with gflags page heap. This double free happens deep within the irmetalirconverter.dll call stack during the call to IRCompilerAllocCompileAndLink:

VERIFIER STOP 0000000000000007: pid 0x8EC0: block already freed 

	0000024E535A1000 : Heap handle
	0000024E6016CFE0 : Heap block
	0000000000000000 : Block size
	0000000000000000 : 

I will make a note to submit this to the feedback assistant, as per your guidance.

Metal Shader Converter thread safety
 
 
Q