Skip to content

[AMDGPU][MC] Fix disassembler for VIMAGE when non-first vaddr is v0 #113569

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Oct 28, 2024
Merged

[AMDGPU][MC] Fix disassembler for VIMAGE when non-first vaddr is v0 #113569

merged 1 commit into from
Oct 28, 2024

Conversation

mbrkusanin
Copy link
Collaborator

For disassembler tables we use V1_V4 variants for VIMAGE and then remove
unused vaddr fields. V1_V1 variant, which has every vaddr field other than
vaddr0 set to 0, was also enabled and caused confusion when decoding cases
which used v0 (whose encoded value is 0)

@llvmbot llvmbot added backend:AMDGPU mc Machine (object) code labels Oct 24, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 24, 2024 •
edited
Loading

@llvm/pr-subscribers-mc

@llvm/pr-subscribers-backend-amdgpu

Author: Mirko Brkušanin (mbrkusanin)

Changes

For disassembler tables we use V1_V4 variants for VIMAGE and then remove
unused vaddr fields. V1_V1 variant, which has every vaddr field other than
vaddr0 set to 0, was also enabled and caused confusion when decoding cases
which used v0 (whose encoded value is 0)

Full diff: https://github.com/llvm/llvm-project/pull/113569.diff

3 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/MIMGInstructions.td (+4-6)
  • (modified) llvm/test/MC/AMDGPU/gfx12_asm_vimage.s (+12)
  • (modified) llvm/test/MC/Disassembler/AMDGPU/gfx12_dasm_vimage.txt (+12)
diff --git a/llvm/lib/Target/AMDGPU/MIMGInstructions.td b/llvm/lib/Target/AMDGPU/MIMGInstructions.td
index 5c49a8116ae7fc..2740c9465b55b8 100644
--- a/llvm/lib/Target/AMDGPU/MIMGInstructions.td
+++ b/llvm/lib/Target/AMDGPU/MIMGInstructions.td
@@ -567,8 +567,7 @@ multiclass MIMG_NoSampler_Src_Helper <mimgopc op, string asm,
           def _V1_gfx12 : VSAMPLE_Sampler_gfx12<op, asm, dst_rc, 1>;
       }
       else {
-        def _V1_gfx12 : VIMAGE_NoSampler_gfx12<op, asm, dst_rc, 1,
-                                               !if(enableDisasm, "GFX12", "")>;
+        def _V1_gfx12 : VIMAGE_NoSampler_gfx12<op, asm, dst_rc, 1>;
       }
     }
   }
