[clang] [llvm] [HLSL] Implement SV_GroupThreadId semantic (PR #117781)

[Tex Riddell via llvm-commits]

================
@@ -2851,7 +2851,21 @@ bool SPIRVInstructionSelector::selectIntrinsic(Register ResVReg,
           .constrainAllUses(TII, TRI, RBI);
     break;
   case Intrinsic::spv_thread_id:
-    return selectSpvThreadId(ResVReg, ResType, I);
+    // The HLSL SV_DispatchThreadID semantic is lowered to llvm.spv.thread.id
+    // intrinsic in LLVM IR for SPIR-V backend.
+    //
+    // In SPIR-V backend, llvm.spv.thread.id is now correctly translated to a
+    // `GlobalInvocationId` builtin variable
----------------
tex3d wrote:

These comments seem redundant to me.  We are in the case handling `llvm.spv.thread.id` and the code translates it to use the `GlobalInvocationId` builtin variable.  What else needs to be said in this context?

"is now correctly translated to ..." as opposed to when?  What does this comment add/clarify?

As far as the HLSL context is concerned, that's defined elsewhere, and it's presumably not in scope for the SPIR-V backend to comment on where an intrinsic may come from in a particular source language.  I can imagine similar expository comments for many other intrinsics being lowered here, but I don't think this is the best place to document such things.

Note: this is just an opinion, feel free to disagree, and I'm curious how others more experienced with the backends feel as well.

https://github.com/llvm/llvm-project/pull/117781