[AMDGPU] Avoid SCC clobbering before S_CSELECT_B32

[alex-t]

Summary

Frame lowering inserts scalar addition to compute the offset to the
stack objects. This instructions inserted in arbitrary place and may clobber
SCC between its definition and S_CSELECT_B32 instruction. This change
workarounds this particular code pattern. It queries the scavenger for SGPR and
if available saves SCC to it and restore its value after frame lowering code
insertion.

[foad]

It queries the scavenger for SGPR and
if available saves SCC to it and restore its value after frame lowering code
insertion.

What if there is no free SGPR? I don't think it's acceptable to generate broken code in that case.

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2245

Need braces and indentation for the body of this "if".

[foad]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

846

Weird indentation here. Can you run git clang-format on this change?

2246

Don't need the != AMDGPU::NoRegister because Register converts to bool.

Need braces around the multi-line BuildMI call.

[alex-t]

In D136169#3865217, @foad wrote:

It queries the scavenger for SGPR and
if available saves SCC to it and restore its value after frame lowering code
insertion.

What if there is no free SGPR? I don't think it's acceptable to generate broken code in that case.

I expected this question and deliberately leave this for discussion. I think that the proper behavior is to report error and bail out.
No alternatives look good to me.
Attempt to use V_ADD_* is unreliable as it assumes to scavenge register again which is also not guaranteed.
Moving the S_ADD_I32 insertion point may be unsolvable due to the data dependence problem

[arsenm]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

843–846

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

2246

So this just remains broken if the scavenge failed?

2247

Shouldn't special case s_cselect*

2375

These dead setting changes should be done separately

[alex-t]

Implementing error reporting if no SGPR scavenged. Formatting.

[alex-t]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2247

I see it looks like a dirty hack. And it is really. Otherwise, we will have a huge amount of SCC save/restore instructions along the code. Most of them will be unnecessary. In fact, only s_cselect and carry out instructions really use SCC but most of the SALU read it.
BTW I am curious why did not we hit any errors related to SCC clobbering before s_cselect patch?

[alex-t]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2246

For now, I would opt for reporting fatal here.
We could try to use v_add for those targets which allow v_add with the SGPR and constant if the frame index user accepts VGPR. This will give a chance in some cases but not in general.

[alex-t]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2247

Oops. Sorry I was wrong.
It is really no need to special case s_cselect here. I checked the TD files and we only have SCC uses where it is really necessary.

[alex-t]

SCC save/restore for all SCC users. Code which sets dead flag in SCC operands removed as it should be in separate change.

[foad]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2231–2232

This still makes me nervous. For graphics we use LLVM as a JIT compiler, and we definitely have cases where SGPRs are spilled (so presumably there are no free SGPRs). What are we supposed to do if this scavenge fails?

[alex-t]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2231–2232

What we were supposed to do if failed the scavenge at line 2198 and line 2243?
In fact, I have just followed the error handling strategy implemented for the same condition in this file earlier. I assumed that if it worked so far it should be okay further.

In general, no strategy exists that ensures we never run out of registers.

We can try to use vector addition with an SGPR and constant but it is possible for a few targets only. Otherwise, we'd need to scavenge VGPR but we already failed to scavenge SGPR which means that we were unable to spill one and probably has no VGPRs available.

The trick with using FrameRegister as a temporary room is already used by frame lowering itself, also SCC copy should be alive from the point before the S_ADD insertion and last to the point right after it.

Moving the insertion point before the SCC definition is restricted by the result register placement as follows:

(1) def R
(2) def SCC
(3) use R
<==== here we're about to insert R = S_ADD_I32 FR, Off
(4) use SCC

We cannot move the insertion point over another use of R.
In the general case, this is unsolvable.

My point is that:

1. "Failed to scavenge" recovery strategy is a complex problem that should be addressed separately.
2. There is no general solution as one always can invent the input on which the compiler runs out of registers.

Please correct me if I am wrong.

[alex-t]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2229

If we've failed to scavenge SGPR for SCC save/restore we never get here. But if we've got here we'd either fail to scavenge SGPR for the frame offset computation.

[alex-t]

A bit more compact code.

[foad]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2216–2224

What is this loop for? Isn't the scavenger already supposed to know whether SCC is live?

[foad]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2263

None of this code should depend on wave size. You don't want getBoolRC here - that's for VCC which might be a register pair in wave64. You can always save SCC in a single 32-bit SGPR.

[alex-t]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2216–2224

What is this loop for? Isn't the scavenger already supposed to know whether SCC is live?

The scavenger's knowledge depends on the accurate dead/kill flags insertion. This loop decreases the number of false-positive "live" SCC and accordingly unnecessary SCC saves/restores.

As Matt rightly pointed out, the dead/kill correct placement is out of this change scope.
It also is going to be a complex task.

Should I add a corresponding TODO here?

[alex-t]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2263

Since back SGPR to SCC copy depends on EXEC/EXEC_LO and I am using the same register for saving and restoring I need to use 64bit unless isWave32. I am not sure if I can use S_AND_B32 reg, exec_lo in wave64 mode?

[foad]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2216–2224

Haven't dead/kill flags been automatically computed before this code runs?

If you do need this loop, shouldn't it conservatively assume that SCC is live at the end of the basic block?

2263

You can just use "s_cmp_lg_u32 sgpr, 0" to copy back to SCC.

[alex-t]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2216–2224

Let me illustrate what I mean. For the code snipped below

(1) $sgpr3 = S_ADD_I32 $sgpr33, 32, implicit-def $scc
(2) $vgpr51 = V_ADD_U32_e64 killed $sgpr3, killed $sgpr43, 0, implicit $exec
(3) $sgpr3 = S_ADD_I32 $sgpr33, 32, implicit-def $scc
(4) $vgpr52 = V_ADD_U32_e64 killed $sgpr3, killed $sgpr44, 0, implicit $exec

