[PATCH] D146023: [AMDGPU] Remove Code Object V2

[Pierre van Houtryve via Phabricator via llvm-commits]

Pierre-vh added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUHSAMetadataStreamer.cpp:520
 
-  assert(Func.getCallingConv() == CallingConv::AMDGPU_KERNEL ||
-         Func.getCallingConv() == CallingConv::SPIR_KERNEL);
+  if (Func.getCallingConv() != CallingConv::AMDGPU_KERNEL &&
+      Func.getCallingConv() != CallingConv::SPIR_KERNEL)
----------------
scott.linder wrote:
> I don't follow this change; was the assert just incorrect previously?
It's for `CodeGen/AMDGPU/no-hsa-graphics-shaders.ll`, it crashes otherwise.

An alternative can be to change this:
```
  if (STM.isAmdHsaOS())
    HSAMetadataStream->emitKernel(*MF, CurrentProgramInfo);
```
So it checks the CC and doesn't call the function if the CC is incorrect. I don't mind either solution


================
Comment at: llvm/test/MC/AMDGPU/hsa-gfx10.s:3
-// RUN: llvm-mc -filetype=obj -triple amdgcn--amdhsa -mcpu=gfx1010 --amdhsa-code-object-version=2 -mattr=-wavefrontsize32,+wavefrontsize64 -show-encoding %s | llvm-readobj -S --sd --syms - | FileCheck %s --check-prefix=ELF
-
-// ELF: Section {
----------------
scott.linder wrote:
> Pierre-vh wrote:
> > arsenm wrote:
> > > I thought we were still going to be able to read old objects 
> > I think llvm-readobj uses all of the MC/Target infrastructure so if we remove emission, we also remove reading, no?
> > 
> > I'm actually not sure if we plan to let readobj/readelf read COV2 object files, it's an interesting question
> I think this is my biggest concern. Do we incur a huge maintenance burden that warrants dropping read support? How much code do we really need to maintain to keep the readobj/objdump like tools universal?
> 
> @t-tye do you have any thoughts on whether we should maintain backwards compatibility in the LLVM tooling, even if we drop generation support?
It's been a while since I wrote this but IIRC there was a discussion about it and it was fine to remove read support. An alternative may be to still identify code object V2, but not read the metadata and instead print a warning about the file format being deprecated?

Or I think it's YAML, maybe we can just raw dump the MD and print a warning?

Most of the maintenance cost would be in the MD mapper which is almost 500 lines of code that'd just be there for the sake of "maybe some needs to read MD"

If we go with one of the above suggestions I can just add a test using yml2obj that emits a V2 file


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146023/new/

https://reviews.llvm.org/D146023