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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
