[PATCH] D143707: [AMDGPU] Allow architected SGPRs for workgroup IDs

[Christudasan Devadasan via Phabricator via llvm-commits]

cdevadas added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:2254
   assert(!Subtarget->hasUserSGPRInit16Bug() || IsShader ||
          Info.getNumPreloadedSGPRs() >= 16);
 }
----------------
Also, should I guard this check with HasArchitectedSGPRS?
The numPreloaded regular SGPRs will vary if TTMP registers are used for workgroup IDs. 


================
Comment at: llvm/test/CodeGen/AMDGPU/workgroup-id-in-arch-sgprs.ll:26
+  %idx = call i32 @llvm.amdgcn.workgroup.id.x()
+  store volatile i32 %idx, ptr addrspace(1) %ptrx
+
----------------
foad wrote:
> Why are all these stores volatile?
Will change.


================
Comment at: llvm/test/CodeGen/AMDGPU/workgroup-id-in-arch-sgprs.ll:53
+; GCN-GISEL-NEXT:    s_waitcnt vmcnt(0)
+; GCN-GISEL-NEXT:    v_mov_b32_e32 v1, ttmp7
+; GCN-GISEL-NEXT:    global_store_dword v0, v1, s[2:3]
----------------
foad wrote:
> Doesn't this value need to be ANDed with 0xffff?
If there is no WorkGroupIDZ, I thought I can skip that part.
Just like the packedTID here.
https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/AMDGPU/SIISelLowering.cpp#L1934


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143707/new/

https://reviews.llvm.org/D143707