Skip to content

Conversation

arsenm
Copy link
Contributor

@arsenm arsenm commented Sep 2, 2025

No description provided.

@arsenm arsenm requested a review from shiltian September 2, 2025 02:31
Copy link
Contributor Author

arsenm commented Sep 2, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@arsenm arsenm marked this pull request as ready for review September 2, 2025 02:31
@llvmbot
Copy link
Member

llvmbot commented Sep 2, 2025

@llvm/pr-subscribers-backend-amdgpu

Author: Matt Arsenault (arsenm)

Changes

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

1 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/DSInstructions.td (+1)
diff --git a/llvm/lib/Target/AMDGPU/DSInstructions.td b/llvm/lib/Target/AMDGPU/DSInstructions.td
index 3703133126b0f..7a22ae0c401e2 100644
--- a/llvm/lib/Target/AMDGPU/DSInstructions.td
+++ b/llvm/lib/Target/AMDGPU/DSInstructions.td
@@ -19,6 +19,7 @@ class DS_Pseudo <string opName, dag outs, dag ins, string asmOps, list<dag> patt
   // Most instruction load and store data, so set this as the default.
   let mayLoad = 1;
   let mayStore = 1;
+  let FixedSize = true;
 
   let hasSideEffects = 0;
   let SchedRW = [WriteLDS];

Copy link
Contributor

@jayfoad jayfoad left a comment

Choose a reason for hiding this comment

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

Seems reasonable. Is this NFC? Could also do this on pretty much everything except SOP* and VOP*.

@arsenm
Copy link
Contributor Author

arsenm commented Sep 2, 2025

Seems reasonable. Is this NFC? Could also do this on pretty much everything except SOP* and VOP*.

Yes. It happened to bypass a crash I ran into on a later patch, and replaced it with a friendlier error later

Copy link
Contributor

@jayfoad jayfoad left a comment

Choose a reason for hiding this comment

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

OK but I wish we had better verification of these size calculations. This check does approximately nothing since it is skipped for pseudos but MI is always a pseudo here:

assert(CodeBytes.size() == STI.getInstrInfo()->getInstSizeInBytes(*MI));

@arsenm arsenm merged commit 372a86b into main Sep 2, 2025
13 checks passed
@arsenm arsenm deleted the users/arsenm/amdgpu/mark-ds-instructions-fixed-size branch September 2, 2025 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants