Skip to content

[AMDGPU][AsmParser]: Use dummy operand for parsing buffer_ SWZ operand. #165305

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, 2025
Merged

[AMDGPU][AsmParser]: Use dummy operand for parsing buffer_ SWZ operand. #165305

merged 1 commit into from
Oct 28, 2025

Conversation

@jrbyrnes
Copy link
Contributor

@jrbyrnes jrbyrnes commented Oct 27, 2025

MCInstrDesc counts the SWZ operand for NumOperands -- thus, since we do not parse this into the MCInst operands, there will be a mismatch between MCInst.getNumOperands and MCInstrDesc.getNumOperands .

llvm-mca assumes that each operand counted by MCInstrDesc.getNumOperands will be present in MCInst.operands

llvm-project/llvm/lib/MCA/InstrBuilder.cpp

Line 324 in 263377a

unsigned NumVariadicOps = MCI.getNumOperands() - MCDesc.getNumOperands();

This parses a dummy operand for the buffer_loads as a placeholder for the implicit SWZ operand. This is similar to the parsing of tbuffer_ variants which automatically parse the dummy operand

llvm-project/llvm/utils/TableGen/AsmMatcherEmitter.cpp

Line 1853 in 263377a

// This may happen for operands that are tied to a suboperand of a

@jrbyrnes
[AMDGPU][AsmParser]: Use dummy operand for parsing buffer_ SWZ operand.
a1e811f 
Change-Id: I7866559e3fe3aad4158d658580d328a5aac2b423
@llvmbot
Copy link
Member

llvmbot commented Oct 27, 2025

@llvm/pr-subscribers-backend-amdgpu

Author: Jeffrey Byrnes (jrbyrnes)

Changes

MCInstrDesc counts the SWZ operand for NumOperands -- thus, since we do not parse this into the MCInst operands, there will be a mismatch between MCInst.getNumOperands and MCInstrDesc.getNumOperands .

llvm-mca assumes that each operand counted by MCInstrDesc.getNumOperands will be present in MCInst.operands

llvm-project/llvm/lib/MCA/InstrBuilder.cpp

Line 324 in 263377a

unsigned NumVariadicOps = MCI.getNumOperands() - MCDesc.getNumOperands();

This parses a dummy operand for the buffer_loads as a placeholder for the implicit SWZ operand. This is similar to the parsing of tbuffer_ variants which automatically parse the dummy operand

llvm-project/llvm/utils/TableGen/AsmMatcherEmitter.cpp

Line 1853 in 263377a

// This may happen for operands that are tied to a suboperand of a

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

3 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp (+3)
  • (added) llvm/test/MC/AMDGPU/buffer-op-swz-operand.s (+43)
  • (added) llvm/test/tools/llvm-mca/AMDGPU/buffer-op-swz-operand.s (+45)
diff --git a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
index 99ba04378ba2e..97eeaaae6ce44 100644
--- a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
+++ b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
@@ -9048,6 +9048,9 @@ void AMDGPUAsmParser::cvtMubufImpl(MCInst &Inst,
 
   addOptionalImmOperand(Inst, Operands, OptionalIdx, AMDGPUOperand::ImmTyOffset);
   addOptionalImmOperand(Inst, Operands, OptionalIdx, AMDGPUOperand::ImmTyCPol, 0);
+  // Parse a dummy operand as a placeholder for the SWZ operand. This enforces
+  // agreement between MCInstrDesc.getNumOperands and MCInst.getNumOperands.
+  Inst.addOperand(MCOperand::createImm(0));
 }
 
 //===----------------------------------------------------------------------===//
