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

[Artem Belevich via llvm-commits]

On Fri, Jan 8, 2021 at 3:16 AM Tye, Tony <Tony.Tye at amd.com> wrote:

> [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.
>

SGTM, and thank you for the update.

--Artem



>
>
> 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> 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
>


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