[systems-assistant]

Prior to this, RTCProgram was using is_version = true and device initialization was using false. On Windows, this was causing us to attempt to load amd_comgr_3.dll at device load and amd_comgr0605.dll for RTC programs. Obviously, there is only one DLL and it must be used consistently.

See ROCm/clr#162

---

🔁 Imported from ROCm/clr#243
🧑‍💻 Originally authored by @rocm-devops

[mangupta]

@zichguan-amd : There is a reason why the 2 different versions of the DLL exist. And there are situations in which we need to load amd_comgr_3 and in some cases we need to load amd_comgr_XXYY.dll. Can you provide a reproducer or example where you see the mismatch in loading so that we can solve this the right way?

[zichguan-amd]

@stellaraccident can you address the reviewer's comment?

[stellaraccident]

@zichguan-amd : There is a reason why the 2 different versions of the DLL exist. And there are situations in which we need to load amd_comgr_3 and in some cases we need to load amd_comgr_XXYY.dll. Can you provide a reproducer or example where you see the mismatch in loading so that we can solve this the right way?

What is the reason? I don't believe it applies to new builds with TheRock: those always have the one DLL version. I know there are various games played with prior setups based on the Adrenaline driver, etc. But none of those are public and are a little hard to comment on. The default build of TheRock on windows has to include this patch. Without it, the naming of comgr that gets compiled does not match the naming of what is attempted to load.

There may be something else mis-aligned there, but it is the same path as on Linux and I think the older integrations have a weird expectation here.

[mangupta]

What is the reason? I don't believe it applies to new builds with TheRock: those always have the one DLL version. I know there are various games played with prior setups based on the Adrenaline driver, etc. But none of those are public and are a little hard to comment on. The default build of TheRock on windows has to include this patch. Without it, the naming of comgr that gets compiled does not match the naming of what is attempted to load.

There may be something else mis-aligned there, but it is the same path as on Linux and I think the older integrations have a weird expectation here.

Similar to nvrtc, ROCm provides hipRTC which lets applications to compile and execute on the fly. However this requires applications to bundle or embed their GPU kernel source as part of their applications, which is not acceptable for customers who have sensitive GPU kernel code.
To workaround this, we provide a bitcode linking mode wherein customers use certain ROCm llvm toolchain to generate LLVM IR based binaries which hipRTC will accept and link+execute on the fly. The restriction of this approach is that LLVM IR is not guaranteed to be backward compatible. So both application developers and the end users of the applications using bitcode linking need to use the same comgr+hipRTC versions. Note that such applications cannot benefit from backward compatibility support from AMD.
Hence we version comgr and hipRTC shared libraries so that application link to the specific version (example hiprtc0604.dll and amd_comgr0604.dll) instead of the generic version (example hiprtc.dll and amd_comgr_3.dll). However HIP applications that do not use hipRTC would need to link to the generic version so that they can benefit from the backward compatibility support from AMD.
Consequently we need both hiprtc.dll and hiprtcXXYY.dll and the corresponding comgr dlls for the two different use cases.

[stellaraccident]

Thanks. We will need to discuss more on Monday. We're mixing a lot of requirements, and I'm pretty sure that there is an actual problem here.

I'd also like to understand more about the bytecode compatibility. Something there isn't matching my mental model of how I've seen other setups work.

[lamb-j]

In the near future (i.e. for ROCm 8), we're aiming to switch back to using amd_comgr.dll everywhere.

We can differentiate version via DLL metadata and amd_comgr_version() APIs. And we can isolate DLLs via SxS assemblies.

For now though, we can land this for now to stop carrying TheRock patch, assuming this doesn't immediately break our Drivers builds of CLR, which haven't been operating with this change (as they don't use TheRock)

[lamb-j]

On second thought, we should make sure we test with a Drivers build first

[github-actions]

This pull request has been inactive for 25 days and will be marked as stale.

If you would like to keep this PR open, please:

• Add new commits
• Add a comment explaining why it should remain open

This PR will be automatically closed in 5 days if no further activity occurs.

[lamb-j]

This is effectively fixed with #4211

Still waiting on the next compiler promotions to remove the effective flag-gating in TheRock: ROCm/TheRock#4730