scavenger considers SCC "live" between (1) and (3) because its internal iterator position points to (2). Calling the "advance" method inside the target hook breaks the outer PEI logic. Literally, PEI does not expect the scavenger state to be changed inside the TargetRegisterInfo::LowerFrameIndex method.

In this particular case, I can add the dead flag to the particular place:

auto Add = BuildMI(*MBB, MI, DL, TII->get(AMDGPU::S_ADD_I32), TmpSReg)
            .addReg(FrameReg)
            .addImm(Offset);
Add->getOperand(3).setIsDead();

Unfortunately, we have plenty of places like that. And for cases that are not as trivial, we are going to have plenty of odd code.

2216–2224

If you do need this loop, shouldn't it conservatively assume that SCC is live at the end of the basic block?

Good point, thanks.

2263

Given that this this is pure scalar context yes

[alex-t]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2216–2224

If you do need this loop, shouldn't it conservatively assume that SCC is live at the end of the basic block?

Good point, thanks.

No, this does not work really. Given that we have a BB with just one SCC def, following the idea we would insert

Some SCC def...
$sgpr38 = S_CSELECT_B32 -1, 0, implicit $scc
$sgpr8 = S_ADD_I32 $sgpr32, 2960, implicit-def $scc
S_CMP_LG_U32 $sgpr38, 0, implicit-def $scc

on each frame lowering S_ADD_I32 since each time we have S_CMP_LG_U32 defining SCC and no other defs to the end of the BB.
So, we will scavenge each time until we are out of registers.

[alex-t]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2216–2224

If you do need this loop, shouldn't it conservatively assume that SCC is live at the end of the basic block?

In fact, the loop aims to restrict a big amount of false positive SCC "live" slots by simulating the register scavenger "forward" job. But if SCC live-in in some BB it is still in scavenger "usedregs".

[alex-t]

SCC copy restore has been made wave size independent.

[foad]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2216–2224

The loop still sounds like a hack to me that will have O(n^2) cost in the worst case. But I really don't know enough about PEI or the RegisterScavenger to review this properly.

[alex-t]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2216–2224

The loop still sounds like a hack to me that will have O(n^2) cost in the worst case. But I really don't know enough about PEI or the RegisterScavenger to review this properly.

I don't like this approach either and would like to look for a neat solution. This is proposed as a workaround to stop Vulkan RT from failing.

Could you also clarify, why you consider it O(n^2)? The worst case is when we have to walk to the end of the BB. This is just on level loop and each instruction is visited once.

[foad]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2216–2224

In a large BB with n instructions that need frame index elimination, you will walk to end of the BB (worst case) for each one of them. So that is O(n^2) overall cost to run this pass.

[alex-t]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2216–2224

Ok, you meant the whole pass, not the loop itself. I got it now.

[alex-t]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2216–2224

PEI main loop process instructions one by one inserting the frame index lowering code before instruction if it has the frame index operand.
Register scavenger steps forward AFTER current instruction has been processed. That means we have the register's liveness information at the point right preceding the instruction being processed. In case the preceding instruction uses SCC, the scavenger considers it "live" even though the current instruction kills it. Without the loop, we'd have a lot of unnecessary SCC save/restore and run out of registers soon.
Consider the following example:

renamable $sgpr50 = S_CSELECT_B32 2924, killed renamable $sgpr50, implicit $scc ** <-- RS is here and RS->isRegUsed(AMDGPU::SCC) is true **
renamable $sgpr40 = S_ADD_I32 killed renamable $sgpr40, %stack.0.__llpc_global_proxy_, implicit-def dead $scc

We do not need to save SCC here, but we cannot know that SCC will be dead at the next instruction without scanning forward.
Typically we break from the loop at the first def or use of SCC.
In the worst case if

• we have the instruction which uses frame index at the beginning of the large BB

• and this instruction also uses SCC

• and no def/use of SCC appears before the end of the BB

Yes, in this case, we scan all the BB in the loop and the whole pass has O(n^2).

[alex-t]

In fact, to avoid this unwanted loop I can use another hack - just forward scavenger one position to look up the SCC liveness at the insertion slot and then immediately backward scavenger to restore its position and avoid breaking the PEI logic.
This looks weird but avoids the loop.

Also, this approach still allows some amount of false positives and, hence, some unnecessary SCC saves/restores. Given that we may already have high SGPR pressure, each false positive spends yet one more SGPR and increases the probability to run out of registers.

Why this approach cannot be improved?
We cannot make a reasonable decision regarding SCC save/restore without scanning down to the nearest use or definition. Register scavenger retrieves liveness information going forward by each instruction at a time. So, at each point, it only knows that the register was defined somewhere before and has not been yet killed by another definition. It may become dead at the next instruction or keep living an arbitrarily long distance.
To make a reasonable decision regarding SCC we need to know if SCC is "live" at the insertion point and if it is used at some point before the next definition. Otherwise, we have to conservatively save/restore SCC if it is "live" at the point where we're about to insert scalar instruction that clobbers it. In all cases where SCC is re-defined before the use, such save/restore is unnecessary but spends SGPR.

To illustrate the above:

def SCC
some others instructions
use of the FI <-- We are going to insert "S_ADD_I32 FrameReg, Offset" right before it
                             SCC is "live" here
        ************
here is an arbitrarily long sequence of instructions
that neither use nor define SCC
        ************
Here is an instruction "I"  that defines or uses SCC

if "I" defines SCC (kills the previous definition) - no SCC save/restore is necessary at the insertion point.
if "I" uses SCC - it is necessary to save it before S_ADD_I32 insertion and restore it after.

