[AMDGPU][MC] Corrected error message "image data size does not match dmask and tfe"

[dp]

Summary

d16 modifier also affects image data size.
See bug 54693.

[dp]

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp

1605

The name of the function was misleading because the function returned true for GFX6 and GFX7.

[dp]

Diff has been updated to show context.

[rampitec]

LGTM

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

private:
   bool validateSMEMOffset(const MCInst &Inst, const OperandVector &Operands);
   bool validateSOPLiteral(const MCInst &Inst) const;
   bool validateConstantBusLimitations(const MCInst &Inst, const OperandVector &Operands);
   bool validateEarlyClobberLimitations(const MCInst &Inst, const OperandVector &Operands);
   bool validateIntClampSupported(const MCInst &Inst);
   bool validateMIMGAtomicDMask(const MCInst &Inst);
   bool validateMIMGGatherDMask(const MCInst &Inst);
   bool validateMovrels(const MCInst &Inst, const OperandVector &Operands);
-  bool validateMIMGDataSize(const MCInst &Inst);
+  Optional<StringRef> validateMIMGDataSize(const MCInst &Inst);
   bool validateMIMGAddrSize(const MCInst &Inst);
   bool validateMIMGD16(const MCInst &Inst);
   bool validateMIMGDim(const MCInst &Inst);
   bool validateMIMGMSAA(const MCInst &Inst);
   bool validateOpSel(const MCInst &Inst);
   bool validateDPP(const MCInst &Inst, const OperandVector &Operands);
   bool validateVccOperand(unsigned Reg) const;
   bool validateVOPLiteral(const MCInst &Inst, const OperandVector &Operands);
if ((Desc.TSFlags & SIInstrFlags::IntClamp) != 0 && !hasIntClamp()) {
     int ClampIdx = AMDGPU::getNamedOperandIdx(Opc, AMDGPU::OpName::clamp);
     assert(ClampIdx != -1);
     return Inst.getOperand(ClampIdx).getImm() == 0;
   }
 
   return true;
 }
 
