[AMDGPU][MC][GFX8] Added BUFFER_STORE_LDS_DWORD Instruction

[dp]

Summary

See bug 36558: https://bugs.llvm.org/show_bug.cgi?id=36558

[artem.tamazov]

The change requires LDS bit in BUFFER_STORE_LDS_DWORD. This looks incorrect. LDS bit affects only MUBUF reads.

The change disallows TFE for BUFFER_STORE_LDS instructions as well as buffer loads to LDS. However, ISA manual disallows the latter case only: it says that it is illegal to set the instruction’s TFE bit for loads to LDS.

I believe that both LDS and TFE do not affect buffer stores. Perhaps it is worth to issue a warning or error in such cases. That's up to you.

[dp]

AMD documentation explicitly states:

"MUBUF Instruction: BUFFER_STORE_LDS_DWORD
    LDS = 1
    IDXEN = 0
    OFFEN = 0
    ..."

See Gfx9_Shader_Programming, Section 3.9.8 "LDS_DMA : LDS to Memory buffer without VGPRs".

Regarding TFE, documentation says that TFE is valid for "fetch" instructions only. By design, TFE is useless with stores.
I believe this instruction is not a "fetch".

[artem.tamazov]

In D43950#1024132, @dp wrote:

Regarding TFE, documentation says that TFE is valid for "fetch" instructions only. By design, TFE is useless with stores.
I believe this instruction is not a "fetch".

Sure. However, there is a difference between "invalid" and "useless". Let's decide which approach we like and use it consistently.
Right now TFE is not disabled in "conventional" buffer stores (I just checked that), but this patch disallows it in LDS stores.
This can be done in a separate patch, but let's not forget that.

In D43950#1024132, @dp wrote:

LDS = 1
IDXEN = 0
OFFEN = 0

Thanks. So we require LDS and disallow both idxen and offen.

LGTM provided that now we stay on the same page.

test/MC/AMDGPU/mubuf.s

775 ↗

