Skip to content

Conversation

Pierre-vh
Copy link
Contributor

No description provided.

@Pierre-vh Pierre-vh requested review from arsenm and jayfoad September 25, 2025 09:17
Copy link
Contributor Author

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

@Pierre-vh Pierre-vh marked this pull request as ready for review September 25, 2025 09:17
@Pierre-vh
Copy link
Contributor Author

This is only used by SIInsertWaitCnt, not sure how to test it? There is no dedicated test for https://reviews.llvm.org/D153295 I believe

@llvmbot
Copy link
Member

llvmbot commented Sep 25, 2025

@llvm/pr-subscribers-backend-amdgpu

Author: Pierre van Houtryve (Pierre-vh)

Changes

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

1 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/SIInstrInfo.cpp (+4-2)
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
index 044ea866342c2..a98dc3fe7552f 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -4319,8 +4319,10 @@ bool SIInstrInfo::mayAccessScratchThroughFlat(const MachineInstr &MI) const {
   if (!isFLAT(MI) || isFLATGlobal(MI))
     return false;
 
-  // If scratch is not initialized, we can never access it.
-  if (MI.getMF()->getFunction().hasFnAttribute("amdgpu-no-flat-scratch-init"))
+  // If scratch is not initialized, we can never access it unless the target has
+  // the ability to access scratch outside its own thread.
+  if (!ST.hasGloballyAddressableScratch() &&
+      MI.getMF()->getFunction().hasFnAttribute("amdgpu-no-flat-scratch-init"))
     return false;
 
   // SCRATCH instructions always access scratch.

Copy link
Contributor

@arsenm arsenm left a comment

Choose a reason for hiding this comment

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

Testcase?

@jayfoad
Copy link
Contributor

jayfoad commented Sep 25, 2025

This is only used by SIInsertWaitCnt

It was also used in SIMemoryLegalizer but I see you removed that in #157640. Prior to that, mayAccessScratchThroughFlat lived in SIInsertWaitcnts.cpp.

For the use in SIInsertWaitcnts I think we only care about accesses to this wave's scratch, so I'm not sure any code change is needed here?

@jayfoad
Copy link
Contributor

jayfoad commented Sep 25, 2025

To explain a bit more: in normal wave termination, the hw waits for outstanding stores to complete, and then releases the wave's VGPRs and scratch. If you send MSG_DEALLOC_VGPRS before the end of the shader, then VGPRs and scratch will be released before the hw waits for outstanding stores to complete. If there were any outstanding stores to this wave's scratch then that will cause a problem.

@Pierre-vh
Copy link
Contributor Author

To explain a bit more: in normal wave termination, the hw waits for outstanding stores to complete, and then releases the wave's VGPRs and scratch. If you send MSG_DEALLOC_VGPRS before the end of the shader, then VGPRs and scratch will be released before the hw waits for outstanding stores to complete. If there were any outstanding stores to this wave's scratch then that will cause a problem.

Ah that makes sense, then indeed no code change is needed here.

@Pierre-vh Pierre-vh closed this Sep 25, 2025
@arsenm arsenm deleted the users/pierre-vh/gas-instrinfo branch September 25, 2025 13:21
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.

4 participants