[AMDGPU] Allow null operands in VImage tensor instructions#200911
Conversation
|
@llvm/pr-subscribers-backend-amdgpu Author: Ryan Mitchell (RyanRio) ChangesNULL is equivalent to passing a block of SGPRs that are set to zero, and is allowed 2 Files Affected:
diff --git a/llvm/lib/Target/AMDGPU/MIMGInstructions.td b/llvm/lib/Target/AMDGPU/MIMGInstructions.td
index 0f31697f15688..a425e06a7554c 100644
--- a/llvm/lib/Target/AMDGPU/MIMGInstructions.td
+++ b/llvm/lib/Target/AMDGPU/MIMGInstructions.td
@@ -2182,9 +2182,9 @@ class VIMAGE_TENSOR_Pseudo<string opName, bit _UpTo2D = 0> :
let hasSideEffects = 0;
bit UpTo2D = _UpTo2D;
- let InOperandList = !if(UpTo2D, (ins SReg_128_XNULL:$vaddr0, SReg_256_XNULL:$vaddr1, R128A16:$r128, CPol:$cpol),
- (ins SReg_128_XNULL:$vaddr0, SReg_256_XNULL:$vaddr1, SReg_128_XNULL:$vaddr2,
- SReg_128_XNULL:$vaddr3, R128A16:$r128, CPol:$cpol));
+ let InOperandList = !if(UpTo2D, (ins SReg_128:$vaddr0, SReg_256:$vaddr1, R128A16:$r128, CPol:$cpol),
+ (ins SReg_128:$vaddr0, SReg_256:$vaddr1, SReg_128:$vaddr2,
+ SReg_128:$vaddr3, R128A16:$r128, CPol:$cpol));
string AsmOperands = " $vaddr0, $vaddr1"#!if(UpTo2D, "", ", $vaddr2, $vaddr3")#"$r128$cpol";
}
diff --git a/llvm/test/MC/AMDGPU/gfx1250_asm_vimage_err.s b/llvm/test/MC/AMDGPU/gfx1250_asm_vimage_err.s
index 2f911ae79c00f..3f8a913b2c458 100644
--- a/llvm/test/MC/AMDGPU/gfx1250_asm_vimage_err.s
+++ b/llvm/test/MC/AMDGPU/gfx1250_asm_vimage_err.s
@@ -25,42 +25,6 @@ tensor_store_from_lds s[0:3], s[4:11], s[12:15], s[16:19] r128
tensor_store_from_lds s[0:3], s[4:11], s[12:15], s[16:19] th:TH_LOAD_NT_HT scope:SCOPE_DEV
// GFX1250-ERR: :[[@LINE-1]]:59: error: invalid th value for store instructions
-tensor_load_to_lds null, s[4:11]
-// GFX1250-ERR: :[[@LINE-1]]:20: error: invalid operand for instruction
-
-tensor_load_to_lds s[0:3], null
-// GFX1250-ERR: :[[@LINE-1]]:28: error: invalid operand for instruction
-
-tensor_load_to_lds null, s[4:11], s[12:15], s[16:19]
-// GFX1250-ERR: :[[@LINE-1]]:20: error: invalid operand for instruction
-
-tensor_load_to_lds s[0:3], null, s[12:15], s[16:19]
-// GFX1250-ERR: :[[@LINE-1]]:28: error: invalid operand for instruction
-
-tensor_load_to_lds s[0:3], s[4:11], null, s[16:19]
-// GFX1250-ERR: :[[@LINE-1]]:37: error: invalid operand for instruction
-
-tensor_load_to_lds s[0:3], s[4:11], s[12:15], null
-// GFX1250-ERR: :[[@LINE-1]]:47: error: invalid operand for instruction
-
-tensor_store_from_lds null, s[4:11]
-// GFX1250-ERR: :[[@LINE-1]]:23: error: invalid operand for instruction
-
-tensor_store_from_lds s[0:3], null
-// GFX1250-ERR: :[[@LINE-1]]:31: error: invalid operand for instruction
-
-tensor_store_from_lds null, s[4:11], s[12:15], s[16:19]
-// GFX1250-ERR: :[[@LINE-1]]:23: error: invalid operand for instruction
-
-tensor_store_from_lds s[0:3], null, s[12:15], s[16:19]
-// GFX1250-ERR: :[[@LINE-1]]:31: error: invalid operand for instruction
-
-tensor_store_from_lds s[0:3], s[4:11], null, s[16:19]
-// GFX1250-ERR: :[[@LINE-1]]:40: error: invalid operand for instruction
-
-tensor_store_from_lds s[0:3], s[4:11], s[12:15], null
-// GFX1250-ERR: :[[@LINE-1]]:50: error: invalid operand for instruction
-
tensor_load_to_lds s[14:17], s[4:11]
// GFX1250-ERR: :[[@LINE-1]]:20: error: invalid register alignment
|
| tensor_store_from_lds s[0:3], s[4:11], s[12:15], s[16:19] th:TH_LOAD_NT_HT scope:SCOPE_DEV | ||
| // GFX1250-ERR: :[[@LINE-1]]:59: error: invalid th value for store instructions | ||
|
|
||
| tensor_load_to_lds null, s[4:11] |
There was a problem hiding this comment.
Hmm, SP3 doesn't allow this syntax.
Also, need to add them to encoding/decoding tests.
rampitec
left a comment
rampitec
left a comment
There was a problem hiding this comment.
First 2 operands can be numbered SGPRs only.
|
Yeah unfortunately that seems to be the case, even if maybe hardware doesn't check for it. |
The other 2 can be null though. |
|
True 😄 adding the asm/dasm tests too |
| // GFX12-ERR: :[[@LINE-1]]:1: error: instruction not supported on this GPU (gfx1200): tensor_store_from_lds | ||
| // GFX1250: tensor_store_from_lds s[0:3], s[4:11] th:TH_STORE_BYPASS scope:SCOPE_SYS ; encoding: [0x01,0x40,0x71,0xd0,0x00,0x00,0x3c,0x7c,0x00,0x04,0x7c,0x7c] | ||
|
|
||
| tensor_store_from_lds s[0:3], s[4:11], null, null th:TH_STORE_NT_HT scope:SCOPE_DEV |
There was a problem hiding this comment.
There seems to be some difference between this and SP3:
tensor_store_from_lds s[0:3], s[4:11], null, null dmask:0x1 th:TH_STORE_NT_HT scope:SCOPE_DEV // 000000000000: D0714001 7C680000 7C7C0400
There was a problem hiding this comment.
Hm you are right, that 0x71 should be 0x31
There was a problem hiding this comment.
This is also incorrect -
tensor_store_from_lds s[0:3], s[4:11] th:TH_STORE_BYPASS scope:SCOPE_SYS
// GFX12-ERR: :[[@line-1]]:1: error: instruction not supported on this GPU (gfx1200): tensor_store_from_lds
// GFX1250: tensor_store_from_lds s[0:3], s[4:11] th:TH_STORE_BYPASS scope:SCOPE_SYS ; encoding: [0x01,0x40,0x71,0xd0,0x00,0x00,0x3c,0x7c,0x00,0x04,0x7c,0x7c]
It has the same hex, and just leaves off the opnds (which btw, seems like in this case the sp3 does not leave them off, not specifying null is an error)
There was a problem hiding this comment.
Are we missing something about the reasoning for this in the tablegen -
let dmask = 1; // sp3
let dim = 1; // sp3
There was a problem hiding this comment.
According to SPG these fields are unused and shall be set to 0.
There was a problem hiding this comment.
Right so it's a mistake they're set to 1 right now yeah
|
Ping, good to merge this, and fix dim/dmask separately? |
3 participants
NULL is equivalent to passing a block of SGPRs that are set to zero, and is allowed