[PATCH] D105507: AMDGPU: Add gfx10 assembler directive to specify shared VGPR count

[Konstantin Zhuravlyov via Phabricator via llvm-commits]

kzhuravl marked an inline comment as done.
kzhuravl added a comment.

In D105507#2860701 <https://reviews.llvm.org/D105507#2860701>, @rampitec wrote:

> Also needs disassembler change.

Disassembler support is already here, because the asm printing is done `AMDGPUTargetAsmStreamer`.



================
Comment at: llvm/docs/AMDGPUUsage.rst:12110
                                                                                                :ref:`amdgpu-amdhsa-compute_pgm_rsrc1-gfx6-gfx10-table`.
+     ``.amdhsa_shared_vgpr_count``                            0                   GFX10        Controls SHARED_VGPR_COUNT in
+                                                                                               :ref:`amdgpu-amdhsa-compute_pgm_rsrc3-gfx10-table`.
----------------
rampitec wrote:
> Should probably specify it is GFX10 only.
It already says "GFX10" in one of the columns.


================
Comment at: llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:4769
+                       COMPUTE_PGM_RSRC3_GFX10_SHARED_VGPR_COUNT,
+                       Val, ValRange);
     } else if (ID == ".amdhsa_exception_fp_ieee_invalid_op") {
----------------
t-tye wrote:
> How does the value relate to the total VGPR count? Are these in addition, or this just indicates how many of the VGPRs are shared? If the later, is there an error for when this value exceeds the requested VGPR count?
> 
> How does new_vgpr work? Is it agnostic to which VGPRs are private and shared? Seems it probably is so that present no issues with adding shared VGPRs but wanted to check.
Will post a follow up patch shortly.


================
Comment at: llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUTargetStreamer.cpp:437
                 amdhsa::COMPUTE_PGM_RSRC1_FWD_PROGRESS);
+    PRINT_FIELD(OS, ".amdhsa_shared_vgpr_count", KD,
+                compute_pgm_rsrc3,
----------------
rampitec wrote:
> Only print it if IVersion.Major >= 10.
It is already the case. See line 427 in this patch set.


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

https://reviews.llvm.org/D105507