[PATCH] D144033: [AMDGPU][MC][GFX11] Add partial NSA format for image sample instructions

Tue Feb 21 08:06:58 PST 2023

mbrkusanin marked an inline comment as done.
mbrkusanin added inline comments.

================
Comment at: llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:10
 #include "AMDKernelCodeT.h"
+#include "GCNSubtarget.h"
 #include "MCTargetDesc/AMDGPUMCTargetDesc.h"
----------------
foad wrote:
> I think the idea is that the assembler should not include any of the "codegen" parts of the backend. If getNSAMaxSize needs to be shared by codegen and the assembler, it should be moved into AMDGPUBaseInfo - that is why AMDGPUBaseInfo exists.
Moved to AMDGPUBaseInfo.

================
Comment at: llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp:910
       if (AddrSize > Info->VAddrDwords) {
-        // The NSA encoding does not contain enough operands for the combination
-        // of base opcode / dimension. Should this be an error?
-        return MCDisassembler::Success;
+        if (!isGFX11Plus()) {
+          // The NSA encoding does not contain enough operands for the
----------------
foad wrote:
> This should check `!hasPartialNSAEncoding`?
Sure, it can.
Also changed isGFX10Plus() to STI.hasFeature(AMDGPU::FeatureNSAEncoding) below.

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

https://reviews.llvm.org/D144033