This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Add more PAL metadata register names
ClosedPublic

Authored by foad on Jan 28 2021, 7:44 AM.

Details

Summary

Add all the registers that are currently used by
LLPC: https://github.com/GPUOpen-Drivers/llpc

This only affects disassembly of PAL metadata generated by LLPC and
similar frontends.

Diff Detail

Event Timeline

foad created this revision.Jan 28 2021, 7:44 AM
foad requested review of this revision.Jan 28 2021, 7:44 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 28 2021, 7:44 AM
foad added a comment.Jan 28 2021, 7:45 AM

I couldn't work out what order the existing entries were in, so I added the new ones in alphabetical order at the end.

I couldn't find any existing test cases for these names.

foad updated this revision to Diff 320136.Jan 29 2021, 8:07 AM

Add tests.

We have slight problems when it comes to differences between hardware versions.
COMPUTE_USER_ACCUM_1 on gfx9 has the same number as COMPUTE_SHADER_CHKSUM on gfx10.
Also, some have slightly different names. E.g. IA_MULTI_VGT_PARAM for gfx9 is called IA_MULTI_VGT_PARAM_PIPED for gfx10.

foad added a comment.Jan 29 2021, 10:33 AM

COMPUTE_USER_ACCUM_1 on gfx9 has the same number as COMPUTE_SHADER_CHKSUM on gfx10.

Luckily LLPC only uses COMPUTE_USER_ACCUM_1 on gfx10+.

Also, some have slightly different names. E.g. IA_MULTI_VGT_PARAM for gfx9 is called IA_MULTI_VGT_PARAM_PIPED for gfx10.

I had not noticed that.

foad added a comment.Feb 24 2021, 3:16 AM

We have slight problems when it comes to differences between hardware versions.
COMPUTE_USER_ACCUM_1 on gfx9 has the same number as COMPUTE_SHADER_CHKSUM on gfx10.
Also, some have slightly different names. E.g. IA_MULTI_VGT_PARAM for gfx9 is called IA_MULTI_VGT_PARAM_PIPED for gfx10.

Ping! Do you think it's OK to commit despite these concerns?

Flakebi accepted this revision.Feb 24 2021, 5:30 AM

If these registers keep changing, we probably want to switch between architectures at some point. But it’s easier to look at (slightly off) names than at hex numbers, so LGTM.

This revision is now accepted and ready to land.Feb 24 2021, 5:30 AM
This revision was landed with ongoing or failed builds.Feb 24 2021, 5:37 AM
This revision was automatically updated to reflect the committed changes.

I am not familiar with graphics side much, but is there any reason to not use COMPUTE_PGM_RSRC* registers from https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/Support/AMDHSAKernelDescriptor.h#L130 ?

foad added a comment.Feb 24 2021, 9:12 AM

I am not familiar with graphics side much, but is there any reason to not use COMPUTE_PGM_RSRC* registers from https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/Support/AMDHSAKernelDescriptor.h#L130 ?

For dumping PAL metadata all we want is the offsets of the registers. AMDHSAKernelDescriptor.h seems to define the fields within some of these registers instead. Obviously it's related, but I don't think there's actually any duplicated information there.