D92434: AMDGPU code object V4 ABI -- docs don't quite match the source tree state.

Fri Jan 8 03:16:53 PST 2021

[AMD Public Use]

Hi Artem,
I believe the documentation is correct in its description of the code object v3, which is what is currently upstream. In addition, it descries code object v4 which is what will be pushed for review shortly. The plan had been to upstream all the changes together, but the holiday break messed that up. Konstantin is planning to push the remaining reviews in the next few days.

Thanks,
-Tony

From: Artem Belevich <tra at google.com>
Sent: Thursday, January 7, 2021 6:46 PM
To: Tye, Tony <Tony.Tye at amd.com>; Liu, Yaxun (Sam) <Yaxun.Liu at amd.com>
Cc: llvm-commits <llvm-commits at lists.llvm.org>
Subject: Re: D92434: AMDGPU code object V4 ABI -- docs don't quite match the source tree state.

[CAUTION: External Email]
+CC: Yaxun Liu.

On Thu, Jan 7, 2021 at 12:22 PM Artem Belevich <tra at google.com<mailto:tra at google.com>> wrote:
Hi,

It appears that the AMDGPU documentation updated in
https://reviews.llvm.org/D92434<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD92434&data=04%7C01%7CTony.Tye%40amd.com%7C2265e4f613b442c6062708d8b36661d7%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637456599612355955%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=FO9LIoZOlZsCr2uC4%2Bpm8kPFqoIdjZ0BFAjoCfBvURs%3D&reserved=0> does not quite match the code we currently have in the LLVM source tree.

It's not very helpful when the recently-updated public docs say one thing, but the code does something completely different.

E.g. I'm failing to find ELF::EF_AMDGPU_FEATURE_XNACK_V2 and other new flags it describes. At the same time, the flags that were removed from the documentation are still there (e.g EF_AMDGPU_XNACK)
https://github.com/llvm/llvm-project/blob/4e2e785ddd35c421a4df9453f17b8317d6b62b2c/llvm/include/llvm/BinaryFormat/ELF.h#L738<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fllvm%2Fllvm-project%2Fblob%2F4e2e785ddd35c421a4df9453f17b8317d6b62b2c%2Fllvm%2Finclude%2Fllvm%2FBinaryFormat%2FELF.h%23L738&data=04%7C01%7CTony.Tye%40amd.com%7C2265e4f613b442c6062708d8b36661d7%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637456599612355955%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=wiTzUahz6UfdWy71BZwBKcnVF%2BYP%2BOwY53tpH8IFkBY%3D&reserved=0>

AFAICT, the current state of the tree matches V3 ABI, but uses different constant names than the ones used in the docs.

Are the corresponding code changes handled in a separate review? If so, I'd appreciate it if you could point me in the right direction and, maybe, update the review tracker, too.

AFAICT, the doc changes that landed in LLVM are part of this change in AMD's fork of the LLVM project:
https://github.com/ROCm-Developer-Tools/amd-llvm-project/commit/1ccc5cc50e77bc626c09043f0709dc0049b2780b<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FROCm-Developer-Tools%2Famd-llvm-project%2Fcommit%2F1ccc5cc50e77bc626c09043f0709dc0049b2780b&data=04%7C01%7CTony.Tye%40amd.com%7C2265e4f613b442c6062708d8b36661d7%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637456599612365952%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=uMIym81kBXVJimd%2BKwbcmejcLelI%2FzoKzfYdUosPm0o%3D&reserved=0>

I assume the changes will eventually be upstreamed into LLVM. Is there an ETA for that?

The reason I'm asking is that I need to build AMD's comgr (https://github.com/RadeonOpenCompute/ROCm-CompilerSupport<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FRadeonOpenCompute%2FROCm-CompilerSupport&data=04%7C01%7CTony.Tye%40amd.com%7C2265e4f613b442c6062708d8b36661d7%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637456599612365952%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=uoz0iifXoLGjdowxeJSQBXQs0WZ%2BDRporUpZjIweWts%3D&reserved=0>) with LLVM from the LLVM's tree. An old version of comgr (3.7) is broken because LLVM has recently changed some ELF-related APIs, and the most recent version of comgr does not build because it needs the ELF changes mentioned above that are not in the LLVM yet.  I can probably build comgr 4.0, which still uses the current version of LLVM's ELF constants, but it will be broken again once AMD's ELF changes do make it into LLVM. If the changes are going to land soon, I may as well wait until both comgr and LLVM are in sync, again.

--Artem

If the code is not quite ready yet, perhaps it would make sense to postpone doc updates (at least the parts that remove the info about the *current* state of the source tree) until the code is ready and clearly mark the parts of the documentation that describe things that do not exist yet.

Thank you,
--Artem Belevich

--
--Artem Belevich
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210108/416df197/attachment.html>