We have to scan down until either "I" is found or the basic block ends.

[rampitec]

Am I correct to assume that frame offset is always DWORD aligned? If so we have couple spare bits:

S_ADDC_U32 $reg, offset
S_BITCMP1_B32 $reg, 0
S_BITSET0_B32 $reg, 0

[rampitec]

For the loop which may potentially consume a lot of time I'd suggest to set a small scan threshold and assume scc is used otherwise. This relies on the possibility of restoring scc without scavenging, like in my previous comment.

[alex-t]

Applied approach suggested by @rampitec

[alex-t]

In D136169#3872475, @rampitec wrote:

Am I correct to assume that frame offset is always DWORD aligned? If so we have couple spare bits:

S_ADDC_U32 $reg, offset
S_BITCMP1_B32 $reg, 0
S_BITSET0_B32 $reg, 0

That's brilliant! Thanks a lot.

[rampitec]

In D136169#3875712, @alex-t wrote:

In D136169#3872475, @rampitec wrote:

Am I correct to assume that frame offset is always DWORD aligned? If so we have couple spare bits:

S_ADDC_U32 $reg, offset
S_BITCMP1_B32 $reg, 0
S_BITSET0_B32 $reg, 0

That's brilliant! Thanks a lot.

Enjoy :)

[rampitec]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2219

I think you need to skip debug instructions without counting so that debug build does not differ from release.

llvm/lib/Target/AMDGPU/SIRegisterInfo.h

96 ↗