(On Diff #136544)

It seems strange that this is not "NOSICI: error: instruction not supported on this GPU"

[arsenm]

lib/Target/AMDGPU/BUFInstructions.td

899 ↗

(On Diff #136544)

This isn't a separate opcode according to the documentation? Isn't this just a modifier bit? i.e. this would be BUFFER_STORE_DWORD_lds and not have a separate string name either

[dp]

According to AMD documentation, this is a separate opcode:

BUFFER_STORE_DWORD: opcode=28
BUFFER_STORE_LDS_DWORD: opcode=61

See e.g. "Vega" Instruction Set Architecture, page 175

[dp]

lib/Target/AMDGPU/BUFInstructions.td

899 ↗

(On Diff #136544)

This is a separate opcode

[artem.tamazov]

Dmitry, I believe you can go ahead.

lib/Target/AMDGPU/BUFInstructions.td

899 ↗

(On Diff #136544)

Matt, if you are thinking that LDS attribute affects buffer stores, then this is not the case. There is a separate opcode for writing from LDS into memory.

[arsenm]

LGTM

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

private:
 
   bool AddNextRegisterToList(unsigned& Reg, unsigned& RegWidth,
                              RegisterKind RegKind, unsigned Reg1,
                              unsigned RegNum);
   bool ParseAMDGPURegister(RegisterKind& RegKind, unsigned& Reg,
                            unsigned& RegNum, unsigned& RegWidth,
                            unsigned *DwordRegIndex);
   void cvtMubufImpl(MCInst &Inst, const OperandVector &Operands,
-                    bool IsAtomic, bool IsAtomicReturn);
+                    bool IsAtomic, bool IsAtomicReturn, bool IsLds = false);
   void cvtDSImpl(MCInst &Inst, const OperandVector &Operands,
                  bool IsGdsHardcoded);
 
 public:
   enum AMDGPUMatchResultTy {
     Match_PreferE32 = FIRST_TARGET_MATCH_RESULT_TY
   };
 
public:
   bool parseSwizzleBitmaskPerm(int64_t &Imm);
   bool parseSwizzleBroadcast(int64_t &Imm);
   bool parseSwizzleSwap(int64_t &Imm);
   bool parseSwizzleReverse(int64_t &Imm);
 
   void cvtMubuf(MCInst &Inst, const OperandVector &Operands) { cvtMubufImpl(Inst, Operands, false, false); }
   void cvtMubufAtomic(MCInst &Inst, const OperandVector &Operands) { cvtMubufImpl(Inst, Operands, true, false); }
   void cvtMubufAtomicReturn(MCInst &Inst, const OperandVector &Operands) { cvtMubufImpl(Inst, Operands, true, true); }
+  void cvtMubufLds(MCInst &Inst, const OperandVector &Operands) { cvtMubufImpl(Inst, Operands, false, false, true); }
   void cvtMtbuf(MCInst &Inst, const OperandVector &Operands);
 
   AMDGPUOperand::Ptr defaultGLC() const;
   AMDGPUOperand::Ptr defaultSLC() const;
   AMDGPUOperand::Ptr defaultTFE() const;
 
   AMDGPUOperand::Ptr defaultD16() const;
   AMDGPUOperand::Ptr defaultDMask() const;

 }
 
 AMDGPUOperand::Ptr AMDGPUAsmParser::defaultTFE() const {
   return AMDGPUOperand::CreateImm(this, 0, SMLoc(), AMDGPUOperand::ImmTyTFE);
 }
 
 void AMDGPUAsmParser::cvtMubufImpl(MCInst &Inst,
                                const OperandVector &Operands,
-                               bool IsAtomic, bool IsAtomicReturn) {
+                               bool IsAtomic,
+                               bool IsAtomicReturn,
+                               bool IsLds) {
+  bool IsLdsOpcode = IsLds;
   bool HasLdsModifier = false;
   OptionalImmIndexMap OptionalIdx;
   assert(IsAtomicReturn ? IsAtomic : true);
 
   for (unsigned i = 1, e = Operands.size(); i != e; ++i) {
     AMDGPUOperand &Op = ((AMDGPUOperand &)*Operands[i]);
 
     // Add the register arguments
void AMDGPUAsmParser::cvtMubufImpl(MCInst &Inst,
 
   // This is a workaround for an llvm quirk which may result in an
   // incorrect instruction selection. Lds and non-lds versions of
   // MUBUF instructions are identical except that lds versions
   // have mandatory 'lds' modifier. However this modifier follows
   // optional modifiers and llvm asm matcher regards this 'lds'
   // modifier as an optional one. As a result, an lds version
   // of opcode may be selected even if it has no 'lds' modifier.
-  if (!HasLdsModifier) {
+  if (IsLdsOpcode && !HasLdsModifier) {
     int NoLdsOpcode = AMDGPU::getMUBUFNoLdsInst(Inst.getOpcode());
     if (NoLdsOpcode != -1) { // Got lds version - correct it.
       Inst.setOpcode(NoLdsOpcode);
+      IsLdsOpcode = false;
     }
   }
 
   // Copy $vdata_in operand and insert as $vdata for MUBUF_Atomic RTN insns.
   if (IsAtomicReturn) {
     MCInst::iterator I = Inst.begin(); // $vdata_in is always at the beginning.
     Inst.insert(I, *I);
   }
 
   addOptionalImmOperand(Inst, Operands, OptionalIdx, AMDGPUOperand::ImmTyOffset);
   if (!IsAtomic) { // glc is hard-coded.
     addOptionalImmOperand(Inst, Operands, OptionalIdx, AMDGPUOperand::ImmTyGLC);
   }
   addOptionalImmOperand(Inst, Operands, OptionalIdx, AMDGPUOperand::ImmTySLC);
 
-  if (!HasLdsModifier) { // tfe is not legal with lds opcodes
+  if (!IsLdsOpcode) { // tfe is not legal with lds opcodes
     addOptionalImmOperand(Inst, Operands, OptionalIdx, AMDGPUOperand::ImmTyTFE);
   }
 }
 
 void AMDGPUAsmParser::cvtMtbuf(MCInst &Inst, const OperandVector &Operands) {
   OptionalImmIndexMap OptionalIdx;
 
   for (unsigned i = 1, e = Operands.size(); i != e; ++i) {

llvm/trunk/lib/Target/AMDGPU/BUFInstructions.td

: MUBUF_Pseudo<opName,
                  !con(getMUBUFIns<addrKindCopy, [], isLds>.ret,
                       !if(HasTiedDest, (ins vdataClass:$vdata_in), (ins))),
                  " $vdata, " # getMUBUFAsmOps<addrKindCopy>.ret # "$glc$slc" #
                    !if(isLds, " lds", "$tfe"),
                  pattern>,
     MUBUF_SetupAddr<addrKindCopy> {
   let PseudoInstr = opName # !if(isLds, "_lds", "") #
                     "_" # getAddrName<addrKindCopy>.ret;
+  let AsmMatchConverter = !if(isLds, "cvtMubufLds", "cvtMubuf");
 
   let Constraints = !if(HasTiedDest, "$vdata = $vdata_in", "");
   let mayLoad = 1;
   let mayStore = 0;
   let maybeAtomic = 1;
   let Uses = !if(isLds, [EXEC, M0], [EXEC]);
   let has_tfe = !if(isLds, 0, 1);
   let lds = isLds;
multiclass MUBUF_Pseudo_Stores<string opName, RegisterClass vdataClass,
   let DisableWQM = 1 in {
     def _OFFSET_exact : MUBUF_Store_Pseudo <opName, BUFAddrKind.Offset, vdataClass>;
     def _OFFEN_exact  : MUBUF_Store_Pseudo <opName, BUFAddrKind.OffEn, vdataClass>;
     def _IDXEN_exact  : MUBUF_Store_Pseudo <opName, BUFAddrKind.IdxEn, vdataClass>;
     def _BOTHEN_exact : MUBUF_Store_Pseudo <opName, BUFAddrKind.BothEn, vdataClass>;
   }
 }
 
+class MUBUF_Pseudo_Store_Lds<string opName>
+  : MUBUF_Pseudo<opName,
+                 (outs),
+                 (ins SReg_128:$srsrc, SCSrc_b32:$soffset, offset:$offset, GLC:$glc, slc:$slc),
+                 " $srsrc, $soffset$offset lds$glc$slc"> {
+  let mayLoad = 0;
+  let mayStore = 1;
+  let maybeAtomic = 1;
+
+  let has_vdata = 0;
+  let has_vaddr = 0;
+  let has_tfe = 0;
+  let lds = 1;
+
+  let Uses = [EXEC, M0];
+  let AsmMatchConverter = "cvtMubufLds";
+}
 
 class getMUBUFAtomicInsDA<RegisterClass vdataClass, bit vdata_in,
                           list<RegisterClass> vaddrList=[]> {
   RegisterClass vaddrClass = !if(!empty(vaddrList), ?, !head(vaddrList));
   dag ret = !if(vdata_in,
     !if(!empty(vaddrList),
       (ins vdataClass:$vdata_in,
            SReg_128:$srsrc, SCSrc_b32:$soffset, offset:$offset, slc:$slc),

 >;
 defm BUFFER_ATOMIC_INC_X2 : MUBUF_Pseudo_Atomics <
   "buffer_atomic_inc_x2", VReg_64, i64, atomic_inc_global
 >;
 defm BUFFER_ATOMIC_DEC_X2 : MUBUF_Pseudo_Atomics <
   "buffer_atomic_dec_x2", VReg_64, i64, atomic_dec_global
 >;
 
+let SubtargetPredicate = isVI in {
+def BUFFER_STORE_LDS_DWORD : MUBUF_Pseudo_Store_Lds <"buffer_store_lds_dword">;
+}
+
 let SubtargetPredicate = isSI in { // isn't on CI & VI
 /*
 defm BUFFER_ATOMIC_RSUB        : MUBUF_Pseudo_Atomics <"buffer_atomic_rsub">;
 defm BUFFER_ATOMIC_FCMPSWAP    : MUBUF_Pseudo_Atomics <"buffer_atomic_fcmpswap">;
 defm BUFFER_ATOMIC_FMIN        : MUBUF_Pseudo_Atomics <"buffer_atomic_fmin">;
 defm BUFFER_ATOMIC_FMAX        : MUBUF_Pseudo_Atomics <"buffer_atomic_fmax">;
 defm BUFFER_ATOMIC_RSUB_X2     : MUBUF_Pseudo_Atomics <"buffer_atomic_rsub_x2">;
 defm BUFFER_ATOMIC_FCMPSWAP_X2 : MUBUF_Pseudo_Atomics <"buffer_atomic_fcmpswap_x2">;

 defm BUFFER_ATOMIC_SMAX_X2      : MUBUF_Real_Atomic_vi <0x66>;
 defm BUFFER_ATOMIC_UMAX_X2      : MUBUF_Real_Atomic_vi <0x67>;
 defm BUFFER_ATOMIC_AND_X2       : MUBUF_Real_Atomic_vi <0x68>;
 defm BUFFER_ATOMIC_OR_X2        : MUBUF_Real_Atomic_vi <0x69>;
 defm BUFFER_ATOMIC_XOR_X2       : MUBUF_Real_Atomic_vi <0x6a>;
 defm BUFFER_ATOMIC_INC_X2       : MUBUF_Real_Atomic_vi <0x6b>;
 defm BUFFER_ATOMIC_DEC_X2       : MUBUF_Real_Atomic_vi <0x6c>;
 
+def BUFFER_STORE_LDS_DWORD_vi   : MUBUF_Real_vi <0x3d, BUFFER_STORE_LDS_DWORD>;
+
 def BUFFER_WBINVL1_vi           : MUBUF_Real_vi <0x3e, BUFFER_WBINVL1>;
 def BUFFER_WBINVL1_VOL_vi       : MUBUF_Real_vi <0x3f, BUFFER_WBINVL1_VOL>;
 
 class MTBUF_Real_vi <bits<4> op, MTBUF_Pseudo ps> :
   MTBUF_Real<ps>,
   Enc64,
   SIMCInstr<ps.PseudoInstr, SIEncodingFamily.VI> {
   let AssemblerPredicate=isVI;

llvm/trunk/test/MC/AMDGPU/mubuf.s

 // RUN: not llvm-mc -arch=amdgcn -mcpu=tahiti -show-encoding %s | FileCheck -check-prefix=GCN -check-prefix=SI -check-prefix=SICI %s
 // RUN: not llvm-mc -arch=amdgcn -mcpu=bonaire -show-encoding %s | FileCheck -check-prefix=GCN -check-prefix=CI -check-prefix=SICI %s
 // RUN: not llvm-mc -arch=amdgcn -mcpu=tonga -show-encoding %s | FileCheck -check-prefix=GCN -check-prefix=VI %s
 
-// RUN: not llvm-mc -arch=amdgcn -mcpu=tahiti %s 2>&1 | FileCheck -check-prefix=NOSI -check-prefix=NOSICIVI %s
-// RUN: not llvm-mc -arch=amdgcn -mcpu=bonaire %s 2>&1 | FileCheck -check-prefix=NOCI -check-prefix=NOSICIVI %s
+// RUN: not llvm-mc -arch=amdgcn -mcpu=tahiti %s 2>&1 | FileCheck -check-prefix=NOSI -check-prefix=NOSICIVI -check-prefix=NOSICI %s
+// RUN: not llvm-mc -arch=amdgcn -mcpu=bonaire %s 2>&1 | FileCheck -check-prefix=NOCI -check-prefix=NOSICIVI -check-prefix=NOSICI %s
 // RUN: not llvm-mc -arch=amdgcn -mcpu=tonga %s 2>&1 | FileCheck -check-prefix=NOVI -check-prefix=NOSICIVI %s
 
 //===----------------------------------------------------------------------===//
 // Test for different operand combinations
 //===----------------------------------------------------------------------===//
 
 //===----------------------------------------------------------------------===//
 // load - immediate offset only

 buffer_load_dword v5, v0, s[8:11], s101 offen lds
 // SICI: buffer_load_dword v5, v0, s[8:11], s101 offen lds ; encoding: [0x00,0x10,0x31,0xe0,0x00,0x05,0x02,0x65]
 // VI:   buffer_load_dword v5, v0, s[8:11], s101 offen lds ; encoding: [0x00,0x10,0x51,0xe0,0x00,0x05,0x02,0x65]
 
 buffer_load_format_x v5, v[0:1], s[8:11], s3 idxen offen offset:4095 glc slc lds
 // SICI: buffer_load_format_x v5, v[0:1], s[8:11], s3 idxen offen offset:4095 glc slc lds ; encoding: [0xff,0x7f,0x01,0xe0,0x00,0x05,0x42,0x03]
 // VI:   buffer_load_format_x v5, v[0:1], s[8:11], s3 idxen offen offset:4095 glc slc lds ; encoding: [0xff,0x7f,0x03,0xe0,0x00,0x05,0x02,0x03]
 
+buffer_store_lds_dword s[4:7], s0 lds
+// NOSICI: error: instruction not supported on this GPU
+// VI: buffer_store_lds_dword s[4:7], s0 lds ; encoding: [0x00,0x00,0xf5,0xe0,0x00,0x00,0x01,0x00]
+
+buffer_store_lds_dword s[4:7], s0 offset:4095 lds
+// NOSICI: error: not a valid operand.
+// VI: buffer_store_lds_dword s[4:7], s0 offset:4095 lds ; encoding: [0xff,0x0f,0xf5,0xe0,0x00,0x00,0x01,0x00]
+
+buffer_store_lds_dword s[4:7], s8 offset:4 lds glc slc
+// NOSICI: error: not a valid operand.
+// VI: buffer_store_lds_dword s[4:7], s8 offset:4 lds glc slc ; encoding: [0x04,0x40,0xf7,0xe0,0x00,0x00,0x01,0x08]
+
 //===----------------------------------------------------------------------===//
 // Errors handling
 //===----------------------------------------------------------------------===//
 
 buffer_load_sbyte v5, off, s[8:11], s3 lds tfe
 // NOSICIVI: error: invalid operand for instruction
 
 buffer_load_dword v5, off, s[8:11], s3 tfe lds
 // NOSICIVI: error: invalid operand for instruction
+
+buffer_store_lds_dword s[4:7], s8 offset:4 lds tfe
+// NOSICI: error: not a valid operand.
+// NOVI:   error: invalid operand for instruction
+
+buffer_store_lds_dword s[4:7], s8 offset:4 tfe lds
+// NOSICI: error: not a valid operand.
+// NOVI:   error: invalid operand for instruction

llvm/trunk/test/MC/Disassembler/AMDGPU/mubuf_vi.txt

 # VI:   buffer_load_ushort v5, v0, s[8:11], s3 idxen offset:4095 glc slc lds ; encoding: [0xff,0x6f,0x4b,0xe0,0x00,0x05,0x02,0x03]
 0xff,0x6f,0x4b,0xe0,0x00,0x05,0x02,0x03
 
 # VI:   buffer_load_dword v5, v0, s[8:11], s101 offen lds ; encoding: [0x00,0x10,0x51,0xe0,0x00,0x05,0x02,0x65]
 0x00,0x10,0x51,0xe0,0x00,0x05,0x02,0x65
 
 # VI:   buffer_load_format_x v5, v[0:1], s[8:11], s3 idxen offen offset:4095 glc slc lds ; encoding: [0xff,0x7f,0x03,0xe0,0x00,0x05,0x02,0x03]
 0xff,0x7f,0x03,0xe0,0x00,0x05,0x02,0x03
+
+# VI:   buffer_store_lds_dword s[4:7], s0 lds ; encoding: [0x00,0x00,0xf5,0xe0,0x00,0x00,0x01,0x00]
+0x00,0x00,0xf5,0xe0,0x00,0x00,0x01,0x00
+
+# VI:   buffer_store_lds_dword s[4:7], s0 offset:4095 lds ; encoding: [0xff,0x0f,0xf5,0xe0,0x00,0x00,0x01,0x00]
+0xff,0x0f,0xf5,0xe0,0x00,0x00,0x01,0x00
+
+# VI:   buffer_store_lds_dword s[4:7], s8 offset:4 lds glc slc ; encoding: [0x04,0x40,0xf7,0xe0,0x00,0x00,0x01,0x08]
+0x04,0x40,0xf7,0xe0,0x00,0x00,0x01,0x08