diff --git a/llvm/test/MC/AMDGPU/buffer-op-swz-operand.s b/llvm/test/MC/AMDGPU/buffer-op-swz-operand.s
new file mode 100644
index 0000000000000..8bd91484d149c
--- /dev/null
+++ b/llvm/test/MC/AMDGPU/buffer-op-swz-operand.s
@@ -0,0 +1,43 @@
+// RUN: llvm-mc -triple=amdgcn-amd-amdhsa -mcpu=gfx1100 --show-inst < %s | FileCheck %s
+
+// CHECK: .amdgcn_target "amdgcn-amd-amdhsa--gfx1100"
+buffer_load_dwordx4 v[0:3], v0, s[0:3], 0, offen offset:4092 slc
+// CHECK: buffer_load_b128 v[0:3], v0, s[0:3], 0 offen offset:4092 slc ; <MCInst #13135 BUFFER_LOAD_DWORDX4_OFFEN_gfx11
+// CHECK-NEXT: ;  <MCOperand Reg:10104>
+// CHECK-NEXT: ;  <MCOperand Reg:486>
+// CHECK-NEXT: ;  <MCOperand Reg:7754>
+// CHECK-NEXT: ;  <MCOperand Imm:0>
+// CHECK-NEXT: ;  <MCOperand Imm:4092>
+// CHECK-NEXT: ;  <MCOperand Imm:2>
+// CHECK-NEXT: ;  <MCOperand Imm:0>>
+buffer_store_dword v0, v1, s[0:3], 0 offen slc
+// CHECK: buffer_store_b32 v0, v1, s[0:3], 0 offen slc ; <MCInst #14553 BUFFER_STORE_DWORD_OFFEN_gfx11
+// CHECK-NEXT: ;  <MCOperand Reg:486>
+// CHECK-NEXT: ;  <MCOperand Reg:487>
+// CHECK-NEXT: ;  <MCOperand Reg:7754>
+// CHECK-NEXT: ;  <MCOperand Imm:0>
+// CHECK-NEXT: ;  <MCOperand Imm:0>
+// CHECK-NEXT: ;  <MCOperand Imm:2>
+// CHECK-NEXT: ;  <MCOperand Imm:0>>
+
+; tbuffer ops use autogenerate asm parsers
+tbuffer_load_format_xyzw v[0:3], v0, s[0:3], 0 format:[BUF_FMT_32_32_SINT] offen offset:4092 slc
+// CHECK: tbuffer_load_format_xyzw v[0:3], v0, s[0:3], 0 format:[BUF_FMT_32_32_SINT] offen offset:4092 slc ; <MCInst #34095 TBUFFER_LOAD_FORMAT_XYZW_OFFEN_gfx11
+// CHECK-NEXT: ;  <MCOperand Reg:10104>
+// CHECK-NEXT: ;  <MCOperand Reg:486>
+// CHECK-NEXT: ;  <MCOperand Reg:7754>
+// CHECK-NEXT: ;  <MCOperand Imm:0>
+// CHECK-NEXT: ;  <MCOperand Imm:4092>
+// CHECK-NEXT: ;  <MCOperand Imm:49>
+// CHECK-NEXT: ;  <MCOperand Imm:2>
+// CHECK-NEXT: ;  <MCOperand Imm:0>>
+tbuffer_store_d16_format_x v0, v1, s[0:3], 0 format:[BUF_FMT_10_10_10_2_SNORM] offen slc
+// CHECK: tbuffer_store_d16_format_x v0, v1, s[0:3], 0 format:[BUF_FMT_10_10_10_2_SNORM] offen slc ; <MCInst #34264 TBUFFER_STORE_FORMAT_D16_X_OFFEN_gfx11
+// CHECK-NEXT: ;  <MCOperand Reg:486>
+// CHECK-NEXT: ;  <MCOperand Reg:487>
+// CHECK-NEXT: ;  <MCOperand Reg:7754>
+// CHECK-NEXT: ;  <MCOperand Imm:0>
+// CHECK-NEXT: ;  <MCOperand Imm:0>
+// CHECK-NEXT: ;  <MCOperand Imm:33>
+// CHECK-NEXT: ;  <MCOperand Imm:2>
+// CHECK-NEXT: ;  <MCOperand Imm:0>>
diff --git a/llvm/test/tools/llvm-mca/AMDGPU/buffer-op-swz-operand.s b/llvm/test/tools/llvm-mca/AMDGPU/buffer-op-swz-operand.s
new file mode 100644
index 0000000000000..932d9d1cc48b5
--- /dev/null
+++ b/llvm/test/tools/llvm-mca/AMDGPU/buffer-op-swz-operand.s
@@ -0,0 +1,45 @@
+# NOTE: Assertions have been autogenerated by utils/update_mca_test_checks.py
+# RUN: llvm-mca -mtriple=amdgcn-amd-amdhsa -mcpu=gfx950 < %s | FileCheck %s
+
+buffer_load_dwordx4 v[30:33], v4, s[0:3], 0, offen offset:4092
+buffer_store_dword v0, v1, s[0:3], 0 offen
+
+# CHECK:      Iterations:        100
+# CHECK-NEXT: Instructions:      200
+# CHECK-NEXT: Total Cycles:      280
+# CHECK-NEXT: Total uOps:        200
+
+# CHECK:      Dispatch Width:    1
+# CHECK-NEXT: uOps Per Cycle:    0.71
+# CHECK-NEXT: IPC:               0.71
+# CHECK-NEXT: Block RThroughput: 2.0
+
+# CHECK:      Instruction Info:
+# CHECK-NEXT: [1]: #uOps
+# CHECK-NEXT: [2]: Latency
+# CHECK-NEXT: [3]: RThroughput
+# CHECK-NEXT: [4]: MayLoad
+# CHECK-NEXT: [5]: MayStore
+# CHECK-NEXT: [6]: HasSideEffects (U)
+
+# CHECK:      [1]    [2]    [3]    [4]    [5]    [6]    Instructions:
+# CHECK-NEXT:  1      80    1.00    *             U     buffer_load_dwordx4 v[30:33], v4, s[0:3], 0 offen offset:4092
+# CHECK-NEXT:  1      80    1.00           *      U     buffer_store_dword v0, v1, s[0:3], 0 offen
+
+# CHECK:      Resources:
+# CHECK-NEXT: [0]   - HWBranch
+# CHECK-NEXT: [1]   - HWExport
+# CHECK-NEXT: [2]   - HWLGKM
+# CHECK-NEXT: [3]   - HWSALU
+# CHECK-NEXT: [4]   - HWVALU
+# CHECK-NEXT: [5]   - HWVMEM
+# CHECK-NEXT: [6]   - HWXDL
+
+# CHECK:      Resource pressure per iteration:
+# CHECK-NEXT: [0]    [1]    [2]    [3]    [4]    [5]    [6]
+# CHECK-NEXT:  -      -      -      -      -     2.00    -
+
+# CHECK:      Resource pressure by instruction:
+# CHECK-NEXT: [0]    [1]    [2]    [3]    [4]    [5]    [6]    Instructions:
+# CHECK-NEXT:  -      -      -      -      -     1.00    -     buffer_load_dwordx4 v[30:33], v4, s[0:3], 0 offen offset:4092
+# CHECK-NEXT:  -      -      -      -      -     1.00    -     buffer_store_dword v0, v1, s[0:3], 0 offen