(On Diff #469731)

100 seems to large number to me. Compiling with spills is already sufficiently and painfully slow. I would not go more than 10 here. In a worst case you are now burning just 2 instructions and probably 8 cycles.

Besides I do not think it deserves a special function, just a constant threshold right inside the code. Just because it is only used once.

[rampitec]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2213

Save is not a right word anymore. Preserve is.

[foad]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2258

-Offset here surely? Why aren't there any codegen tests for this?

2384–2385

Remove these changes from the patch.

[alex-t]

Corrected wrong Frame Register restore code.
Several minor changes as requested.

[alex-t]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2384–2385

This is obviously a mistake. "-Offset" cannot work here.
Given that we always have Reg and Offset aligned, hence even, we should just use S_SUBB_U32 here.

llvm/lib/Target/AMDGPU/SIRegisterInfo.h

96 ↗

(On Diff #469731)

100 seems to large number to me. Compiling with spills is already sufficiently and painfully slow. I would not go more than 10 here. In a worst case you are now burning just 2 instructions and probably 8 cycles.

Besides I do not think it deserves a special function, just a constant threshold right inside the code. Just because it is only used once.

I would not agree. Really there may be tens of the odd SCC preserving code sequences in 100 lines length BB if it has frequent stack access. So, this is not just 2 instructions but 2*N. I can show you the example of assembly if necessary.

[alex-t]

Temporary workaround to avoid SCC liveness scanning loop and using manually set
"dead" flags to decrease the amount of unnecessary SCC preserving code.

[sebastian-ne]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2247

Maybe it makes sense to add an assert, that the least significant bit of Offset is indeed 0? (here and for s_subb below)

[foad]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2258

The bitcmp1/bitset0 trick only works with addc. It doesn't work with subb, because if scc is set then the result will be 1 *less* than you wanted, so bitset0 will not correct it.

[alex-t]

Since the https://reviews.llvm.org/D137574 has been landed, this review is updated to use backward PEI.

[alex-t]

added test for using and restoring frame register for offset calculation.
S_SUBB_U32 changed to S_ADDC_U32 with "-Offset"

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2247

As far as I know, the flat scratch offset is always dword-aligned. If it is not we've got the UB anyway. So, the assert does not help.

2258

Oops. Good point.
BTW S_ADDC_U32 with "-Offset" may cause overflow itself. So, we override SCC again.
Although offset has uint64_t, I guess that we always have a flat scratch offset value small enough to never get into this trouble. Just because we subtract the same value as we were adding in the previous step.
And yes, we've got no problem with this so far as we had no tests for that case. I am going to add one.

[foad]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

1637

Do you still need all these setIsDead changes? In any case can you remove them from *this* patch, so it just contains the essential changes? Maybe put them all in a separate patch, if there is still any need for them.

[alex-t]

removed unrelated changes

[alex-t]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

1637

No. I don't need them anymore. Just my inaccuracy. Removed.

[foad]

There are still setIsDead changes and whitespace changes. Please try to strip them all out so we get a minimal patch to review.

[sebastian-ne]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2247

Sure, an assert doesn’t help correctness. But if at some point in the future, we encounter a case where the offset is not aligned – maybe due to a bug elsewhere – having an assert makes it easy to find why some memory got corrupted.
Without an assert, it will surely take hours to find out what is going on.

[alex-t]

In D136169#3943669, @foad wrote:

There are still setIsDead changes and whitespace changes. Please try to strip them all out so we get a minimal patch to review.

Ok. I thought you wanted me to remove all the setIsDead calls since they are not needed anymore.

[alex-t]

removed changes not relevant directly to the current revision
added assert to check if the flat scratch offset is aligned

[foad]

LGTM, thanks!

In D136169#3944366, @alex-t wrote:

In D136169#3943669, @foad wrote:

There are still setIsDead changes and whitespace changes. Please try to strip them all out so we get a minimal patch to review.

Ok. I thought you wanted me to remove all the setIsDead calls since they are not needed anymore.

That would probably be fine, but it should be a separate patch.

[alex-t]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2247

The assert which is currently added only ensures that the offset is even.
This is enough to make sure we have the correct arithmetics here. Although to ensure correct alignment it should be

!(Offset & flat_scratch_min_align)

I am not sure if it makes sense but maybe there should be another error message?

[arsenm]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2231–2232

I think we just need to start reserving an SGPR ahead of time in case we run into these scenarios, and un-reserve it after allocation if unused

[arsenm]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2246

Needs a comment explaining this trick

[alex-t]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2231–2232

I guess it is for a separate change already.

2246

Okay, too late but could I just push yet another commit just adding the comment?

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

Register FIReg = MRI.createVirtualRegister(
       ST.enableFlatScratch() ? &AMDGPU::SReg_32_XM0RegClass
                              : &AMDGPU::VGPR_32RegClass);
 
   BuildMI(*MBB, Ins, DL, TII->get(AMDGPU::S_MOV_B32), OffsetReg)
     .addImm(Offset);
   BuildMI(*MBB, Ins, DL, TII->get(MovOpc), FIReg)
     .addFrameIndex(FrameIdx);
 
-  if (ST.enableFlatScratch() ) {
+  if (ST.enableFlatScratch()) {
     BuildMI(*MBB, Ins, DL, TII->get(AMDGPU::S_ADD_I32), BaseReg)
         .addReg(OffsetReg, RegState::Kill)
         .addReg(FIReg);
     return BaseReg;
   }
 
   TII->getAddNoCarry(*MBB, Ins, DL, BaseReg)
     .addReg(OffsetReg, RegState::Kill)
     .addReg(FIReg)
     .addImm(0); // clamp bit
 
   return BaseReg;
void SIRegisterInfo::buildSpillLoadStore(
 
   if (ScratchOffsetRegDelta != 0) {
     // Subtract the offset we added to the ScratchOffset register.
     BuildMI(MBB, MI, DL, TII->get(AMDGPU::S_ADD_I32), SOffset)
         .addReg(SOffset)
         .addImm(-ScratchOffsetRegDelta);
   }
 }
 
 void SIRegisterInfo::buildVGPRSpillLoadStore(SGPRSpillBuilder &SB, int Index,
                                              int Offset, bool IsLoad,
                                              bool IsKill) const {
   // Load/store VGPR
   MachineFrameInfo &FrameInfo = SB.MF.getFrameInfo();
   assert(FrameInfo.getStackID(Index) != TargetStackID::SGPRSpill);
 
   Register FrameReg =
default: {
           if (FrameReg)
             MIB.addReg(FrameReg);
           else
             MIB.addImm(Offset);
 
           return;
         }
 
+        bool NeedSaveSCC = false;
+        MachineBasicBlock::iterator I(MI);
+        if (RS->isRegUsed(AMDGPU::SCC)) {
+          while (I != MBB->end()) {
+            if (*I != MI && I->readsRegister(AMDGPU::SCC)) {
+              NeedSaveSCC = true;
+              break;
+            }
+            if (*I != MI && I->definesRegister(AMDGPU::SCC))
+              break;
+            I++;
+          }
+        }
+
         Register TmpSReg =
             UseSGPR ? TmpReg
                     : RS->scavengeRegister(&AMDGPU::SReg_32_XM0RegClass, MI, 0,
                                            !UseSGPR);
 
         // TODO: for flat scratch another attempt can be made with a VGPR index
         //       if no SGPRs can be scavenged.
         if ((!TmpSReg && !FrameReg) || (!TmpReg && !UseSGPR))
           report_fatal_error("Cannot scavenge register in FI elimination!");
 
         if (!TmpSReg) {
           // Use frame register and restore it after.
           TmpSReg = FrameReg;
           FIOp.setReg(FrameReg);
           FIOp.setIsKill(false);
         }
 
-        BuildMI(*MBB, MI, DL, TII->get(AMDGPU::S_ADD_I32), TmpSReg)
+        I = BuildMI(*MBB, MI, DL, TII->get(AMDGPU::S_ADD_I32), TmpSReg)
                 .addReg(FrameReg)
                 .addImm(Offset);
+        MachineBasicBlock::iterator P = I;
 
         if (!UseSGPR)
-          BuildMI(*MBB, MI, DL, TII->get(AMDGPU::V_MOV_B32_e32), TmpReg)
+          I = BuildMI(*MBB, MI, DL, TII->get(AMDGPU::V_MOV_B32_e32), TmpReg)
                   .addReg(TmpSReg, RegState::Kill);
 
         if (TmpSReg == FrameReg) {
           // Undo frame register modification.
-          BuildMI(*MBB, std::next(MI), DL, TII->get(AMDGPU::S_ADD_I32),
+          I = BuildMI(*MBB, std::next(MI), DL, TII->get(AMDGPU::S_ADD_I32),
                       FrameReg)
                   .addReg(FrameReg)
                   .addImm(-Offset);
         }
 
+        if (NeedSaveSCC) {
+          Register SCCCopy =
+              RS->scavengeRegister(getBoolRC(), std::prev(P), 0, !UseSGPR);
+          if (!SCCCopy)
+            report_fatal_error("Cannot scavenge register in FI elimination!");
+          BuildMI(*MBB, P, DL,
+                  TII->get(ST.isWave32() ? AMDGPU::S_CSELECT_B32
+                                         : AMDGPU::S_CSELECT_B64),
+                  SCCCopy)
+              .addImm(-1)
+              .addImm(0);
+          unsigned Opcode =
+              ST.isWave32() ? AMDGPU::S_AND_B32 : AMDGPU::S_AND_B64;
+          Register Exec = ST.isWave32() ? AMDGPU::EXEC_LO : AMDGPU::EXEC;
+          BuildMI(*MBB, std::next(I), DL, TII->get(Opcode))
+              .addReg(SCCCopy, getDefRegState(true))
+              .addReg(SCCCopy)
+              .addReg(Exec);
+        }
         return;
       }
 
       bool IsMUBUF = TII->isMUBUF(*MI);
 
       if (!IsMUBUF && !MFI->isEntryFunction()) {
         // Convert to a swizzled stack address by scaling by the wave size.
         // In an entry function/kernel the offset is already swizzled.
default: {
             Register ScaledReg = TmpScaledReg.isValid() ? TmpScaledReg : FrameReg;
 
             BuildMI(*MBB, MI, DL, TII->get(AMDGPU::S_LSHR_B32), ScaledReg)
               .addReg(FrameReg)
               .addImm(ST.getWavefrontSizeLog2());
             BuildMI(*MBB, MI, DL, TII->get(AMDGPU::S_ADD_I32), ScaledReg)
                 .addReg(ScaledReg, RegState::Kill)
                 .addImm(Offset);
             if (!IsSALU)
               BuildMI(*MBB, MI, DL, TII->get(AMDGPU::COPY), ResultReg)
                   .addReg(ScaledReg, RegState::Kill);
             else
               ResultReg = ScaledReg;
 
             // If there were truly no free SGPRs, we need to undo everything.
             if (!TmpScaledReg.isValid()) {
               BuildMI(*MBB, MI, DL, TII->get(AMDGPU::S_ADD_I32), ScaledReg)
                   .addReg(ScaledReg, RegState::Kill)
                   .addImm(-Offset);
               BuildMI(*MBB, MI, DL, TII->get(AMDGPU::S_LSHL_B32), ScaledReg)
                   .addReg(FrameReg)
                   .addImm(ST.getWavefrontSizeLog2());
             }
           }
         }
 
         // Don't introduce an extra copy if we're just materializing in a mov.
         if (IsCopy)
           MI->eraseFromParent();
         else

llvm/test/CodeGen/AMDGPU/vgpr-spill-scc-clobber.mir

body:             |
   ; MUBUF-NEXT: {{  $}}
   ; MUBUF-NEXT: bb.2:
   ; MUBUF-NEXT:   S_ENDPGM 0, amdgpu_allvgprs
   ; GFX9-FLATSCR-LABEL: name: mubuf_load_restore_clobber_scc
   ; GFX9-FLATSCR: bb.0:
   ; GFX9-FLATSCR-NEXT:   successors: %bb.2(0x40000000), %bb.1(0x40000000)
   ; GFX9-FLATSCR-NEXT: {{  $}}
   ; GFX9-FLATSCR-NEXT:   S_CMP_EQ_U32 0, 0, implicit-def $scc
+  ; GFX9-FLATSCR-NEXT:   $sgpr4_sgpr5 = S_CSELECT_B64 -1, 0, implicit $scc
   ; GFX9-FLATSCR-NEXT:   $vcc_hi = S_ADD_I32 $sgpr32, 8200, implicit-def $scc
   ; GFX9-FLATSCR-NEXT:   $vgpr1 = V_MOV_B32_e32 killed $vcc_hi, implicit $exec
+  ; GFX9-FLATSCR-NEXT:   $sgpr4_sgpr5 = S_AND_B64 $sgpr4_sgpr5, $exec, implicit-def $scc
   ; GFX9-FLATSCR-NEXT:   $vgpr0 = BUFFER_LOAD_DWORD_OFFEN killed $vgpr1, $sgpr0_sgpr1_sgpr2_sgpr3, 0, 0, 0, 0, 0, implicit $exec :: (load (s32) from %stack.1, addrspace 5)
   ; GFX9-FLATSCR-NEXT:   S_CBRANCH_SCC1 %bb.2, implicit $scc
   ; GFX9-FLATSCR-NEXT: {{  $}}
   ; GFX9-FLATSCR-NEXT: bb.1:
   ; GFX9-FLATSCR-NEXT:   successors: %bb.2(0x80000000)
   ; GFX9-FLATSCR-NEXT: {{  $}}
   ; GFX9-FLATSCR-NEXT:   S_NOP 0
   ; GFX9-FLATSCR-NEXT: {{  $}}
   ; GFX9-FLATSCR-NEXT: bb.2:
   ; GFX9-FLATSCR-NEXT:   S_ENDPGM 0, amdgpu_allvgprs
   ; GFX10-FLATSCR-LABEL: name: mubuf_load_restore_clobber_scc
   ; GFX10-FLATSCR: bb.0:
   ; GFX10-FLATSCR-NEXT:   successors: %bb.2(0x40000000), %bb.1(0x40000000)
   ; GFX10-FLATSCR-NEXT: {{  $}}
   ; GFX10-FLATSCR-NEXT:   S_CMP_EQ_U32 0, 0, implicit-def $scc
+  ; GFX10-FLATSCR-NEXT:   $sgpr4 = S_CSELECT_B32 -1, 0, implicit $scc
   ; GFX10-FLATSCR-NEXT:   $vcc_lo = S_ADD_I32 $sgpr32, 8200, implicit-def $scc
   ; GFX10-FLATSCR-NEXT:   $vgpr1 = V_MOV_B32_e32 killed $vcc_lo, implicit $exec
+  ; GFX10-FLATSCR-NEXT:   $sgpr4 = S_AND_B32 $sgpr4, $exec_lo, implicit-def $scc
   ; GFX10-FLATSCR-NEXT:   $vgpr0 = BUFFER_LOAD_DWORD_OFFEN killed $vgpr1, $sgpr0_sgpr1_sgpr2_sgpr3, 0, 0, 0, 0, 0, implicit $exec :: (load (s32) from %stack.1, addrspace 5)
   ; GFX10-FLATSCR-NEXT:   S_CBRANCH_SCC1 %bb.2, implicit $scc
   ; GFX10-FLATSCR-NEXT: {{  $}}
   ; GFX10-FLATSCR-NEXT: bb.1:
   ; GFX10-FLATSCR-NEXT:   successors: %bb.2(0x80000000)
   ; GFX10-FLATSCR-NEXT: {{  $}}
   ; GFX10-FLATSCR-NEXT:   S_NOP 0
   ; GFX10-FLATSCR-NEXT: {{  $}}
body:             |
   ; MUBUF-NEXT:   S_ENDPGM 0, amdgpu_allvgprs
   ; GFX9-FLATSCR-LABEL: name: mubuf_load_restore_clobber_scc_no_vgprs_emergency_stack_slot
   ; GFX9-FLATSCR: bb.0:
   ; GFX9-FLATSCR-NEXT:   successors: %bb.2(0x40000000), %bb.1(0x40000000)
   ; GFX9-FLATSCR-NEXT:   liveins: $vgpr0_vgpr1_vgpr2_vgpr3_vgpr4_vgpr5_vgpr6_vgpr7_vgpr8_vgpr9_vgpr10_vgpr11_vgpr12_vgpr13_vgpr14_vgpr15, $vgpr16_vgpr17_vgpr18_vgpr19_vgpr20_vgpr21_vgpr22_vgpr23_vgpr24_vgpr25_vgpr26_vgpr27_vgpr28_vgpr29_vgpr30_vgpr31, $vgpr32_vgpr33_vgpr34_vgpr35_vgpr36_vgpr37_vgpr38_vgpr39_vgpr40_vgpr41_vgpr42_vgpr43_vgpr44_vgpr45_vgpr46_vgpr47, $vgpr48_vgpr49_vgpr50_vgpr51_vgpr52_vgpr53_vgpr54_vgpr55_vgpr56_vgpr57_vgpr58_vgpr59_vgpr60_vgpr61_vgpr62_vgpr63, $vgpr64_vgpr65_vgpr66_vgpr67_vgpr68_vgpr69_vgpr70_vgpr71_vgpr72_vgpr73_vgpr74_vgpr75_vgpr76_vgpr77_vgpr78_vgpr79, $vgpr80_vgpr81_vgpr82_vgpr83_vgpr84_vgpr85_vgpr86_vgpr87_vgpr88_vgpr89_vgpr90_vgpr91_vgpr92_vgpr93_vgpr94_vgpr95, $vgpr96_vgpr97_vgpr98_vgpr99_vgpr100_vgpr101_vgpr102_vgpr103_vgpr104_vgpr105_vgpr106_vgpr107_vgpr108_vgpr109_vgpr110_vgpr111, $vgpr112_vgpr113_vgpr114_vgpr115_vgpr116_vgpr117_vgpr118_vgpr119_vgpr120_vgpr121_vgpr122_vgpr123_vgpr124_vgpr125_vgpr126_vgpr127, $vgpr128_vgpr129_vgpr130_vgpr131_vgpr132_vgpr133_vgpr134_vgpr135_vgpr136_vgpr137_vgpr138_vgpr139_vgpr140_vgpr141_vgpr142_vgpr143, $vgpr144_vgpr145_vgpr146_vgpr147_vgpr148_vgpr149_vgpr150_vgpr151_vgpr152_vgpr153_vgpr154_vgpr155_vgpr156_vgpr157_vgpr158_vgpr159, $vgpr160_vgpr161_vgpr162_vgpr163_vgpr164_vgpr165_vgpr166_vgpr167_vgpr168_vgpr169_vgpr170_vgpr171_vgpr172_vgpr173_vgpr174_vgpr175, $vgpr176_vgpr177_vgpr178_vgpr179_vgpr180_vgpr181_vgpr182_vgpr183_vgpr184_vgpr185_vgpr186_vgpr187_vgpr188_vgpr189_vgpr190_vgpr191, $vgpr192_vgpr193_vgpr194_vgpr195_vgpr196_vgpr197_vgpr198_vgpr199_vgpr200_vgpr201_vgpr202_vgpr203_vgpr204_vgpr205_vgpr206_vgpr207, $vgpr208_vgpr209_vgpr210_vgpr211_vgpr212_vgpr213_vgpr214_vgpr215_vgpr216_vgpr217_vgpr218_vgpr219_vgpr220_vgpr221_vgpr222_vgpr223, $vgpr224_vgpr225_vgpr226_vgpr227_vgpr228_vgpr229_vgpr230_vgpr231_vgpr232_vgpr233_vgpr234_vgpr235_vgpr236_vgpr237_vgpr238_vgpr239, $vgpr240_vgpr241_vgpr242_vgpr243_vgpr244_vgpr245_vgpr246_vgpr247, $vgpr248_vgpr249_vgpr250_vgpr251, $vgpr252_vgpr253_vgpr254_vgpr255
   ; GFX9-FLATSCR-NEXT: {{  $}}
   ; GFX9-FLATSCR-NEXT:   S_CMP_EQ_U32 0, 0, implicit-def $scc
   ; GFX9-FLATSCR-NEXT:   SCRATCH_STORE_DWORD_SADDR killed $vgpr1, $sgpr32, 0, 0, implicit $exec, implicit $flat_scr :: (store (s32) into %stack.2, addrspace 5)
+  ; GFX9-FLATSCR-NEXT:   $sgpr4_sgpr5 = S_CSELECT_B64 -1, 0, implicit $scc
   ; GFX9-FLATSCR-NEXT:   $vcc_hi = S_ADD_I32 $sgpr32, 8200, implicit-def $scc
   ; GFX9-FLATSCR-NEXT:   $vgpr1 = V_MOV_B32_e32 killed $vcc_hi, implicit $exec
+  ; GFX9-FLATSCR-NEXT:   $sgpr4_sgpr5 = S_AND_B64 $sgpr4_sgpr5, $exec, implicit-def $scc
   ; GFX9-FLATSCR-NEXT:   $vgpr0 = BUFFER_LOAD_DWORD_OFFEN killed $vgpr1, $sgpr0_sgpr1_sgpr2_sgpr3, 0, 0, 0, 0, 0, implicit $exec :: (load (s32) from %stack.1, addrspace 5)
   ; GFX9-FLATSCR-NEXT:   $vgpr1 = SCRATCH_LOAD_DWORD_SADDR $sgpr32, 0, 0, implicit $exec, implicit $flat_scr :: (load (s32) from %stack.2, addrspace 5)
   ; GFX9-FLATSCR-NEXT:   S_CBRANCH_SCC1 %bb.2, implicit $scc
   ; GFX9-FLATSCR-NEXT: {{  $}}
   ; GFX9-FLATSCR-NEXT: bb.1:
   ; GFX9-FLATSCR-NEXT:   successors: %bb.2(0x80000000)
   ; GFX9-FLATSCR-NEXT: {{  $}}
   ; GFX9-FLATSCR-NEXT:   S_NOP 0
   ; GFX9-FLATSCR-NEXT: {{  $}}
   ; GFX9-FLATSCR-NEXT: bb.2:
   ; GFX9-FLATSCR-NEXT:   S_ENDPGM 0, amdgpu_allvgprs
   ; GFX10-FLATSCR-LABEL: name: mubuf_load_restore_clobber_scc_no_vgprs_emergency_stack_slot
   ; GFX10-FLATSCR: bb.0:
   ; GFX10-FLATSCR-NEXT:   successors: %bb.2(0x40000000), %bb.1(0x40000000)
   ; GFX10-FLATSCR-NEXT:   liveins: $vgpr0_vgpr1_vgpr2_vgpr3_vgpr4_vgpr5_vgpr6_vgpr7_vgpr8_vgpr9_vgpr10_vgpr11_vgpr12_vgpr13_vgpr14_vgpr15, $vgpr16_vgpr17_vgpr18_vgpr19_vgpr20_vgpr21_vgpr22_vgpr23_vgpr24_vgpr25_vgpr26_vgpr27_vgpr28_vgpr29_vgpr30_vgpr31, $vgpr32_vgpr33_vgpr34_vgpr35_vgpr36_vgpr37_vgpr38_vgpr39_vgpr40_vgpr41_vgpr42_vgpr43_vgpr44_vgpr45_vgpr46_vgpr47, $vgpr48_vgpr49_vgpr50_vgpr51_vgpr52_vgpr53_vgpr54_vgpr55_vgpr56_vgpr57_vgpr58_vgpr59_vgpr60_vgpr61_vgpr62_vgpr63, $vgpr64_vgpr65_vgpr66_vgpr67_vgpr68_vgpr69_vgpr70_vgpr71_vgpr72_vgpr73_vgpr74_vgpr75_vgpr76_vgpr77_vgpr78_vgpr79, $vgpr80_vgpr81_vgpr82_vgpr83_vgpr84_vgpr85_vgpr86_vgpr87_vgpr88_vgpr89_vgpr90_vgpr91_vgpr92_vgpr93_vgpr94_vgpr95, $vgpr96_vgpr97_vgpr98_vgpr99_vgpr100_vgpr101_vgpr102_vgpr103_vgpr104_vgpr105_vgpr106_vgpr107_vgpr108_vgpr109_vgpr110_vgpr111, $vgpr112_vgpr113_vgpr114_vgpr115_vgpr116_vgpr117_vgpr118_vgpr119_vgpr120_vgpr121_vgpr122_vgpr123_vgpr124_vgpr125_vgpr126_vgpr127, $vgpr128_vgpr129_vgpr130_vgpr131_vgpr132_vgpr133_vgpr134_vgpr135_vgpr136_vgpr137_vgpr138_vgpr139_vgpr140_vgpr141_vgpr142_vgpr143, $vgpr144_vgpr145_vgpr146_vgpr147_vgpr148_vgpr149_vgpr150_vgpr151_vgpr152_vgpr153_vgpr154_vgpr155_vgpr156_vgpr157_vgpr158_vgpr159, $vgpr160_vgpr161_vgpr162_vgpr163_vgpr164_vgpr165_vgpr166_vgpr167_vgpr168_vgpr169_vgpr170_vgpr171_vgpr172_vgpr173_vgpr174_vgpr175, $vgpr176_vgpr177_vgpr178_vgpr179_vgpr180_vgpr181_vgpr182_vgpr183_vgpr184_vgpr185_vgpr186_vgpr187_vgpr188_vgpr189_vgpr190_vgpr191, $vgpr192_vgpr193_vgpr194_vgpr195_vgpr196_vgpr197_vgpr198_vgpr199_vgpr200_vgpr201_vgpr202_vgpr203_vgpr204_vgpr205_vgpr206_vgpr207, $vgpr208_vgpr209_vgpr210_vgpr211_vgpr212_vgpr213_vgpr214_vgpr215_vgpr216_vgpr217_vgpr218_vgpr219_vgpr220_vgpr221_vgpr222_vgpr223, $vgpr224_vgpr225_vgpr226_vgpr227_vgpr228_vgpr229_vgpr230_vgpr231_vgpr232_vgpr233_vgpr234_vgpr235_vgpr236_vgpr237_vgpr238_vgpr239, $vgpr240_vgpr241_vgpr242_vgpr243_vgpr244_vgpr245_vgpr246_vgpr247, $vgpr248_vgpr249_vgpr250_vgpr251, $vgpr252_vgpr253_vgpr254_vgpr255
   ; GFX10-FLATSCR-NEXT: {{  $}}
   ; GFX10-FLATSCR-NEXT:   S_CMP_EQ_U32 0, 0, implicit-def $scc
   ; GFX10-FLATSCR-NEXT:   SCRATCH_STORE_DWORD_SADDR killed $vgpr1, $sgpr32, 0, 0, implicit $exec, implicit $flat_scr :: (store (s32) into %stack.2, addrspace 5)
+  ; GFX10-FLATSCR-NEXT:   $sgpr4 = S_CSELECT_B32 -1, 0, implicit $scc
   ; GFX10-FLATSCR-NEXT:   $vcc_lo = S_ADD_I32 $sgpr32, 8200, implicit-def $scc
   ; GFX10-FLATSCR-NEXT:   $vgpr1 = V_MOV_B32_e32 killed $vcc_lo, implicit $exec
+  ; GFX10-FLATSCR-NEXT:   $sgpr4 = S_AND_B32 $sgpr4, $exec_lo, implicit-def $scc
   ; GFX10-FLATSCR-NEXT:   $vgpr0 = BUFFER_LOAD_DWORD_OFFEN killed $vgpr1, $sgpr0_sgpr1_sgpr2_sgpr3, 0, 0, 0, 0, 0, implicit $exec :: (load (s32) from %stack.1, addrspace 5)
   ; GFX10-FLATSCR-NEXT:   $vgpr1 = SCRATCH_LOAD_DWORD_SADDR $sgpr32, 0, 0, implicit $exec, implicit $flat_scr :: (load (s32) from %stack.2, addrspace 5)
   ; GFX10-FLATSCR-NEXT:   S_CBRANCH_SCC1 %bb.2, implicit $scc
   ; GFX10-FLATSCR-NEXT: {{  $}}
   ; GFX10-FLATSCR-NEXT: bb.1:
   ; GFX10-FLATSCR-NEXT:   successors: %bb.2(0x80000000)
   ; GFX10-FLATSCR-NEXT: {{  $}}
   ; GFX10-FLATSCR-NEXT:   S_NOP 0
body:             |
   ; MUBUF-NEXT:   liveins: $vgpr0
   ; MUBUF-NEXT: {{  $}}
   ; MUBUF-NEXT:   S_ENDPGM 0, implicit $vgpr0
   ; GFX9-FLATSCR-LABEL: name: v_mov_clobber_scc
   ; GFX9-FLATSCR: bb.0:
   ; GFX9-FLATSCR-NEXT:   successors: %bb.2(0x40000000), %bb.1(0x40000000)
   ; GFX9-FLATSCR-NEXT: {{  $}}
   ; GFX9-FLATSCR-NEXT:   S_CMP_EQ_U32 0, 0, implicit-def $scc
+  ; GFX9-FLATSCR-NEXT:   $sgpr4_sgpr5 = S_CSELECT_B64 -1, 0, implicit $scc
   ; GFX9-FLATSCR-NEXT:   $vcc_hi = S_ADD_I32 $sgpr32, 8200, implicit-def $scc
+  ; GFX9-FLATSCR-NEXT:   $sgpr4_sgpr5 = S_AND_B64 $sgpr4_sgpr5, $exec, implicit-def $scc
   ; GFX9-FLATSCR-NEXT:   $vgpr0 = V_MOV_B32_e32 killed $vcc_hi, implicit $exec
   ; GFX9-FLATSCR-NEXT:   S_CBRANCH_SCC1 %bb.2, implicit $scc
   ; GFX9-FLATSCR-NEXT: {{  $}}
   ; GFX9-FLATSCR-NEXT: bb.1:
   ; GFX9-FLATSCR-NEXT:   successors: %bb.2(0x80000000)
   ; GFX9-FLATSCR-NEXT: {{  $}}
   ; GFX9-FLATSCR-NEXT:   S_NOP 0
   ; GFX9-FLATSCR-NEXT: {{  $}}
   ; GFX9-FLATSCR-NEXT: bb.2:
   ; GFX9-FLATSCR-NEXT:   liveins: $vgpr0
   ; GFX9-FLATSCR-NEXT: {{  $}}
   ; GFX9-FLATSCR-NEXT:   S_ENDPGM 0, implicit $vgpr0
   ; GFX10-FLATSCR-LABEL: name: v_mov_clobber_scc
   ; GFX10-FLATSCR: bb.0:
   ; GFX10-FLATSCR-NEXT:   successors: %bb.2(0x40000000), %bb.1(0x40000000)
   ; GFX10-FLATSCR-NEXT: {{  $}}
   ; GFX10-FLATSCR-NEXT:   S_CMP_EQ_U32 0, 0, implicit-def $scc
+  ; GFX10-FLATSCR-NEXT:   $sgpr4 = S_CSELECT_B32 -1, 0, implicit $scc
   ; GFX10-FLATSCR-NEXT:   $vcc_lo = S_ADD_I32 $sgpr32, 8200, implicit-def $scc
+  ; GFX10-FLATSCR-NEXT:   $sgpr4 = S_AND_B32 $sgpr4, $exec_lo, implicit-def $scc
   ; GFX10-FLATSCR-NEXT:   $vgpr0 = V_MOV_B32_e32 killed $vcc_lo, implicit $exec
   ; GFX10-FLATSCR-NEXT:   S_CBRANCH_SCC1 %bb.2, implicit $scc
   ; GFX10-FLATSCR-NEXT: {{  $}}
   ; GFX10-FLATSCR-NEXT: bb.1:
   ; GFX10-FLATSCR-NEXT:   successors: %bb.2(0x80000000)
   ; GFX10-FLATSCR-NEXT: {{  $}}
   ; GFX10-FLATSCR-NEXT:   S_NOP 0
   ; GFX10-FLATSCR-NEXT: {{  $}}