Skip to content

[AMDGPU] Fix MC/Disassembler/AMDGPU/decode-err.txt. #96621

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

Merged
merged 3 commits into from
Jun 27, 2024
Merged

[AMDGPU] Fix MC/Disassembler/AMDGPU/decode-err.txt. #96621

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4c066a2
[AMDGPU] Fix MC/Disassembler/AMDGPU/decode-err.txt.
kosarev Jun 25, 2024
7688ca3
Use -filetype=null.
kosarev Jun 25, 2024
313c7cf
Fix failures.
kosarev Jun 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 10 additions & 9 deletions llvm/test/MC/Disassembler/AMDGPU/decode-err.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
# RUN: llvm-mc -triple=amdgcn -mcpu=gfx900 -disassemble -show-encoding < %s 2>&1 | FileCheck -check-prefix=GCN %s
# RUN: llvm-mc -triple=amdgcn -mcpu=gfx1100 -disassemble -show-encoding < %s 2>&1 | FileCheck -check-prefixes=GFX11,W32 %s
# RUN: llvm-mc -triple=amdgcn -mcpu=gfx1100 -mattr=-WavefrontSize32,+WavefrontSize64 -disassemble -show-encoding < %s 2>&1 | FileCheck -check-prefixes=GFX11,W64 %s
# RUN: llvm-mc -triple=amdgcn -mcpu=gfx1200 -disassemble -show-encoding < %s 2>&1 | FileCheck -check-prefix=GFX12 %s
# RUN: llvm-mc -triple=amdgcn -mcpu=gfx900 -disassemble -show-encoding -filetype=null < %s 2>&1 | FileCheck -check-prefix=GCN-ERR %s
# RUN: llvm-mc -triple=amdgcn -mcpu=gfx1100 -disassemble -show-encoding < %s 2>&1 | FileCheck -check-prefixes=W32 %s
# RUN: llvm-mc -triple=amdgcn -mcpu=gfx1100 -mattr=-WavefrontSize32,+WavefrontSize64 -disassemble -show-encoding < %s 2>&1 | FileCheck -check-prefixes=W64 %s
# RUN: llvm-mc -triple=amdgcn -mcpu=gfx1100 -disassemble -show-encoding -filetype=null < %s 2>&1 | FileCheck -check-prefix=GFX11-ERR %s
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The capitalized wavefrontsize features shouldn't even parse? You also should never need to specify the negated form

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You also should never need to specify the negated form

@arsenm why do you say that? If you don't specify the negated form, you can end up with both wavefrontsize32 and wavefrontsize64 set, which is sometimes harmless but sometimes not. Related: #86957

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The subtarget features are processed as a state machine assigning values to the subtarget variable as each case is encountered. Code shouldn't be relying on a boolean is this feature present, but checking the resulting value of WavefrontSize

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plenty of code goes straight to the feature bits:

$ grep -r getFeatureBits.*FeatureWavefrontSize lib/Target/AMDGPU/
lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:  if (isGFX10Plus() && getFeatureBits()[AMDGPU::FeatureWavefrontSize64] &&
lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:      !getFeatureBits()[AMDGPU::FeatureWavefrontSize32]) {
lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:      if (!getFeatureBits()[AMDGPU::FeatureWavefrontSize32])
lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:      if (!getFeatureBits()[AMDGPU::FeatureWavefrontSize64])
lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:      if (!getFeatureBits()[AMDGPU::FeatureWavefrontSize32])
lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:      if (!getFeatureBits()[AMDGPU::FeatureWavefrontSize64])
lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCKernelDescriptor.cpp:    if (STI->getFeatureBits().test(FeatureWavefrontSize32))
lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp:  if (STI->getFeatureBits().test(FeatureWavefrontSize16))
lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp:  if (STI->getFeatureBits().test(FeatureWavefrontSize32))
lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp:      STI->getFeatureBits().test(FeatureWavefrontSize32);
lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp:      STI->getFeatureBits().test(FeatureWavefrontSize32);
lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp:  bool IsWave32 = STI->getFeatureBits().test(FeatureWavefrontSize32);
lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp:  if (STI->getFeatureBits().test(FeatureWavefrontSize32)) {

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The capitalized wavefrontsize features shouldn't even parse?

It seems to get lowered before processing. Not sure if it should or shouldn't be done, though.

Re negations: that would be an unrelated change anyway, so I will submit this as is, if no objections.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plenty of code is broken?

# RUN: llvm-mc -triple=amdgcn -mcpu=gfx1200 -disassemble -show-encoding -filetype=null < %s 2>&1 | FileCheck -check-prefix=GFX12-ERR %s

# GCN: [[@LINE+1]]:1: warning: invalid instruction encoding
# GCN-ERR: [[@LINE+1]]:1: warning: invalid instruction encoding
0xdf,0x00,0x00,0x02

# this is s_singleuse_vdst 0x1234, which is only valid on gfx1150
# GFX11: [[@LINE+1]]:1: warning: invalid instruction encoding
# GFX11-ERR: [[@LINE+1]]:1: warning: invalid instruction encoding
0x34,0x12,0x93,0xbf

# this is s_waitcnt_vscnt exec_hi, 0x1234, which is valid on gfx11, but not on gfx12
# GFX12: [[@LINE+1]]:1: warning: invalid instruction encoding
# GFX12-ERR: [[@LINE+1]]:1: warning: invalid instruction encoding
0x34,0x12,0x7f,0xbc

# W32: v_dual_add_f32 v5, 0xaf123456, v2 :: v_dual_fmaak_f32 v6, v3, v1, 0xaf123456 ; encoding: [0xff,0x04,0x02,0xc9,0x03,0x03,0x06,0x05,0x56,0x34,0x12,0xaf]
Expand Down Expand Up @@ -39,11 +40,11 @@
0x10,0x40,0x40,0xcc,0x00,0x11,0x02,0x18 # src2 sgpr0

# this is ds_add_f32 with gds bit which is not valid on gfx12+
# GFX12: [[@LINE+1]]:1: warning: invalid instruction encoding
# GFX12-ERR: [[@LINE+1]]:1: warning: invalid instruction encoding
0x00,0x00,0x56,0xd8,0x00,0x01,0x00,0x00

# this is image_msaa_load where samp field for gfx12 VSAMPLE is not all zeros
# GFX12: [[@LINE+1]]:1: warning: invalid instruction encoding
# GFX12-ERR: [[@LINE+1]]:1: warning: invalid instruction encoding
0x06,0x00,0x46,0xe4,0x01,0x10,0x80,0x00,0x05,0x06,0x07,0x00

# This is ds_read_b32 with gds bit which is not valid on gfx90a.
Expand Down
Loading