rampitec
rampitec approved these changes Oct 27, 2025
View reviewed changes
Copy link
Collaborator

@rampitec rampitec left a comment

Choose a reason for hiding this comment

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

LGTM. I assume this is related to recently reported llvm-mca crash.

@jrbyrnes
Copy link
Contributor Author

jrbyrnes commented Oct 27, 2025

I assume this is related to recently reported llvm-mca crash.

Yes, llvm-mca assumes each operand counted by MCInstrDesc numOperands will be in MCInst.operands. By not parsing a dummy for the SWZ, we violate this assumption and llvm-mca fails.

@jrbyrnes
Copy link
Contributor Author

jrbyrnes commented Oct 28, 2025

I can't get the "CI Checks / Build and Test Linux AArch64" job to pass, but it looks like this bot is just down -- has been faililng for all the recent PRs with the same "The self-hosted runner lost communication with the server". Seems unrelated to this PR.

@jrbyrnes jrbyrnes merged commit 66b4815 into llvm:main Oct 28, 2025
23 of 28 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Oct 28, 2025

LLVM Buildbot has detected a new failure on builder clang-aarch64-quick running on linaro-clang-aarch64-quick while building llvm at step 5 "ninja check 1".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/65/builds/24604

Here is the relevant piece of the build log for the reference 
Step 5 (ninja check 1) failure: stage 1 checked (failure)
******************** TEST 'Clangd Unit Tests :: ./ClangdTests/70/335' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:/home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/tools/clang/tools/extra/clangd/unittests/./ClangdTests-Clangd Unit Tests-3417469-70-335.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=335 GTEST_SHARD_INDEX=70 /home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/tools/clang/tools/extra/clangd/unittests/./ClangdTests
--

