[AMDGPU] Incorrect error message regarding SCC modifier

[jwanggit86]

Summary

For the AMD GFX90A GPU, the SCC instruction modifier is allowed
for certain classes of instructions. However, the current assembler
generates an error message, "scc is not supported on this GPU",
regardless of the instruciton. This fix modifies the message as well
as the logic for generating the message. Related tests are moved from
gfx90a_err.s to gfx90a_asm_features.s.

[arsenm]

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

4483

I thought we had predefined masks for all the memory instruction types

llvm/test/MC/AMDGPU/gfx90a_err.s

198

needs to add the positive tests for these successfully parsing

[jwanggit86]

(1) created a const to hold the flags for all the 4 classes of instructions (2) tests removed from gfx90a_err.s are added to gfx90a_asm_features.s.

[arsenm]

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

4483–4489

4483–4489

apply clang-format, line looks too long

[jwanggit86]

Made 2 changes per review comments: (1) var name changed from scc_flags to HasSCCInsts (2) code formatted with clang-format.

[arsenm]

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

4483–4489

SCCInsts isn't accurate. HasScalarCacheControl?

[jwanggit86]

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

4483–4489

I thought SCC stands for System Cache Coherence. Anyway, SCC is not spelled out in this function, or anywhere else. A more accurate name for this tmp var is probably "AllowSCC" b/c the rhs is the flags representing 4 classes of instructions that allow SCC. Your thoughts?

[arsenm]

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

4483–4489

SCC is also a completely unrelated register so the whole thing is confusing

[jwanggit86]

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

4483–4489

How about "AllowSCCModifier"? That should make it clear that we are referring to the modifier, not the register.

[arsenm]

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

4483–4489

Sure

[jwanggit86]

Changed local var name from "HasSCCInsts" to "AllowSCCModifier".

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

if (TSFlags & SIInstrFlags::SMRD) {
     }
     if (CPol & ~(AMDGPU::CPol::GLC | AMDGPU::CPol::DLC)) {
       Error(IDLoc, "invalid cache policy for SMEM instruction");
       return false;
     }
   }
 
   if (isGFX90A() && !isGFX940() && (CPol & CPol::SCC)) {
+    if (!(TSFlags & (SIInstrFlags::MUBUF | SIInstrFlags::MTBUF | SIInstrFlags::MIMG | SIInstrFlags::FLAT))) {
       SMLoc S = getImmLoc(AMDGPUOperand::ImmTyCPol, Operands);
       StringRef CStr(S.getPointer());
       S = SMLoc::getFromPointer(&CStr.data()[CStr.find("scc")]);
-    Error(S, "scc is not supported on this GPU");
+      Error(S, "scc modifier is not supported for this instruction on this GPU");
       return false;
     }
+  }
 
   if (!(TSFlags & (SIInstrFlags::IsAtomicNoRet | SIInstrFlags::IsAtomicRet)))
     return true;
 
   if (TSFlags & SIInstrFlags::IsAtomicRet) {
     if (!(TSFlags & SIInstrFlags::MIMG) && !(CPol & CPol::GLC)) {
       Error(IDLoc, isGFX940() ? "instruction must use sc0"
                               : "instruction must use glc");

llvm/test/MC/AMDGPU/gfx90a_err.s

 // GFX90A: :[[@LINE-1]]:{{[0-9]+}}: error: instruction not supported on this GPU
 
 image_sample_cd v[0:3], v[0:1], s[4:11], s[16:19] dmask:0xf
 // GFX90A: :[[@LINE-1]]:{{[0-9]+}}: error: instruction not supported on this GPU
 
 image_sample_b v[0:3], v[0:1], s[4:11], s[16:19] dmask:0xf
 // GFX90A: :[[@LINE-1]]:{{[0-9]+}}: error: instruction not supported on this GPU
 
-global_atomic_add_f32 v0, v[0:1], v2, off glc scc
-// GFX90A: :[[@LINE-1]]:{{[0-9]+}}: error: scc is not supported on this GPU
-
-global_atomic_add_f32 v[0:1], v2, off scc
-// GFX90A: :[[@LINE-1]]:{{[0-9]+}}: error: scc is not supported on this GPU
-
-global_atomic_add_f32 v0, v2, s[0:1] scc
-// GFX90A: :[[@LINE-1]]:{{[0-9]+}}: error: scc is not supported on this GPU
-
-global_atomic_add_f32 v1, v0, v2, s[0:1] glc scc
-// GFX90A: :[[@LINE-1]]:{{[0-9]+}}: error: scc is not supported on this GPU
-
-global_atomic_pk_add_f16 v0, v[0:1], v2, off glc scc
-// GFX90A: :[[@LINE-1]]:{{[0-9]+}}: error: scc is not supported on this GPU
-
-flat_atomic_add_f64 v[0:1], v[0:1], v[2:3] glc scc
-// GFX90A: :[[@LINE-1]]:{{[0-9]+}}: error: scc is not supported on this GPU
-
-flat_atomic_add_f64 v[0:1], v[2:3] scc
-// GFX90A: :[[@LINE-1]]:{{[0-9]+}}: error: scc is not supported on this GPU
-
-flat_atomic_min_f64 v[0:1], v[2:3] scc
-// GFX90A: :[[@LINE-1]]:{{[0-9]+}}: error: scc is not supported on this GPU
-
-flat_atomic_max_f64 v[0:1], v[2:3] scc
-// GFX90A: :[[@LINE-1]]:{{[0-9]+}}: error: scc is not supported on this GPU
-
-global_atomic_add_f64 v[0:1], v[2:3], off scc
-// GFX90A: :[[@LINE-1]]:{{[0-9]+}}: error: scc is not supported on this GPU
-
-global_atomic_min_f64 v[0:1], v[2:3], off scc
-// GFX90A: :[[@LINE-1]]:{{[0-9]+}}: error: scc is not supported on this GPU
-
-global_atomic_max_f64 v[0:1], v[2:3], off scc
-// GFX90A: :[[@LINE-1]]:{{[0-9]+}}: error: scc is not supported on this GPU
-
-buffer_atomic_add_f32 v4, off, s[8:11], s3 scc
-// GFX90A: :[[@LINE-1]]:{{[0-9]+}}: error: scc is not supported on this GPU
-
-buffer_atomic_pk_add_f16 v4, off, s[8:11], s3 scc
-// GFX90A: :[[@LINE-1]]:{{[0-9]+}}: error: scc is not supported on this GPU
-
-buffer_atomic_add_f64 v[4:5], off, s[8:11], s3 scc
-// GFX90A: :[[@LINE-1]]:{{[0-9]+}}: error: scc is not supported on this GPU
-
-buffer_atomic_max_f64 v[4:5], off, s[8:11], s3 scc
-// GFX90A: :[[@LINE-1]]:{{[0-9]+}}: error: scc is not supported on this GPU
-
-buffer_atomic_min_f64 v[4:5], off, s[8:11], s3 scc
-// GFX90A: :[[@LINE-1]]:{{[0-9]+}}: error: scc is not supported on this GPU
-
 v_mov_b32_sdwa v1, src_lds_direct dst_sel:DWORD
 // GFX90A: :[[@LINE-1]]:{{[0-9]+}}: error: lds_direct is not supported on this GPU
 
 v_add_f32_sdwa v5, v1, lds_direct dst_sel:DWORD
 // GFX90A: :[[@LINE-1]]:{{[0-9]+}}: error: lds_direct is not supported on this GPU
 
 v_ashrrev_i16 v0, lds_direct, v0
 // GFX90A: :[[@LINE-1]]:{{[0-9]+}}: error: lds_direct is not supported on this GPU