-bool AMDGPUAsmParser::validateMIMGDataSize(const MCInst &Inst) {
+Optional<StringRef> AMDGPUAsmParser::validateMIMGDataSize(const MCInst &Inst) {
 
   const unsigned Opc = Inst.getOpcode();
   const MCInstrDesc &Desc = MII.get(Opc);
 
   if ((Desc.TSFlags & SIInstrFlags::MIMG) == 0)
-    return true;
+    return None;
 
   int VDataIdx = AMDGPU::getNamedOperandIdx(Opc, AMDGPU::OpName::vdata);
   int DMaskIdx = AMDGPU::getNamedOperandIdx(Opc, AMDGPU::OpName::dmask);
   int TFEIdx   = AMDGPU::getNamedOperandIdx(Opc, AMDGPU::OpName::tfe);
 
   assert(VDataIdx != -1);
 
   if (DMaskIdx == -1 || TFEIdx == -1) // intersect_ray
-    return true;
+    return None;
 
   unsigned VDataSize = AMDGPU::getRegOperandSize(getMRI(), Desc, VDataIdx);
   unsigned TFESize = (TFEIdx != -1 && Inst.getOperand(TFEIdx).getImm()) ? 1 : 0;
   unsigned DMask = Inst.getOperand(DMaskIdx).getImm() & 0xf;
   if (DMask == 0)
     DMask = 1;
 
+  bool isPackedD16 = false;
   unsigned DataSize =
     (Desc.TSFlags & SIInstrFlags::Gather4) ? 4 : countPopulation(DMask);
   if (hasPackedD16()) {
     int D16Idx = AMDGPU::getNamedOperandIdx(Opc, AMDGPU::OpName::d16);
-    if (D16Idx >= 0 && Inst.getOperand(D16Idx).getImm())
+    isPackedD16 = D16Idx >= 0;
+    if (isPackedD16 && Inst.getOperand(D16Idx).getImm())
       DataSize = (DataSize + 1) / 2;
   }
 
-  return (VDataSize / 4) == DataSize + TFESize;
+  if ((VDataSize / 4) == DataSize + TFESize)
+    return None;
+
+  return StringRef(isPackedD16
+                       ? "image data size does not match dmask, d16 and tfe"
+                       : "image data size does not match dmask and tfe");
 }
 
 bool AMDGPUAsmParser::validateMIMGAddrSize(const MCInst &Inst) {
   const unsigned Opc = Inst.getOpcode();
   const MCInstrDesc &Desc = MII.get(Opc);
 
   if ((Desc.TSFlags & SIInstrFlags::MIMG) == 0 || !isGFX10Plus())
     return true;
if (!validateMIMGDim(Inst)) {
     Error(IDLoc, "dim modifier is required on this GPU");
     return false;
   }
   if (!validateMIMGMSAA(Inst)) {
     Error(getImmLoc(AMDGPUOperand::ImmTyDim, Operands),
           "invalid dim; must be MSAA type");
     return false;
   }
-  if (!validateMIMGDataSize(Inst)) {
-    Error(IDLoc,
-      "image data size does not match dmask and tfe");
+  if (auto ErrMsg = validateMIMGDataSize(Inst)) {
+    Error(IDLoc, *ErrMsg);
     return false;
   }
   if (!validateMIMGAddrSize(Inst)) {
     Error(IDLoc,
       "image address size does not match dim and a16");
     return false;
   }
   if (!validateMIMGAtomicDMask(Inst)) {

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp

bool isKernelCC(const Function *Func) {
   return AMDGPU::isModuleEntryFunctionCC(Func->getCallingConv());
 }
 
 bool hasXNACK(const MCSubtargetInfo &STI) {
   return STI.getFeatureBits()[AMDGPU::FeatureXNACK];
 }
 
 bool hasSRAMECC(const MCSubtargetInfo &STI) {
   return STI.getFeatureBits()[AMDGPU::FeatureSRAMECC];
 }
 
 bool hasMIMG_R128(const MCSubtargetInfo &STI) {
   return STI.getFeatureBits()[AMDGPU::FeatureMIMG_R128] && !STI.getFeatureBits()[AMDGPU::FeatureR128A16];
 }
 
 bool hasGFX10A16(const MCSubtargetInfo &STI) {
   return STI.getFeatureBits()[AMDGPU::FeatureGFX10A16];
 }
 
 bool hasG16(const MCSubtargetInfo &STI) {
   return STI.getFeatureBits()[AMDGPU::FeatureG16];
 }
 
 bool hasPackedD16(const MCSubtargetInfo &STI) {
-  return !STI.getFeatureBits()[AMDGPU::FeatureUnpackedD16VMem];
+  return !STI.getFeatureBits()[AMDGPU::FeatureUnpackedD16VMem] && !isCI(STI) &&
+         !isSI(STI);
 }
 
 bool isSI(const MCSubtargetInfo &STI) {
   return STI.getFeatureBits()[AMDGPU::FeatureSouthernIslands];
 }
 
 bool isCI(const MCSubtargetInfo &STI) {
   return STI.getFeatureBits()[AMDGPU::FeatureSeaIslands];

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
+// NOGFX10: :[[@LINE-1]]:{{[0-9]+}}: error: image data size does not match dmask, d16 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

llvm/test/MC/AMDGPU/gfx10_err_pos.s

 // 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_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_3D
 // CHECK-NEXT:{{^}}^
 
 //==============================================================================
-// image data size does not match dmask and tfe
+// image data size does not match dmask, d16 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: error: image data size does not match dmask, d16 and tfe
 // CHECK-NEXT:{{^}}image_load v[0:1], v0, s[0:7] dmask:0xf dim:SQ_RSRC_IMG_1D
 // CHECK-NEXT:{{^}}^
 
 //==============================================================================
 // instruction must use glc
 
 flat_atomic_cmpswap v0, v[1:2], v[3:4] offset:2047
 // CHECK: error: instruction must use glc

llvm/test/MC/AMDGPU/mimg.s

 
 //===----------------------------------------------------------------------===//
 // Image Load/Store: d16 unpacked
 //===----------------------------------------------------------------------===//
 
 image_load v[5:6], v[1:4], s[8:15] dmask:0x3 d16
 // NOSICI:   error: d16 modifier is not supported on this GPU
 // GFX8_0:   image_load v[5:6], v[1:4], s[8:15] dmask:0x3 d16 ; encoding: [0x00,0x03,0x00,0xf0,0x01,0x05,0x02,0x80]
-// NOGFX8_1: error: image data size does not match dmask and tfe
-// NOGFX9:   error: image data size does not match dmask and tfe
+// NOGFX8_1: error: image data size does not match dmask, d16 and tfe
+// NOGFX9:   error: image data size does not match dmask, d16 and tfe
 
 image_load v[5:7], v[1:4], s[8:15] dmask:0x7 d16
 // NOSICI:   error: d16 modifier is not supported on this GPU
 // GFX8_0:   image_load v[5:7], v[1:4], s[8:15] dmask:0x7 d16 ; encoding: [0x00,0x07,0x00,0xf0,0x01,0x05,0x02,0x80]
-// NOGFX8_1: error: image data size does not match dmask and tfe
-// NOGFX9:   error: image data size does not match dmask and tfe
+// NOGFX8_1: error: image data size does not match dmask, d16 and tfe
+// NOGFX9:   error: image data size does not match dmask, d16 and tfe
 
 image_load v[5:8], v[1:4], s[8:15] dmask:0xf d16
 // NOSICI:   error: d16 modifier is not supported on this GPU
 // GFX8_0:   image_load v[5:8], v[1:4], s[8:15] dmask:0xf d16 ; encoding: [0x00,0x0f,0x00,0xf0,0x01,0x05,0x02,0x80]
-// NOGFX8_1: error: image data size does not match dmask and tfe
-// NOGFX9:   error: image data size does not match dmask and tfe
+// NOGFX8_1: error: image data size does not match dmask, d16 and tfe
+// NOGFX9:   error: image data size does not match dmask, d16 and tfe
 
 image_load v[5:7], v[1:4], s[8:15] dmask:0x3 tfe d16
 // NOSICI:   error: d16 modifier is not supported on this GPU
 // GFX8_0:   image_load v[5:7], v[1:4], s[8:15] dmask:0x3 tfe d16 ; encoding: [0x00,0x03,0x01,0xf0,0x01,0x05,0x02,0x80]
-// NOGFX8_1: error: image data size does not match dmask and tfe
-// NOGFX9:   error: image data size does not match dmask and tfe
+// NOGFX8_1: error: image data size does not match dmask, d16 and tfe
+// NOGFX9:   error: image data size does not match dmask, d16 and tfe
 
 image_load v[5:8], v[1:4], s[8:15] dmask:0x7 tfe d16
 // NOSICI:   error: d16 modifier is not supported on this GPU
 // GFX8_0:   image_load v[5:8], v[1:4], s[8:15] dmask:0x7 tfe d16 ; encoding: [0x00,0x07,0x01,0xf0,0x01,0x05,0x02,0x80]
-// NOGFX8_1: error: image data size does not match dmask and tfe
-// NOGFX9:   error: image data size does not match dmask and tfe
+// NOGFX8_1: error: image data size does not match dmask, d16 and tfe
+// NOGFX9:   error: image data size does not match dmask, d16 and tfe
 
 //===----------------------------------------------------------------------===//
 // Image Load/Store: d16 packed
 //===----------------------------------------------------------------------===//
 
 image_load v5, v[1:4], s[8:15] dmask:0x3 d16
 // NOSICI:   error: d16 modifier is not supported on this GPU
 // NOGFX8_0: error: image data size does not match dmask and tfe

 
 //===----------------------------------------------------------------------===//
 // Image Sample: d16 packed
 //===----------------------------------------------------------------------===//
 
 image_sample  v[193:195], v[237:240], s[28:35], s[4:7] dmask:0x7 d16
 // NOSICI:   error: d16 modifier is not supported on this GPU
 // GFX8_0:   image_sample v[193:195], v[237:240], s[28:35], s[4:7] dmask:0x7 d16 ; encoding: [0x00,0x07,0x80,0xf0,0xed,0xc1,0x27,0x80]
-// NOGFX8_1: error: image data size does not match dmask and tfe
-// NOGFX9:   error: image data size does not match dmask and tfe
+// NOGFX8_1: error: image data size does not match dmask, d16 and tfe
+// NOGFX9:   error: image data size does not match dmask, d16 and tfe
 
 //===----------------------------------------------------------------------===//
 // Image Sample: d16 unpacked
 //===----------------------------------------------------------------------===//
 
 image_sample  v[193:194], v[237:240], s[28:35], s[4:7] dmask:0x7 d16
 // NOSICI:   error: d16 modifier is not supported on this GPU
 // NOGFX8_0: error: image data size does not match dmask and tfe

 // GCN: image_gather4 v[5:8], v[1:4], s[8:15], s[12:15] dmask:0x4 ; encoding: [0x00,0x04,0x00,0xf1,0x01,0x05,0x62,0x00]
 
 image_gather4 v[5:8], v[1:4], s[8:15], s[12:15] dmask:0x8
 // GCN: image_gather4 v[5:8], v[1:4], s[8:15], s[12:15] dmask:0x8 ; encoding: [0x00,0x08,0x00,0xf1,0x01,0x05,0x62,0x00]
 
 image_gather4 v[5:8], v1, s[8:15], s[12:15] dmask:0x1 d16
 // NOSICI:   error: d16 modifier is not supported on this GPU
 // GFX8_0:   image_gather4 v[5:8], v1, s[8:15], s[12:15] dmask:0x1 d16 ; encoding: [0x00,0x01,0x00,0xf1,0x01,0x05,0x62,0x80]
-// NOGFX8_1: error: image data size does not match dmask and tfe
-// NOGFX9:   error: image data size does not match dmask and tfe
+// NOGFX8_1: error: image data size does not match dmask, d16 and tfe
+// NOGFX9:   error: image data size does not match dmask, d16 and tfe
 
 image_gather4 v[5:6], v1, s[8:15], s[12:15] dmask:0x1 d16
 // NOSICI:   error: d16 modifier is not supported on this GPU
 // NOGFX8_0: error: image data size does not match dmask and tfe
 // GFX8_1:   image_gather4 v[5:6], v1, s[8:15], s[12:15] dmask:0x1 d16 ; encoding: [0x00,0x01,0x00,0xf1,0x01,0x05,0x62,0x80]
 // GFX9:     image_gather4 v[5:6], v1, s[8:15], s[12:15] dmask:0x1 d16 ; encoding: [0x00,0x01,0x00,0xf1,0x01,0x05,0x62,0x80]
 
 image_gather4 v[5:6], v1, s[8:15], s[12:15] dmask:0x1
 // NOSICI:   error: image data size does not match dmask and tfe
 // NOGFX8_0: error: image data size does not match dmask and tfe
-// NOGFX8_1: error: image data size does not match dmask and tfe
-// NOGFX9:   error: image data size does not match dmask and tfe
+// NOGFX8_1: error: image data size does not match dmask, d16 and tfe
+// NOGFX9:   error: image data size does not match dmask, d16 and tfe
 
 image_gather4 v[5:8], v1, s[8:15], s[12:15] dmask:0x1 a16
 // GFX9:     image_gather4 v[5:8], v1, s[8:15], s[12:15] dmask:0x1 a16 ; encoding: [0x00,0x81,0x00,0xf1,0x01,0x05,0x62,0x00]
 // NOSICI:   error: a16 modifier is not supported on this GPU
 // NOVI:   error: a16 modifier is not supported on this GPU
 
 image_gather4_b_cl v[5:8], v[1:4], s[8:15], s[12:15] dmask:0x1 a16
 // GFX9:    image_gather4_b_cl v[5:8], v[1:4], s[8:15], s[12:15] dmask:0x1 a16 ; encoding: [0x00,0x81,0x18,0xf1,0x01,0x05,0x62,0x00]
 // NOSICI:   error: a16 modifier is not supported on this GPU
 // NOVI:   error: a16 modifier is not supported on this GPU