Script:
--
/home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/tools/clang/tools/extra/clangd/unittests/./ClangdTests --gtest_filter=LSPTest.Diagnostics
--
�[0;34m<<< initialize: {}
�[0m�[0;33m<-- initialize(0)
�[0m�[0;33m--> reply:initialize(0) 119 ms
�[0m�[0;34m>>> reply: {
  "capabilities": {
    "astProvider": true,
    "callHierarchyProvider": true,
    "clangdInlayHintsProvider": true,
    "codeActionProvider": true,
    "compilationDatabase": {
      "automaticReload": true
    },
    "completionProvider": {
      "resolveProvider": false,
      "triggerCharacters": [
        ".",
        "<",
        ">",
        ":",
        "\"",
        "/",
        "*"
      ]
    },
    "declarationProvider": true,
    "definitionProvider": true,
    "documentFormattingProvider": true,
    "documentHighlightProvider": true,
    "documentLinkProvider": {
      "resolveProvider": false
    },
    "documentOnTypeFormattingProvider": {
      "firstTriggerCharacter": "\n",
      "moreTriggerCharacter": []
    },
    "documentRangeFormattingProvider": {
      "rangesSupport": true
    },
    "documentSymbolProvider": true,
    "executeCommandProvider": {
...

@llvm-ci
Copy link
Collaborator

llvm-ci commented Oct 28, 2025

LLVM Buildbot has detected a new failure on builder clang-ppc64le-linux-test-suite running on ppc64le-clang-test-suite while building llvm at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/95/builds/18975

Here is the relevant piece of the build log for the reference 
Step 6 (test-build-unified-tree-check-all) failure: 1200 seconds without output running [b'ninja', b'check-all'], attempting to kill
...
0.800 [2/3/1] Building CXX object compiler-rt/lib/ubsan/CMakeFiles/RTUbsan_dynamic_version_script_dummy.powerpc64le.dir/dummy.cpp.o
0.915 [1/3/2] Building CXX object compiler-rt/lib/asan/CMakeFiles/RTAsan_dynamic_version_script_dummy.powerpc64le.dir/dummy.cpp.o
1.765 [0/3/3] Linking CXX shared library /home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-test-suite/clang-ppc64le-test-suite/build/lib/clang/22/lib/powerpc64le-unknown-linux-gnu/libclang_rt.ubsan_standalone.so
3.360 [0/2/4] Linking CXX shared library /home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-test-suite/clang-ppc64le-test-suite/build/lib/clang/22/lib/powerpc64le-unknown-linux-gnu/libclang_rt.asan.so
5.755 [0/1/5] Generating /home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-test-suite/clang-ppc64le-test-suite/build/compile_commands.json
57.308 [3/78/12] cd /home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-test-suite/clang-ppc64le-test-suite/build/runtimes/runtimes-bins && /home/buildbots/llvm-external-buildbots/cmake-3.31.2/bin/cmake --build /home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-test-suite/clang-ppc64le-test-suite/build/runtimes/runtimes-bins/ --target runtimes-test-depends --config Release
ninja: no work to do.
57.821 [2/78/13] No install step for 'runtimes'
58.231 [1/78/15] Completed 'runtimes'
440.822 [1/77/16] Preparing lit tests
command timed out: 1200 seconds without output running [b'ninja', b'check-all'], attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=1641.482621

Lukacma pushed a commit to Lukacma/llvm-project that referenced this pull request Oct 29, 2025
@jrbyrnes @Lukacma
[AMDGPU][AsmParser]: Use dummy operand for parsing buffer_ SWZ operan…
864eead 
…d. (llvm#165305)

`MCInstrDesc` counts the SWZ operand for `NumOperands` -- thus, since we
do not parse this into the `MCInst` operands, there will be a mismatch
between `MCInst.getNumOperands` and `MCInstrDesc.getNumOperands` .

`llvm-mca` assumes that each operand counted by
`MCInstrDesc.getNumOperands` will be present in `MCInst.operands`
https://github.com/llvm/llvm-project/blob/263377a17570e1cbe6eeae9ffa5ce02f240839ef/llvm/lib/MCA/InstrBuilder.cpp#L324

This parses a dummy operand for the buffer_loads as a placeholder for
the implicit SWZ operand. This is similar to the parsing of `tbuffer_`
variants which automatically parse the dummy operand
https://github.com/llvm/llvm-project/blob/263377a17570e1cbe6eeae9ffa5ce02f240839ef/llvm/utils/TableGen/AsmMatcherEmitter.cpp#L1853
aokblast pushed a commit to aokblast/llvm-project that referenced this pull request Oct 30, 2025
@jrbyrnes @aokblast
[AMDGPU][AsmParser]: Use dummy operand for parsing buffer_ SWZ operan…
6751a2c 
…d. (llvm#165305)

`MCInstrDesc` counts the SWZ operand for `NumOperands` -- thus, since we
do not parse this into the `MCInst` operands, there will be a mismatch
between `MCInst.getNumOperands` and `MCInstrDesc.getNumOperands` .

`llvm-mca` assumes that each operand counted by
`MCInstrDesc.getNumOperands` will be present in `MCInst.operands`
https://github.com/llvm/llvm-project/blob/263377a17570e1cbe6eeae9ffa5ce02f240839ef/llvm/lib/MCA/InstrBuilder.cpp#L324

This parses a dummy operand for the buffer_loads as a placeholder for
the implicit SWZ operand. This is similar to the parsing of `tbuffer_`
variants which automatically parse the dummy operand
https://github.com/llvm/llvm-project/blob/263377a17570e1cbe6eeae9ffa5ce02f240839ef/llvm/utils/TableGen/AsmMatcherEmitter.cpp#L1853
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rampitec rampitec rampitec approved these changes

@arsenm arsenm Awaiting requested review from arsenm

@kosarev kosarev Awaiting requested review from kosarev

Assignees

No one assigned

Labels

backend:AMDGPU

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jrbyrnes @llvmbot @llvm-ci @rampitec