Page MenuHomePhabricator

Add AMDPAL Code Conventions section to AMD docs
Needs ReviewPublic

Authored by timcorringham on Apr 4 2018, 3:50 AM.

Details

Summary

This is a first version of the AMDPAL code conventions.
Further updates will undoubtably be required to fully
document AMDPAL.

Diff Detail

Event Timeline

timcorringham created this revision.Apr 4 2018, 3:50 AM
nhaehnle accepted this revision.Apr 4 2018, 4:27 AM

Thanks, that should be useful. One typo :)

docs/AMDGPUUsage.rst
3799 ↗(On Diff #140924)

Typo: dispatch

This revision is now accepted and ready to land.Apr 4 2018, 4:27 AM
This revision was automatically updated to reflect the committed changes.
t-tye reopened this revision.Apr 4 2018, 5:31 PM
t-tye added inline comments.
llvm/trunk/docs/AMDGPUUsage.rst
3797 ↗(On Diff #140952)

Do all generations support 32 user SGPRs or was this increased to 32 in GFX9? Do compute shaders still only support 16 use SGPRs?

3816 ↗(On Diff #140952)

Are these SGPR register numbers? If so suggest changing header to say that.

3823 ↗(On Diff #140952)

Could this be defined?

3838–3844 ↗(On Diff #140952)

Need to remove the "+" as that is making a bulleted list. I think this is what you want which puts a list of the possible values in the 1-15 table row:

============= ================================
User Register Description
============= ================================
0             :ref:`amdpal_global_internal_table` (32-bit pointer)
1-15          Application Controlled User Data
              (1-15 Contiguous 32-bit Values in Registers)

              - Per-Shader Internal Table (32-bit pointer)
              - Spill Table (32-bit pointer)
              - Draw Index (First Stage Only)
              - Vertex Offset (First Stage Only)
              - Instance Offset (First Stage Only)
============= ================================
3842–3844 ↗(On Diff #140952)

Could these be defined?

3864 ↗(On Diff #140952)

To be consistent with the section name:

.. _amdpal_global_internal_table:
3902 ↗(On Diff #140952)

Could there be sections to define the other structures mentioned?

  • Per-Shader Internal Table (32-bit pointer)
  • Spill Table (32-bit pointer)
3902 ↗(On Diff #140952)

Could there be a section to define the metadata and how it specifies how these structures are set up?

This revision is now accepted and ready to land.Apr 4 2018, 5:31 PM
t-tye requested changes to this revision.Apr 12 2018, 10:02 PM
This revision now requires changes to proceed.Apr 12 2018, 10:02 PM
This revision was not accepted when it landed; it landed in state Needs Revision.Oct 7 2019, 4:33 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptOct 7 2019, 4:33 AM
Herald added a subscriber: jvesely. · View Herald Transcript
t-tye reopened this revision.Oct 7 2019, 8:32 AM
t-tye added inline comments.
llvm/docs/AMDGPUUsage.rst
3808–3810

Clarify that this is User SGPR Registers. Also should this define the System SGPR and VGPR Registers.

3816

User SGPR Registers

3829

Add System SGPR and VGPR mapping information.

3835

User SGPR Registers

3836–3845

Need to remove the "+" as that is making a bulleted list. I think this is what you want which puts a list of the possible values in the 1-15 table row:

User Register Description

0 :ref:amdpal_global_internal_table (32-bit pointer)
1-15 Application Controlled User Data

(1-15 Contiguous 32-bit Values in Registers)

- Per-Shader Internal Table (32-bit pointer)
- Spill Table (32-bit pointer)
- Draw Index (First Stage Only)
- Vertex Offset (First Stage Only)
- Instance Offset (First Stage Only)

Define these fields and how the metadata sets them up.

3858–3862

Define how the mapping and re-mapping is conveyed and what the rules are. From tjis description I know there is a mapping but am unclear on how it is expressed.

3864

To be consistent with the section name:

.. _amdpal_global_internal_table:

3870

Where is the concept of an "engine" defined? Is this just another term for the PAL runtime? If so I would stick with saying PAL runtime. I would clarify use of runtime with PAL Runtime since the higher levels also have runtimes.

3872

Would be helpful to reference where the target hardware format is defined.

3879–3896

Add sections to define the other structures mentioned:

Per-Shader Internal Table (32-bit pointer)
Spill Table (32-bit pointer)

Section to define the metadata and how it specifies how these structures are set up.

3898–3901

Suggest being more explicit. What does "top 32 bits should be assumed to be the same as

the top 32 bits of the pipeline" mean? Presumably it is the loaded PC address of the code for the shader pipeline.