@@ -789,8 +788,7 @@ multiclass MIMG_Store_Addr_Helper <mimgopc op, string asm,
         }
       }
       if op.HAS_GFX12 then {
-        def _V1_gfx12 : VIMAGE_Store_gfx12 <op, asm, data_rc, 1,
-                                            !if(enableDisasm, "GFX12", "")>;
+        def _V1_gfx12 : VIMAGE_Store_gfx12 <op, asm, data_rc, 1>;
       }
     }
     let VAddrDwords = 2 in {
@@ -1017,9 +1015,9 @@ multiclass MIMG_Atomic_Addr_Helper_m <mimgopc op, string asm,
       }
       if op.HAS_GFX12 then {
         if !empty(renamed) then
-          def _V1_gfx12 : VIMAGE_Atomic_gfx12 <op, asm, data_rc, 1, enableDasm>;
+          def _V1_gfx12 : VIMAGE_Atomic_gfx12 <op, asm, data_rc, 1>;
         else
-          def _V1_gfx12 : VIMAGE_Atomic_gfx12_Renamed <op, renamed, data_rc, 1, enableDasm>;
+          def _V1_gfx12 : VIMAGE_Atomic_gfx12_Renamed <op, renamed, data_rc, 1>;
       }
     }
     let VAddrDwords = 2 in {
diff --git a/llvm/test/MC/AMDGPU/gfx12_asm_vimage.s b/llvm/test/MC/AMDGPU/gfx12_asm_vimage.s
index 39010883a3c0b8..196d75db426052 100644
--- a/llvm/test/MC/AMDGPU/gfx12_asm_vimage.s
+++ b/llvm/test/MC/AMDGPU/gfx12_asm_vimage.s
@@ -155,6 +155,9 @@ image_load v[0:2], [v4, v5], s[8:15] dmask:0xf dim:SQ_RSRC_IMG_2D_ARRAY th:TH_LO
 image_load v[0:2], [v4, v5], s[8:15] dmask:0xf dim:SQ_RSRC_IMG_2D_ARRAY th:TH_LOAD_HT scope:SCOPE_SE r128 a16 tfe d16
 // GFX12: encoding: [0x75,0x00,0xc0,0xd3,0x00,0x10,0xa4,0x00,0x04,0x05,0x00,0x00]
 
+image_load v[4:7], [v1, v0], s[4:11] dmask:0xf dim:SQ_RSRC_IMG_2D
+// GFX12: encoding: [0x01,0x00,0xc0,0xd3,0x04,0x08,0x00,0x00,0x01,0x00,0x00,0x00]
+
 image_load_mip v[252:255], [v0, v1], s[0:7] dmask:0xf dim:SQ_RSRC_IMG_1D
 // GFX12: encoding: [0x00,0x40,0xc0,0xd3,0xfc,0x00,0x00,0x00,0x00,0x01,0x00,0x00]
 
@@ -402,6 +405,9 @@ image_store v0, v0, s[0:7] dmask:0x1 dim:SQ_RSRC_IMG_1D th:TH_STORE_NT_WB
 image_store v0, v0, s[0:7] dmask:0x1 dim:SQ_RSRC_IMG_1D th:TH_STORE_BYPASS scope:SCOPE_SYS
 // GFX12: encoding: [0x00,0x80,0x41,0xd0,0x00,0x00,0x3c,0x00,0x00,0x00,0x00,0x00]
 
+image_store v[1:4], [v2, v0], s[4:11] dmask:0xf dim:SQ_RSRC_IMG_2D
+// GFX12: encoding: [0x01,0x80,0xc1,0xd3,0x01,0x08,0x00,0x00,0x02,0x00,0x00,0x00]
+
 image_store_mip v[252:255], [v0, v1], s[0:7] dmask:0xf dim:SQ_RSRC_IMG_1D
 // GFX12: encoding: [0x00,0xc0,0xc1,0xd3,0xfc,0x00,0x00,0x00,0x00,0x01,0x00,0x00]
 
@@ -559,6 +565,9 @@ image_atomic_swap v[3:4], [v4, v5], s[8:15] dmask:0x3 dim:SQ_RSRC_IMG_2D_MSAA a1
 image_atomic_swap v[254:255], [v4, v5], s[96:103] dmask:0x3 dim:SQ_RSRC_IMG_2D_MSAA_ARRAY a16
 // GFX12: encoding: [0x47,0x80,0xc2,0xd0,0xfe,0xc0,0x00,0x00,0x04,0x05,0x00,0x00]
 
+image_atomic_swap v1, [v2, v0], s[4:11] dmask:0x1 dim:SQ_RSRC_IMG_2D
+// GFX12: encoding: [0x01,0x80,0x42,0xd0,0x01,0x08,0x00,0x00,0x02,0x00,0x00,0x00]
+
 image_atomic_cmpswap v[0:1], v0, s[0:7] dmask:0x3 dim:SQ_RSRC_IMG_1D
 // GFX12: encoding: [0x00,0xc0,0xc2,0xd0,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00]
 
@@ -613,6 +622,9 @@ image_atomic_add_uint v[254:255], [v4, v5, v6, v7], s[96:103] dmask:0x3 dim:SQ_R
 image_atomic_add_uint v0, v0, s[0:7] dmask:0x1 dim:SQ_RSRC_IMG_1D th:TH_ATOMIC_NT
 // GFX12: encoding: [0x00,0x00,0x43,0xd0,0x00,0x00,0x20,0x00,0x00,0x00,0x00,0x00]
 
+image_atomic_add_uint v1, [v2, v0], s[4:11] dmask:0x1 dim:SQ_RSRC_IMG_2D
+// GFX12: encoding: [0x01,0x00,0x43,0xd0,0x01,0x08,0x00,0x00,0x02,0x00,0x00,0x00]
+
 image_atomic_sub_uint v0, v0, s[0:7] dmask:0x1 dim:SQ_RSRC_IMG_1D
 // GFX12: encoding: [0x00,0x40,0x43,0xd0,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00]
 
diff --git a/llvm/test/MC/Disassembler/AMDGPU/gfx12_dasm_vimage.txt b/llvm/test/MC/Disassembler/AMDGPU/gfx12_dasm_vimage.txt
index aa49caacb4fccd..08e9bef8cf6785 100644
--- a/llvm/test/MC/Disassembler/AMDGPU/gfx12_dasm_vimage.txt
+++ b/llvm/test/MC/Disassembler/AMDGPU/gfx12_dasm_vimage.txt
@@ -157,6 +157,9 @@
 # GFX12: image_load v[0:2], [v4, v5], s[8:15] dmask:0xf dim:SQ_RSRC_IMG_2D_ARRAY th:TH_LOAD_HT scope:SCOPE_SE r128 a16 tfe d16 ; encoding: [0x75,0x00,0xc0,0xd3,0x00,0x10,0xa4,0x00,0x04,0x05,0x00,0x00]
 0x75,0x00,0xc0,0xd3,0x00,0x10,0xa4,0x00,0x04,0x05,0x00,0x00
 
+# GFX12: image_load v[4:7], [v1, v0], s[4:11] dmask:0xf dim:SQ_RSRC_IMG_2D ; encoding: [0x01,0x00,0xc0,0xd3,0x04,0x08,0x00,0x00,0x01,0x00,0x00,0x00]
+0x01,0x00,0xc0,0xd3,0x04,0x08,0x00,0x00,0x01,0x00,0x00,0x00
+
 # GFX12: image_load_mip v[252:255], [v0, v1], s[0:7] dmask:0xf dim:SQ_RSRC_IMG_1D ; encoding: [0x00,0x40,0xc0,0xd3,0xfc,0x00,0x00,0x00,0x00,0x01,0x00,0x00]
 0x00,0x40,0xc0,0xd3,0xfc,0x00,0x00,0x00,0x00,0x01,0x00,0x00
 
@@ -403,6 +406,9 @@
 # GFX12: image_store v0, v0, s[0:7] dmask:0x1 dim:SQ_RSRC_IMG_1D th:TH_STORE_BYPASS scope:SCOPE_SYS ; encoding: [0x00,0x80,0x41,0xd0,0x00,0x00,0x3c,0x00,0x00,0x00,0x00,0x00]
 0x00,0x80,0x41,0xd0,0x00,0x00,0x3c,0x00,0x00,0x00,0x00,0x00
 
+# GFX12: image_store v[1:4], [v2, v0], s[4:11] dmask:0xf dim:SQ_RSRC_IMG_2D ; encoding: [0x01,0x80,0xc1,0xd3,0x01,0x08,0x00,0x00,0x02,0x00,0x00,0x00]
+0x01,0x80,0xc1,0xd3,0x01,0x08,0x00,0x00,0x02,0x00,0x00,0x00
+
 # GFX12: image_store_mip v[252:255], [v0, v1], s[0:7] dmask:0xf dim:SQ_RSRC_IMG_1D ; encoding: [0x00,0xc0,0xc1,0xd3,0xfc,0x00,0x00,0x00,0x00,0x01,0x00,0x00]
 0x00,0xc0,0xc1,0xd3,0xfc,0x00,0x00,0x00,0x00,0x01,0x00,0x00
 
@@ -559,6 +565,9 @@
 # GFX12: image_atomic_swap v[254:255], [v4, v5], s[96:103] dmask:0x3 dim:SQ_RSRC_IMG_2D_MSAA_ARRAY a16 ; encoding: [0x47,0x80,0xc2,0xd0,0xfe,0xc0,0x00,0x00,0x04,0x05,0x00,0x00]
 0x47,0x80,0xc2,0xd0,0xfe,0xc0,0x00,0x00,0x04,0x05,0x00,0x00
 
+# GFX12: image_atomic_swap v1, [v2, v0], s[4:11] dmask:0x1 dim:SQ_RSRC_IMG_2D ; encoding: [0x01,0x80,0x42,0xd0,0x01,0x08,0x00,0x00,0x02,0x00,0x00,0x00]
+0x01,0x80,0x42,0xd0,0x01,0x08,0x00,0x00,0x02,0x00,0x00,0x00
+
 # GFX12: image_atomic_cmpswap v[0:1], v0, s[0:7] dmask:0x3 dim:SQ_RSRC_IMG_1D ; encoding: [0x00,0xc0,0xc2,0xd0,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00]
 0x00,0xc0,0xc2,0xd0,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00
 
@@ -613,6 +622,9 @@
 # GFX12: image_atomic_add_uint v0, v0, s[0:7] dmask:0x1 dim:SQ_RSRC_IMG_1D th:TH_ATOMIC_NT ; encoding: [0x00,0x00,0x43,0xd0,0x00,0x00,0x20,0x00,0x00,0x00,0x00,0x00]
 0x00,0x00,0x43,0xd0,0x00,0x00,0x20,0x00,0x00,0x00,0x00,0x00
 
+# GFX12: image_atomic_add_uint v1, [v2, v0], s[4:11] dmask:0x1 dim:SQ_RSRC_IMG_2D ; encoding: [0x01,0x00,0x43,0xd0,0x01,0x08,0x00,0x00,0x02,0x00,0x00,0x00]
+0x01,0x00,0x43,0xd0,0x01,0x08,0x00,0x00,0x02,0x00,0x00,0x00
+
 # GFX12: image_atomic_sub_uint v0, v0, s[0:7] dmask:0x1 dim:SQ_RSRC_IMG_1D ; encoding: [0x00,0x40,0x43,0xd0,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00]
 0x00,0x40,0x43,0xd0,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00

@mbrkusanin
[AMDGPU][MC] Fix disassembler for VIMAGE when non-first vaddr is v0
87286d9 
For disassembler tables we use *V1_V4* variants for VIMAGE and then remove
unused vaddr fields. *V1_V1* variant, which has every vaddr field other than
vaddr0 set to 0, was also enabled and caused confusion when decoding cases
which used v0 (whose encoded value is 0)
@mbrkusanin mbrkusanin force-pushed the fix-disasm-vimage-zero branch from c0111e2 to 87286d9 Compare October 25, 2024 15:33
@mbrkusanin mbrkusanin merged commit fa4790e into llvm:main Oct 28, 2024
8 checks passed
@mbrkusanin mbrkusanin deleted the fix-disasm-vimage-zero branch October 28, 2024 09:43
NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this pull request Nov 4, 2024
@mbrkusanin @NoumanAmir657
[AMDGPU][MC] Fix disassembler for VIMAGE when non-first vaddr is v0 (l…
3d70857 
…lvm#113569)

For disassembler tables we use *V1_V4* variants for VIMAGE and then
remove unused vaddr fields. *V1_V1* variant, which has every vaddr
field other than vaddr0 set to 0, was also enabled and caused confusion
when decoding cases which used v0 (whose encoded value is 0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arsenm arsenm arsenm approved these changes

Assignees
No one assigned
Labels
backend:AMDGPU mc Machine (object) code
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mbrkusanin @llvmbot @arsenm