[AMDGPU] Allow oversize vaddr in GFX10 MIMG assembly

[critson]

Summary

As a follow up to D103672, we should allow vaddr to be larger than
required when assembling GFX10 MIMG instructions.

[foad]

I think this needs some rationale in a comment explaining why the assembler is tolerant in this particular way.

[dp]

llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

3457

To preserve backward compatibility with existing code it would be sufficient to allow size=8 when 5 is required. I'm not sure if we need to enable other cases.

As a minor note I think the code would be more readable if we do the following change:

VAddrSize -> ActualAddrSize
AddrSize -> ExpectedAddrSize

[critson]

• Address reviewer comments

[dp]

LGTM

llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

bool AMDGPUAsmParser::validateMIMGAddrSize(const MCInst &Inst) {
   unsigned VAddrSize =
       IsNSA ? SrsrcIdx - VAddr0Idx
             : AMDGPU::getRegOperandSize(getMRI(), Desc, VAddr0Idx) / 4;
   bool IsA16 = (A16Idx != -1 && Inst.getOperand(A16Idx).getImm());
 
   unsigned AddrSize =
       AMDGPU::getAddrSizeMIMGOp(BaseOpcode, DimInfo, IsA16, hasG16());
 
-  if (!IsNSA) {
-    if (AddrSize > 8)
-      AddrSize = 16;
-    else if (AddrSize > 5)
-      AddrSize = 8;
-  }
+  if (IsNSA)
+    return VAddrSize == AddrSize;
+
 
+  if (VAddrSize <= AddrSize)
     return VAddrSize == AddrSize;
+
+  // vaddr maybe oversize if it is one of the defined size classes
+  switch (VAddrSize) {
+  case 1:
+  case 2:
+  case 3:
+  case 4:
+  case 5:
+  case 8:
+  case 16:
+    return true;
+  default:
+    return false;
+  }
 }
 
 bool AMDGPUAsmParser::validateMIMGAtomicDMask(const MCInst &Inst) {
 
   const unsigned Opc = Inst.getOpcode();
   const MCInstrDesc &Desc = MII.get(Opc);
 
   if ((Desc.TSFlags & SIInstrFlags::MIMG) == 0)

llvm/test/MC/AMDGPU/gfx10_asm_mimg.s

 image_sample_c_cd_cl_g16 v[0:3], [v0, v1, v3, v5, v6, v7], s[0:7], s[8:11] dmask:0xf dim:SQ_RSRC_IMG_2D
 ; GFX10: image_sample_c_cd_cl_g16 v[0:3], [v0, v1, v3, v5, v6, v7], s[0:7], s[8:11] dmask:0xf dim:SQ_RSRC_IMG_2D ; encoding: [0x0d,0x0f,0xac,0xf1,0x00,0x00,0x40,0x00,0x01,0x03,0x05,0x06,0x07,0x00,0x00,0x00]
 
 image_sample_c_d_o_g16 v0, [v0, v1, v2, v4, v6, v7, v8], s[0:7], s[8:11] dmask:0x4 dim:SQ_RSRC_IMG_2D_ARRAY
 ; GFX10: image_sample_c_d_o_g16 v0, [v0, v1, v2, v4, v6, v7, v8], s[0:7], s[8:11] dmask:0x4 dim:SQ_RSRC_IMG_2D_ARRAY ; encoding: [0x2d,0x04,0xe8,0xf0,0x00,0x00,0x40,0x00,0x01,0x02,0x04,0x06,0x07,0x08,0x00,0x00]
 
 image_sample_c_d_o_g16 v[0:1], [v0, v1, v2, v4, v6, v7, v8], s[0:7], s[8:11] dmask:0x6 dim:SQ_RSRC_IMG_2D_ARRAY
 ; GFX10: image_sample_c_d_o_g16 v[0:1], [v0, v1, v2, v4, v6, v7, v8], s[0:7], s[8:11] dmask:0x6 dim:SQ_RSRC_IMG_2D_ARRAY ; encoding: [0x2d,0x06,0xe8,0xf0,0x00,0x00,0x40,0x00,0x01,0x02,0x04,0x06,0x07,0x08,0x00,0x00]
+
+
+; Test that wider than required vaddr registers are accepted by assembler
+
+image_load v[0:3], v[0:1], s[0:7] dmask:0xf dim:SQ_RSRC_IMG_1D
+; GFX10: image_load v[0:3], v[0:1], s[0:7] dmask:0xf dim:SQ_RSRC_IMG_1D ; encoding: [0x00,0x0f,0x00,0xf0,0x00,0x00,0x00,0x00]
+
+image_load v[0:3], v[0:2], s[0:7] dmask:0xf dim:SQ_RSRC_IMG_1D
+; GFX10: image_load v[0:3], v[0:2], s[0:7] dmask:0xf dim:SQ_RSRC_IMG_1D ; encoding: [0x00,0x0f,0x00,0xf0,0x00,0x00,0x00,0x00]
+
+image_sample_d v[0:3], v[0:3], s[0:7], s[8:11] dmask:0xf dim:SQ_RSRC_IMG_1D a16
+; GFX10: image_sample_d v[0:3], v[0:3], s[0:7], s[8:11] dmask:0xf dim:SQ_RSRC_IMG_1D a16 ; encoding: [0x00,0x0f,0x88,0xf0,0x00,0x00,0x40,0x40]
+
+image_sample_d v[0:3], v[0:4], s[0:7], s[8:11] dmask:0xf dim:SQ_RSRC_IMG_1D a16
+; GFX10: image_sample_d v[0:3], v[0:4], s[0:7], s[8:11] dmask:0xf dim:SQ_RSRC_IMG_1D a16 ; encoding: [0x00,0x0f,0x88,0xf0,0x00,0x00,0x40,0x40]
+
+image_sample_d v[0:3], v[0:7], s[0:7], s[8:11] dmask:0xf dim:SQ_RSRC_IMG_2D a16
+; GFX10: image_sample_d v[0:3], v[0:7], s[0:7], s[8:11] dmask:0xf dim:SQ_RSRC_IMG_2D a16 ; encoding: [0x08,0x0f,0x88,0xf0,0x00,0x00,0x40,0x40]
+
+image_sample_d v[0:3], v[0:15], s[0:7], s[8:11] dmask:0xf dim:SQ_RSRC_IMG_2D a16
+; GFX10: image_sample_d v[0:3], v[0:15], s[0:7], s[8:11] dmask:0xf dim:SQ_RSRC_IMG_2D a16 ; encoding: [0x08,0x0f,0x88,0xf0,0x00,0x00,0x40,0x40]

llvm/test/MC/AMDGPU/gfx10_asm_mimg_err.s

 // RUN: not llvm-mc -arch=amdgcn -mcpu=gfx1010 %s 2>&1 | FileCheck --check-prefixes=NOGFX10 --implicit-check-not=error: %s
 
 // TODO: more helpful error message for missing dim operand
 image_load v[0:3], v0, s[0:7] dmask:0xf unorm
 // NOGFX10: :[[@LINE-1]]:{{[0-9]+}}: error: operands are not valid for this GPU or mode
 
 image_load v[0:3], v0, s[0:7] dmask:0xf dim:SQ_RSRC_IMG_1D da
 // NOGFX10: :[[@LINE-1]]:{{[0-9]+}}: error: invalid operand for instruction
 
 image_load_pck v[0:3], v0, s[0:7] dmask:0xf dim:SQ_RSRC_IMG_1D d16
 // NOGFX10: :[[@LINE-1]]:{{[0-9]+}}: error: invalid operand for instruction
 
 image_load v[0:1], v0, s[0:7] dmask:0xf dim:SQ_RSRC_IMG_1D
 // NOGFX10: :[[@LINE-1]]:{{[0-9]+}}: error: image data size does not match dmask and tfe
 
-image_load v[0:3], v[0:1], s[0:7] dmask:0xf dim:SQ_RSRC_IMG_1D
-// NOGFX10: :[[@LINE-1]]:{{[0-9]+}}: error: image address size does not match dim and a16
-
 image_load_mip v[0:3], v[0:2], s[0:7] dmask:0xf dim:SQ_RSRC_IMG_CUBE
 // NOGFX10: :[[@LINE-1]]:{{[0-9]+}}: error: image address size does not match dim and a16
 
 image_sample_d v[0:3], [v0, v1, v2, v3, v4], s[0:7], s[8:11] dmask:0xf dim:SQ_RSRC_IMG_1D_ARRAY
 // NOGFX10: :[[@LINE-1]]:{{[0-9]+}}: error: image address size does not match dim and a16
 
 image_sample_b_cl v[0:3], v[0:3], s[0:7], s[8:11] dmask:0xf dim:SQ_RSRC_IMG_CUBE
 // NOGFX10: :[[@LINE-1]]:{{[0-9]+}}: error: image address size does not match dim and a16

llvm/test/MC/AMDGPU/gfx10_err_pos.s

 ds_swizzle_b32 v8, v2 offset:swizzle(BROADCAST,1,0)
 // CHECK: error: group size must be in the interval [2,32]
 // CHECK-NEXT:{{^}}ds_swizzle_b32 v8, v2 offset:swizzle(BROADCAST,1,0)
 // CHECK-NEXT:{{^}}                                               ^
 
 //==============================================================================
 // image address size does not match dim and a16
 
-image_load v[0:3], v[0:1], s[0:7] dmask:0xf dim:SQ_RSRC_IMG_1D
+image_load v[0:3], v[0:1], s[0:7] dmask:0xf dim:SQ_RSRC_IMG_3D
 // CHECK: error: image address size does not match dim and a16
-// CHECK-NEXT:{{^}}image_load v[0:3], v[0:1], s[0:7] dmask:0xf dim:SQ_RSRC_IMG_1D
+// CHECK-NEXT:{{^}}image_load v[0:3], v[0:1], s[0:7] dmask:0xf dim:SQ_RSRC_IMG_3D
 // CHECK-NEXT:{{^}}^
 
 //==============================================================================
 // image data size does not match dmask and tfe
 
 image_load v[0:1], v0, s[0:7] dmask:0xf dim:SQ_RSRC_IMG_1D
 // CHECK: error: image data size does not match dmask and tfe
 // CHECK-NEXT:{{^}}image_load v[0:1], v0, s[0:7] dmask:0xf dim:SQ_RSRC_IMG_1D