[PATCH] D136169: [AMDGPU] Avoid SCC clobbering before S_CSELECT_B32

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp:843-846
+    auto Add = BuildMI(*MBB, Ins, DL, TII->get(AMDGPU::S_ADD_I32), BaseReg)
         .addReg(OffsetReg, RegState::Kill)
         .addReg(FIReg);
+        Add->getOperand(3).setIsDead(); // Mark SCC as dead.
----------------
This isn't harmful, but I also doubt helps you. This is only used by LocalStackSlotAllocation and runs before dead flags should automatically be computed


================
Comment at: llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp:2235
+        while (I != MBB->end()) {
+          if (*I != MI && I->getOpcode() ==  AMDGPU::S_CSELECT_B32) {
+            NeedSaveSCC = true;
----------------
Shouldn't special case s_cselect*


================
Comment at: llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp:2247
+          SCCCopy = RS->scavengeRegister(getBoolRC(), MI, 0, !UseSGPR);
+          if (SCCCopy != AMDGPU::NoRegister)
+            BuildMI(*MBB, MI, DL,
----------------
foad wrote:
> Don't need the ` != AMDGPU::NoRegister` because Register converts to bool.
> 
> Need braces around the multi-line BuildMI call.
So this just remains broken if the scavenge failed?


================
Comment at: llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp:2378
                 .addImm(Offset);
+                Add->getOperand(3).setIsDead(); // Mark SCC as dead.
             if (!IsSALU)
----------------
These dead setting changes should be done separately 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136169