[AMDGPU] Improve v_cmpx usage on GFX10.3.

[tsymalla]

Summary

On GFX10.3 targets, the following instruction sequence

v_cmp_* SGPR, ...
s_and_saveexec ..., SGPR

leads to a fairly long stall caused by a VALU write to a SGPR and having the
following SALU wait for the SGPR.

An equivalent sequence is to save the exec mask manually instead of letting
s_and_saveexec do the work and use a v_cmpx instruction instead to do the
comparison.

This patch modifies the SIOptimizeExecMasking pass as this is the last position
where s_and_saveexec instructions are inserted. It does the transformation by
trying to find the pattern, extracting the operands and generating the new
instruction sequence.

It also changes some existing lit tests and introduces a few new tests to show
the changed behavior on GFX10.3 targets.

[sebastian-ne]

llvm/test/CodeGen/AMDGPU/wqm.ll

384–386

I think the v_cmpx instruction should be inserted the the place of the s_and_saveexec, not at the place of the v_cmp. Otherwise this v_mov gets executed with the wrong exec mask.

[tsymalla]

llvm/test/CodeGen/AMDGPU/wqm.ll

384–386

Thanks, going to fix this.

[nhaehnle]

Thank you for doing ths. Please make sure to address that clang-format complaints. I have a bunch of other comments inline.

llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp

301–303

Just look at the operands explicitly by argument instead of iterating over them.

305

Can it be anything else? This should probably just be an assertion.

325

VCmpDest should be equal to Reg by definition. Again, this can be an assertion.

326

This doesn't work how you want it to work because after SSA elimination, if you need to iterate over defs then almost certainly you have multiple uses as well.

Do you have liveness information available and can you use it to get to the relevant defining instruction in a more targeted way?

Alternatively: this transform is really only helpful if the V_CMP and S_AND_SAVEEXEC are close together. If the V_CMP appears much earlier in the code (perhaps even in a different basic block!) then we shouldn't do the transform. So a better approach may be to do a bounded backwards walk through instructions. (This interacts with the point about checking for intermediate EXEC writes in a later comment.)

359–363

There shouldn't be any EXEC modifications between those two instructions. If there are, it's a sign that something really weird is going on and certainly this trick isn't sufficient to ensure correctness. You should probably move this check into the find function and have that function bail out if it finds an EXEC modification.

576–577

Spurious whitespace change.

641

gfx10.3 and later.

653–664

The S_AND_SAVEEXEC_Bnn instruction has to be one of the last instructions of the basic block. It actually gets derived from a terminator instruction, though for some reason we apparently never changed the S_xxx_SAVEEXEC instructions to be terminators as well.

In any case, what you should do here from a compile-time perspective is to iterate basic blocks backwards and limit the lop to a small number of iterations. Maybe 5?

Also, this loop should break if it finds a different instruction that writes EXEC (shouldn't happen in practice, but...).

llvm/lib/Target/AMDGPU/VOPCInstructions.td

222

Shouldn't the is_vcmp field here logically be 1?

259

Why the complicated expression for is_vcmp? Shouldn't that just be 0?

More to the point, VOPC_Pseudo already derives from VCMPVCMPXTable, so this is rather dodgy. I'd suggest changing the table to have an is_cmpx field and using a let here to override the field's default value from the VOPC_Pseudo.

[tsymalla]

llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp

359–363

This is possible. In wqm.ll, check the following sequence:

; GFX10-W32-NEXT:    v_cmp_nlt_f32_e32 vcc_lo, 0, v0
; GFX10-W32-NEXT:    s_and_b32 exec_lo, exec_lo, s12
; GFX10-W32-NEXT:    buffer_store_dword v4, v2, s[0:3], 0 idxen
; GFX10-W32-NEXT:    ; implicit-def: $vgpr0
; GFX10-W32-NEXT:    s_wqm_b32 exec_lo, exec_lo
; GFX10-W32-NEXT:    s_and_saveexec_b32 s13, vcc_lo

Here, the second instruction modifies the exec register.
However, I investigated that case already and found out that there's no way to give correctness here, so I decided to do as you proposed and moved the check into the find function. :-)

llvm/test/CodeGen/AMDGPU/wqm.ll

384–386

Apparently, there is another case to consider. Moving the v_cmpx to the place of the s_and_saveexec will use the wrong value for v0, so a copy needs to be inserted whenever a v_cmp register source operand gets redefined inbetween in the original sequence, and the cmpx needs to use the new operand.

[tsymalla]

llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp

326

Working with backwards checking now, which should work. Thanks.

llvm/lib/Target/AMDGPU/VOPCInstructions.td

222

I renamed the variables. What I want to achieve is here is to map v_cmpx instructions to their v_cmp pendants in the mapping.

259

Using just 0 would not work as there are other VOPC instructions which would then be inserted into the result table, which is not wanted.

The idea is the following:

Evaluate if subst(op, v_cmpx, v_cmp) != v_cmpx which should yield true if the substitution succeeds. This is, if the instruction starts with v_cmpx. This would be way easier to do if TableGen would support string operations like "beginsWith". However, the way you proposed should also work, so I'm going to change this!

[tsymalla]

llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp

653–664

Looking at the other changes, this transformation should only be applied when it doesn't break correctness or doesn't introduce overhead (spilling). In the next revision, the find instruction looks at the instructions of a basic block backwards from a given s_and_saveexec instruction and breaks whenever one of these cases appear. I am going to change that again so that it only iterates a maximum amount of instructions backwards when trying to find the compare instruction which should reduce the work to do.

[arsenm]

llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp

641

Why only do this on gfx10.3? Every target has v_cmpx?

[arsenm]

llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp

320

Look at the def operands directly instead? Also since this is post-SSA you need to defend against subregisters

[tsymalla]

llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp

641

We decided to do so because it is unclear if on < GFX10 this gives any performance advantage and on 10.1 / 10.2 an additional s_waitcnt_depctr needs to be inserted for correctness, so probably there won't be any performance gain from this on these targets.

@foad might elaborate more details for this.

[foad]

llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp

641

I don't have much to add. The performance of v_cmp followed by s_and_savexec is particularly bad on GFX10 because of instruction counter stalls. So yes we probably could do this pre GFX10, but I don't know if there's any reason to.

For the bugs in GFX10.1 and GFX10.2 see GCNHazardRecognizer::fixVcmpxExecWARHazard. Having to insert S_WAITCNT_DEPCTR probably negates any advantage you get from using v_cmpx in the first place.

[tsymalla]

Addresses the comments from the review.
Adds additional checks to prevent the transformation from
being applied, if the result would be less optimal than
the original code or simply incorrect.

[tsymalla]

It would probably make sense to only insert the pattern if Exec is not only not written but not implicitly used by a VALU instruction between the cmp and the saveexec - otherwise, the VALU will be executed with the wrong exec mask.

[sebastian-ne]

llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp

303

Instead of taking a SmallVector, you can take an ArrayRef<MCRegister>, then you can also pass {SaveExecSrc0->getReg()} without needing a vector.

Also, if you needed a vector here because it’s modified inside the function, it is considered better style to pass a SmallVectorImpl<MCRegister>, so that the caller can decide the number for inline allocation.

310–322

I think this could be clearer:

if ((Pred(&*A))
  return &*A;

if (DisallowDefBetween) {
  // …
}
++CurrentIteration;

llvm/test/CodeGen/AMDGPU/branch-relaxation-gfx10-branch-offset-bug.ll

1

This file doesn’t look autogenerated, but maybe it makes sense to generate it.

[sebastian-ne]

The pre-merge builds report some test failure in MC/AMDGPU and MC/Disassembler/AMDGPU. I think the assembler fix in VOPCInstructions.td and these test changes could be a separate patch.

llvm/test/CodeGen/AMDGPU/wqm.ll

2962–2965

This and the test above with the buffer_store_dword still look incorrect.

[tsymalla]

Addressing review changes, reducing the amount of cases where the
transformation takes places.

llvm/test/CodeGen/AMDGPU/branch-relaxation-gfx10-branch-offset-bug.ll

1

The changes here are minor, and I am not really sure what this test actually tests, so I am just going to remove the auto-generate line for now.

[sebastian-ne]

llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp

326–330

I’m not sure what I thought before, but a buffer_store obviously only changes memory and not its register arguments, so it’s completely fine if a buffer_store is between the v_cmp and the s_and_saveexec. Checking modifiesRegister should be enough.

356

There’s quite a lot of these isReg and !getSubReg checks in this patch. What are they guarding? I replaced them by asserts and there was no amdgpu codegen test that failed. (Leaving aside the isReg test for v_cmp arguments, which can also be constants.)

655

Not sure if this was already mentioned, but do you make sure that the v_cmp is not used anywhere outside the s_saveexec?

I guess it would still be beneficial to insert a v_cmpx if the v_cmp has more uses, but it shouldn’t be removed in that case.

[tsymalla]

llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp

326–330

You are correct, any memory load or store will just update the contents behind the address stored in the VGPR. Thanks for that. I did not think thoroughly about this change.

356

Regarding the v_cmp arguments, these are constants as you mentioned so these checks make sense.
However, I think we can safely assume that the check for s_and_saveexec will always pass as the arguments are registers, so I'm going to remove these.

655

No, I did not take this into account (it should mostly just write to VCC as this is checking sequences of s_and_saveexec and v_cmp).
I am going to add some more control flow for cases like this.

I don't really get your last comment though. Why would it beneficial to add the v_cmpx in addition to the v_cmp in cases like this?

[Joe_Nash]

llvm/lib/Target/AMDGPU/VOPCInstructions.td

268

I think this can be
"let IsVCMPX = 1;"
Maybe it happened when you inverted the table to have IsVCMPX instead of isVCMP.

[tsymalla]

llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp

655

Looking at this again.

The result of v_cmp will be VCC = Exec & ThreadsPassed, the result of v_cmpx will be Exec[Executing Thread] = bool for each executing thread. It could be that the result inside the VCC Is used as integer carry, so in these cases the transformation would be wrong. Another case which was already discussed was the early-exit option by using VCCZ, but I think it makes sense to ignore this for now.

It would require additional logic to make sure any following instructions using VCC (if there are any) are using the correct input value, which probably reduces the gain here - so I'm going to just don't do the transformation if any following instruction uses the destination operand of v_cmp.

[arsenm]

llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp

641

Should explain this in the comment

[tsymalla]

Adding liveness checks to prevent accidental pattern transformation.
Addressed review comments.

[sebastian-ne]

llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp

356

Did you remove any of these checks? I still see a lot of isReg and getSubReg checks

362–363

As far as I recall, the stall between v_cmp and s_and_saveexec is quite long, so

v_cmp
s_mov
v_cmpx

is probably faster than

v_cmp
s_and_saveexec

and it’s worth to do this transformation and keep the v_cmp around.
@foad?

[tsymalla]

llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp

356

Yes, I removed the check around the s_and_saveexec operand. For the v_cmp checks, either src0 or src1 could be a register and shouldn't be a subregister, so I think these checks still make sense.

[sebastian-ne]

llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp

356

Ah, I missed that. I’m still wondering what the getSubReg checks do?
There’s no change on the llvm tests with or without them. I guess a test that hits these code paths would be nice?

[foad]

llvm/lib/Target/AMDGPU/GCNHazardRecognizer.cpp

896 ↗

(On Diff #410477)

Commit this separately, no review required.

llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp

645

Don't need &* here.

llvm/lib/Target/AMDGPU/SIShrinkInstructions.cpp

730–731

Typo "write explicitly write"

730–731

Use "DstReg"?

734

"continue" doesn't seem right. We *should* be shrinking cmpx instructions from VOP3 encoding to VOPC encoding.

[tsymalla]

llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp

356

As far as I understood, after register allocation, no subregister indices should be used as we are dealing with physical registers. In Post-SSA there can still be virtual registers. These subregs can be used to get lane bitmasks which represent the parts that are covered by the subregister index if the register is still virtual. So, this should be a check for completeness, but it's probably not necessary here. I have seen some assertions in the codegen where they assume the register does not consist of subregs anymore post-ra (where SIOptimizeExecMaskingPass fits in), So probably it's nice to have, but would not affect tests either way.

[foad]

llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp

356

Physical register operands don't have subregs and MachineVerifier checks this, so please just remove the checks.

[tsymalla]

Fixed shrinking of VOPCX instructions.
Removed superfluous subreg checks.
Addressed review comments.

[tsymalla]

Newline fix.

[nhaehnle]

llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp

362–363

As far as I recall, the stall between v_cmp and s_and_saveexec is quite long, so

v_cmp
s_mov
v_cmpx

is probably faster than

v_cmp
s_and_saveexec

and it’s worth to do this transformation and keep the v_cmp around.
@foad?

IIRC the VALU->SALU stall is a simple conservative stall based on outstanding SGPR writes from the VALU. The v_cmp -> s_mov in the first snippet would already have the same stall as v_cmp -> s_and_saveexec, whether it uses the result of the v_cmp or not. So it's better to choose the code sequence with s_and_saveexec.

[tsymalla]

Can somebody please take a look again?

[foad]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

3869–3871

Testing MI.getOperand(0).isReg() is ugly here. It happens to work, because operand 0 of a nosdst cmpx e64 instruction is src0_modifiers which is an immediate. But really I think this code ought to say:

if MI has vdst
  copy MI.vdst to Inst32
[else] if MI has sdst
  assert that MI.sdst is vcc (so there is no need to copy it, because the Inst32 will implicitly use vcc)

[foad]

llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp

344–345

The code just finds the last v_cmp instruction. Then you hope that it defs the saveexec input operand? Do you ever actually check that?

374

I don't think you need this check (all vcmp instructions have src0 and src1 operands, don't they?).

377

You could have checked for Exec in the first call to findInstrBackwards, just to bail out a bit earlier.

[tsymalla]

llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp

344–345

You are correct. I included this check previously but somehow accidentally deleted it during review changes. Good find. Thank you.

[tsymalla]

Addressing review comments: adding missing v_cmp dest check.
Other minor improvements.

[tsymalla]

@Flakebi @foad What are your thoughts on this? It seems like the matching is constrained enough so it will only be applied in certain cases.

[sebastian-ne]

Looks good to me

[tsymalla]

Resolve merge conflicts.

[tsymalla]

Adding multiple checks to prevent generating incorrect code sequences.

[sebastian-ne]

Can you add lit tests for the fixes you made please?

llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp

325

Typo: an register, an basic block, an specific instruction

I was a bit confused by the comment, maybe calling the function isRegisterLiveAfter(MachineInstr &Stop, MCRegister Reg, …) is more clear? (The MBB can be fetched from Stop.getParent())

I think this function checks if Reg is still live/used after Stop?

333–335

I think a reverse iterator iterates from MBB.rbegin() to MBB.rend() (I’m always confused by this, but it’s used in SIOptimizeExecMaskingPreRA.cpp:412 in this way).

379

Does this also need the TRI?

[critson]

Need to fix missing TRI for modifiesRegister calls.
Also please add a test of the form:

$sgpr0 = S_BFE_U32 killed renamable $sgpr3, 524296, implicit-def dead $scc
$vcc = V_CMP_GT_U32_e64 killed $sgpr0, killed $vgpr0, implicit $exec
$sgpr0_sgpr1 = COPY $exec, implicit-def $exec
$sgpr0_sgpr1 = S_AND_B64 killed renamable $sgpr0_sgpr1, killed renamable $vcc, implicit-def dead $scc
$exec = S_MOV_B64_term killed renamable $sgpr0_sgpr

Where V_CMP source is subregister of saved EXEC mask, which can happen in wave64.

llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp

379

modifiesRegister must be passed TRI otherwise subregisters will not be checked.

384

Here too.

[tsymalla]

llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp

333–335

Good catch. rbegin() == end(), while rend() == begin().

[tsymalla]

Fixed subregister handling.
Added some more tests.

[tsymalla]

@critson @sebastian-ne Would you mind taking a look again? Adding the TRI argument should have fixed the remaining issues.

[sebastian-ne]

Looks good, thanks, just left some small comments.

llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp

325

I think the comment is not completely correct.
The function checks if Reg is in use after Stop. If Reg is dead after Stop but still defined and used afterwards, this function will still return false (because Reg is not in use directly after stop).

The fact that the function uses backwards iteration to find the set of live registers can be considered an implementation detail.

333–335

I think the first ++A is wrong, rbegin() should already be a valid iterator?

llvm/test/CodeGen/AMDGPU/vcmp-saveexec-to-vcmpx.ll

168

Nit: There’s a newline missing at the end of the file.

llvm/test/CodeGen/AMDGPU/vcmp-saveexec-to-vcmpx.mir

2

This test does not look autogenerated?
Can you add a short comment that explains what this test does? (The test and function name contain vcmpx, but the code does not use v_cmpx, which makes it non-obvious.)

[tsymalla]

Addressed review comments.

[tsymalla]

Addressed review comments.

[tsymalla]

llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp

333–335

I assume this is not exactly wrong, as we don't care about defines at the end of the basic block in this case, but it shouldn't matter either.

[tsymalla]

@critson Would you mind taking a look again please? Thanks

[critson]

LGTM, with a few minor nits.

Also please double check all comments in the code are up to date, as there has been a lot revisions.
Thanks!

llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp

298

Comment mentions code that no longer exists DisallowDefBetween?

651

Could this be more future proof?
e.g.
if (AMDGPU::isGFX10Plus(ST) && !ST.hasVcmpxExecWARHazard())

657

It's unfortunate that we have to scan all instructions to find these.
Normally they should be within an instruction or two of the terminators.
However, I can see that the waterfall code violates the principle of EXEC manipulation at the end of block right now, so we don't have a choice.

[tsymalla]

llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp

651

I am not sure if the conditions check would be correct, as I don't know the logic behind setting the value returned by hasVcmpxExecWARHazard(). We don't want to have this transformation enabled on either GFX10.1 or GFX10.2 - so for now I'm leaving it as it is - we can change that later perhaps.

[tsymalla]

Changes based on the review comments (only comments, so NFC).

llvm/lib/Target/AMDGPU/SIInstrInfo.h

namespace AMDGPU {
   /// of an SS (SADDR) form.
   LLVM_READONLY
   int getFlatScratchInstSVfromSS(uint16_t Opcode);
 
   /// \returns earlyclobber version of a MAC MFMA is exists.
   LLVM_READONLY
   int getMFMAEarlyClobberOp(uint16_t Opcode);
 
+  /// \returns v_cmpx version of a v_cmp instruction.
+  LLVM_READONLY
+  int getVCMPXOpFromVCMP(uint16_t Opcode);
+
   const uint64_t RSRC_DATA_FORMAT = 0xf00000000000LL;
   const uint64_t RSRC_ELEMENT_SIZE_SHIFT = (32 + 19);
   const uint64_t RSRC_INDEX_STRIDE_SHIFT = (32 + 21);
   const uint64_t RSRC_TID_ENABLE = UINT64_C(1) << (32 + 23);
 
 } // end namespace AMDGPU
 
 namespace SI {

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

if (Use.isUse() &&
       Use.setIsKill(Orig.isKill());
       return;
     }
   }
 }
 
 MachineInstr *SIInstrInfo::buildShrunkInst(MachineInstr &MI,
                                            unsigned Op32) const {
-  MachineBasicBlock *MBB = MI.getParent();;
+  MachineBasicBlock *MBB = MI.getParent();
   MachineInstrBuilder Inst32 =
     BuildMI(*MBB, MI, MI.getDebugLoc(), get(Op32))
     .setMIFlags(MI.getFlags());
 
   // Add the dst operand if the 32-bit encoding also has an explicit $vdst.
   // For VOPC instructions, this is replaced by an implicit def of vcc.
-  int Op32DstIdx = AMDGPU::getNamedOperandIdx(Op32, AMDGPU::OpName::vdst);
-  if (Op32DstIdx != -1) {
+  if (AMDGPU::getNamedOperandIdx(Op32, AMDGPU::OpName::vdst) != -1) {
     // dst
     Inst32.add(MI.getOperand(0));
-  } else {
+  } else if (AMDGPU::getNamedOperandIdx(Op32, AMDGPU::OpName::sdst) != -1) {
+    // VOPCX instructions won't be writing to an explicit dst, so this should
+    // not fail for these instructions.
     assert(((MI.getOperand(0).getReg() == AMDGPU::VCC) ||
             (MI.getOperand(0).getReg() == AMDGPU::VCC_LO)) &&
            "Unexpected case");
   }
 
   Inst32.add(*getNamedOperand(MI, AMDGPU::OpName::src0));
 
   const MachineOperand *Src1 = getNamedOperand(MI, AMDGPU::OpName::src1);

llvm/lib/Target/AMDGPU/SIInstrInfo.td

 def getMFMAEarlyClobberOp : InstrMapping {
   let FilterClass = "MFMATable";
   let RowFields = ["FMAOp"];
   let ColFields = ["IsMac"];
   let KeyCol = ["1"];
   let ValueCols = [["0"]];
 }
 
+// Maps an v_cmp instruction to its v_cmpx equivalent.
+def getVCMPXOpFromVCMP : InstrMapping {
+  let FilterClass = "VCMPVCMPXTable";
+  let RowFields = ["VCMPOp"];
+  let ColFields = ["IsVCMPX"];
+  let KeyCol = ["0"];
+  let ValueCols = [["1"]];
+}
+
 include "SIInstructions.td"
 
 include "DSInstructions.td"
 include "MIMGInstructions.td"

llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp

static bool isLiveOut(const MachineBasicBlock &MBB, unsigned Reg) {
   for (MachineBasicBlock *Succ : MBB.successors()) {
     if (Succ->isLiveIn(Reg))
       return true;
   }
 
   return false;
 }
 
+// Backwards-iterate from Origin (for n=MaxInstructions iterations) until either
+// the beginning of the BB is reached or Pred evaluates to true - which can be
+// an arbitrary condition based on the current MachineInstr, for instance an
+// target instruction. Breaks prematurely by returning nullptr if  one of the
+// registers given in NonModifiableRegs is modified by the current instruction.
+static MachineInstr *
+findInstrBackwards(MachineInstr &Origin,
+                   std::function<bool(MachineInstr *)> Pred,
+                   ArrayRef<MCRegister> NonModifiableRegs,
+                   const SIRegisterInfo *TRI, unsigned MaxInstructions = 5) {
+  MachineBasicBlock::reverse_iterator A = Origin.getReverseIterator(),
+                                      E = Origin.getParent()->rend();
+  unsigned CurrentIteration = 0;
+
+  for (++A; CurrentIteration < MaxInstructions && A != E; ++A) {
+    if (Pred(&*A))
+      return &*A;
+
+    for (MCRegister Reg : NonModifiableRegs) {
+      if (A->modifiesRegister(Reg, TRI))
+        return nullptr;
+    }
+
+    ++CurrentIteration;
+  }
+
+  return nullptr;
+}
+
+// Determine if a register Reg is not re-defined and still in use
+// in the range (Stop..BB.end].
+// It does so by backwards calculating liveness from the end of the BB until
+// either Stop or the beginning of the BB is reached.
+// After liveness is calculated, we can determine if Reg is still in use and not
+// defined inbetween the instructions.
+static bool isRegisterInUseAfter(MachineInstr &Stop, MCRegister Reg,
+                                 const SIRegisterInfo *TRI,
+                                 MachineRegisterInfo &MRI) {
+  LivePhysRegs LR(*TRI);
+  LR.addLiveOuts(*Stop.getParent());
+
+  for (auto A = Stop.getParent()->rbegin();
+       A != Stop.getParent()->rend() && A != Stop; ++A) {
+    LR.stepBackward(*A);
+  }
+
+  return !LR.available(MRI, Reg);
+}
+
+// Tries to find a possibility to optimize a v_cmp ..., s_and_saveexec sequence
+// by looking at an instance of a s_and_saveexec instruction. Returns a pointer
+// to the v_cmp instruction if it is safe to replace the sequence (see the
+// conditions in the function body). This is after register allocation, so some
+// checks on operand dependencies need to be considered.
+static MachineInstr *findPossibleVCMPVCMPXOptimization(
+    MachineInstr &SaveExec, MCRegister Exec, const SIRegisterInfo *TRI,
+    const SIInstrInfo *TII, MachineRegisterInfo &MRI) {
+
+  MachineInstr *VCmp = nullptr;
+
+  Register SaveExecDest = SaveExec.getOperand(0).getReg();
+  if (!TRI->isSGPRReg(MRI, SaveExecDest))
+    return nullptr;
+
+  MachineOperand *SaveExecSrc0 =
+      TII->getNamedOperand(SaveExec, AMDGPU::OpName::src0);
+  if (!SaveExecSrc0->isReg())
+    return nullptr;
+
+  // Try to find the last v_cmp instruction that defs the saveexec input
+  // operand without any write to Exec inbetween.
+  VCmp = findInstrBackwards(
+      SaveExec,
+      [&](MachineInstr *Check) {
+        return AMDGPU::getVCMPXOpFromVCMP(Check->getOpcode()) != -1 &&
+               Check->modifiesRegister(SaveExecSrc0->getReg(), TRI);
+      },
+      {Exec, SaveExecSrc0->getReg()}, TRI);
+
+  if (!VCmp)
+    return nullptr;
+  
+  MachineOperand *VCmpDest = TII->getNamedOperand(*VCmp, AMDGPU::OpName::sdst);
+  assert(VCmpDest && "Should have an sdst operand!");
+
+  // Check if any of the v_cmp source operands is written by the saveexec.
+  MachineOperand *Src0 = TII->getNamedOperand(*VCmp, AMDGPU::OpName::src0);
+  if (Src0->isReg() && TRI->isSGPRReg(MRI, Src0->getReg()) &&
+      SaveExec.modifiesRegister(Src0->getReg(), TRI))
+    return nullptr;
+
+  MachineOperand *Src1 = TII->getNamedOperand(*VCmp, AMDGPU::OpName::src1);
+  if (Src1->isReg() && TRI->isSGPRReg(MRI, Src1->getReg()) &&
+      SaveExec.modifiesRegister(Src1->getReg(), TRI))
+    return nullptr;
+
+  // Don't do the transformation if the destination operand is included in
+  // it's MBB Live-outs, meaning it's used in any of it's successors, leading
+  // to incorrect code if the v_cmp and therefore the def of
+  // the dest operand is removed.
+  if (isLiveOut(*VCmp->getParent(), VCmpDest->getReg()))
+    return nullptr;
+
+  // If the v_cmp target is in use after the s_and_saveexec, skip the
+  // optimization.
+  if (isRegisterInUseAfter(SaveExec, VCmpDest->getReg(), TRI,
+                          MRI))
+    return nullptr;
+
+  // Try to determine if there is a write to any of the VCmp
+  // operands between the saveexec and the vcmp.
+  // If yes, additional VGPR spilling might need to be inserted. In this case,
+  // it's not worth replacing the instruction sequence.
+  SmallVector<MCRegister, 2> NonDefRegs;
+  if (Src0->isReg())
+    NonDefRegs.push_back(Src0->getReg());
+
+  if (Src1->isReg())
+    NonDefRegs.push_back(Src1->getReg());
+
+  if (!findInstrBackwards(
+          SaveExec, [&](MachineInstr *Check) { return Check == VCmp; },
+          NonDefRegs, TRI))
+    return nullptr;
+
+  return VCmp;
+}
+
+// Inserts the optimized s_mov_b32 / v_cmpx sequence based on the
+// operands extracted from a v_cmp ..., s_and_saveexec pattern.
+static bool optimizeVCMPSaveExecSequence(MachineInstr &SaveExecInstr,
+                                         MachineInstr &VCmp, MCRegister Exec,
+                                         const SIInstrInfo *TII,
+                                         const SIRegisterInfo *TRI,
+                                         MachineRegisterInfo &MRI) {
+  const int NewOpcode = AMDGPU::getVCMPXOpFromVCMP(VCmp.getOpcode());
+
+  if (NewOpcode == -1)
+    return false;
+
+  MachineOperand *Src0 = TII->getNamedOperand(VCmp, AMDGPU::OpName::src0);
+  MachineOperand *Src1 = TII->getNamedOperand(VCmp, AMDGPU::OpName::src1);
+
+  Register MoveDest = SaveExecInstr.getOperand(0).getReg();
+
+  MachineBasicBlock::instr_iterator InsertPosIt = SaveExecInstr.getIterator();
+  if (!SaveExecInstr.uses().empty()) {
+    bool isSGPR32 = TRI->getRegSizeInBits(MoveDest, MRI) == 32;
+    unsigned MovOpcode = isSGPR32 ? AMDGPU::S_MOV_B32 : AMDGPU::S_MOV_B64;
+    BuildMI(*SaveExecInstr.getParent(), InsertPosIt,
+            SaveExecInstr.getDebugLoc(), TII->get(MovOpcode), MoveDest)
+        .addReg(Exec);
+  }
+
+  // Omit dst as V_CMPX is implicitly writing to EXEC.
+  // Add dummy src and clamp modifiers, if needed.
+  auto Builder = BuildMI(*VCmp.getParent(), std::next(InsertPosIt),
+                         VCmp.getDebugLoc(), TII->get(NewOpcode));
+
+  if (AMDGPU::getNamedOperandIdx(NewOpcode, AMDGPU::OpName::src0_modifiers) !=
+      -1)
+    Builder.addImm(0);
+
+  Builder.add(*Src0);
+
+  if (AMDGPU::getNamedOperandIdx(NewOpcode, AMDGPU::OpName::src1_modifiers) !=
+      -1)
+    Builder.addImm(0);
+
+  Builder.add(*Src1);
+
+  if (AMDGPU::getNamedOperandIdx(NewOpcode, AMDGPU::OpName::clamp) != -1)
+    Builder.addImm(0);
+
+  return true;
+}
+
 bool SIOptimizeExecMasking::runOnMachineFunction(MachineFunction &MF) {
   if (skipFunction(MF.getFunction()))
     return false;
 
   const GCNSubtarget &ST = MF.getSubtarget<GCNSubtarget>();
   const SIRegisterInfo *TRI = ST.getRegisterInfo();
   const SIInstrInfo *TII = ST.getInstrInfo();
+  MachineRegisterInfo *MRI = &MF.getRegInfo();
   MCRegister Exec = ST.isWave32() ? AMDGPU::EXEC_LO : AMDGPU::EXEC;
 
   // Optimize sequences emitted for control flow lowering. They are originally
   // emitted as the separate operations because spill code may need to be
   // inserted for the saved copy of exec.
   //
   //     x = copy exec
   //     z = s_<op>_b64 x, y
for (MachineBasicBlock::iterator J
         }
 
         unsigned SaveExecOp = getSaveExecOp(J->getOpcode());
         if (SaveExecOp == AMDGPU::INSTRUCTION_LIST_END)
           break;
 
         if (ReadsCopyFromExec) {
           SaveExecInst = &*J;
           LLVM_DEBUG(dbgs() << "Found save exec op: " << *SaveExecInst << '\n');
           continue;
         } else {
           LLVM_DEBUG(dbgs()
                      << "Instruction does not read exec copy: " << *J << '\n');
           break;
         }
       } else if (ReadsCopyFromExec && !SaveExecInst) {
         // Make sure no other instruction is trying to use this copy, before it
         // will be rewritten by the saveexec, i.e. hasOneUse. There may have
for (MachineBasicBlock &MBB : MF) {
     CopyToExecInst->eraseFromParent();
 
     for (MachineInstr *OtherInst : OtherUseInsts) {
       OtherInst->substituteRegister(CopyToExec, Exec,
                                     AMDGPU::NoSubRegister, *TRI);
     }
 
     Changed = true;
   }
 
+  // After all s_op_saveexec instructions are inserted,
+  // replace (on GFX10.3 and later)
+  // v_cmp_* SGPR, IMM, VGPR
+  // s_and_saveexec_b32 EXEC_SGPR_DEST, SGPR
+  // with
+  // s_mov_b32 EXEC_SGPR_DEST, exec_lo
+  // v_cmpx_* IMM, VGPR
+  // to reduce pipeline stalls.
+  if (ST.hasGFX10_3Insts()) {
+    DenseMap<MachineInstr *, MachineInstr *> SaveExecVCmpMapping;
+    const unsigned AndSaveExecOpcode =
+        ST.isWave32() ? AMDGPU::S_AND_SAVEEXEC_B32 : AMDGPU::S_AND_SAVEEXEC_B64;
+
+    for (MachineBasicBlock &MBB : MF) {
+      for (MachineInstr &MI : MBB) {
+        // Record relevant v_cmp / s_and_saveexec instruction pairs for
+        // replacement.
+        if (MI.getOpcode() != AndSaveExecOpcode)
+          continue;
+
+        if (MachineInstr *VCmp =
+                findPossibleVCMPVCMPXOptimization(MI, Exec, TRI, TII, *MRI))
+          SaveExecVCmpMapping[&MI] = VCmp;
+      }
+    }
+
+    for (const auto &Entry : SaveExecVCmpMapping) {
+      MachineInstr *SaveExecInstr = Entry.getFirst();
+      MachineInstr *VCmpInstr = Entry.getSecond();
+
+      if (optimizeVCMPSaveExecSequence(*SaveExecInstr, *VCmpInstr, Exec, TII,
+                                       TRI, *MRI)) {
+        SaveExecInstr->eraseFromParent();
+        VCmpInstr->eraseFromParent();
+
+        Changed = true;
+      }
+    }
+  }
+
   return Changed;
 }

llvm/lib/Target/AMDGPU/SIShrinkInstructions.cpp

for (I = MBB.begin(); I != MBB.end(); I = Next) {
         // it.
         if (!MI.isCommutable() || !TII->commuteInstruction(MI) ||
             !TII->canShrink(MI, MRI))
           continue;
       }
 
       int Op32 = AMDGPU::getVOPe32(MI.getOpcode());
 
       if (TII->isVOPC(Op32)) {
-        Register DstReg = MI.getOperand(0).getReg();
+        MachineOperand &Op0 = MI.getOperand(0);
+        if (Op0.isReg()) {
+          // Exclude VOPCX instructions as these don't explicitly write a
+          // dst.
+          Register DstReg = Op0.getReg();
           if (DstReg.isVirtual()) {
             // VOPC instructions can only write to the VCC register. We can't
             // force them to use VCC here, because this is only one register and
             // cannot deal with sequences which would require multiple copies of
             // VCC, e.g. S_AND_B64 (vcc = V_CMP_...), (vcc = V_CMP_...)
             //
-          // So, instead of forcing the instruction to write to VCC, we provide
-          // a hint to the register allocator to use VCC and then we will run
-          // this pass again after RA and shrink it if it outputs to VCC.
-          MRI.setRegAllocationHint(MI.getOperand(0).getReg(), 0, VCCReg);
+            // So, instead of forcing the instruction to write to VCC, we
+            // provide a hint to the register allocator to use VCC and then we
+            // will run this pass again after RA and shrink it if it outputs to
+            // VCC.
+            MRI.setRegAllocationHint(DstReg, 0, VCCReg);
             continue;
           }
           if (DstReg != VCCReg)
             continue;
         }
+      }
 
       if (Op32 == AMDGPU::V_CNDMASK_B32_e32) {
         // We shrink V_CNDMASK_B32_e64 using regalloc hints like we do for VOPC
         // instructions.
         const MachineOperand *Src2 =
             TII->getNamedOperand(MI, AMDGPU::OpName::src2);
         if (!Src2->isReg())
           continue;

llvm/lib/Target/AMDGPU/VOPCInstructions.td

list<dag> ret = !if(P.HasModifiers,
       [(set i1:$sdst, (setcc P.Src0VT:$src0, P.Src1VT:$src1, cond))]);
 }
 
 class VCMPXNoSDstTable <bit has_sdst, string Name> {
   bit HasSDst = has_sdst;
   string NoSDstOp = Name;
 }
 
+class VCMPVCMPXTable <string Name> {
+  bit IsVCMPX = 0;
+  string VCMPOp = Name;
+}
+
 multiclass VOPC_Pseudos <string opName,
                          VOPC_Profile P,
                          SDPatternOperator cond = COND_NULL,
                          string revOp = opName,
                          bit DefExec = 0> {
 
   def _e32 : VOPC_Pseudo <opName, P>,
              Commutable_REV<revOp#"_e32", !eq(revOp, opName)>,
-             VCMPXNoSDstTable<1, opName#"_e32"> {
+             VCMPXNoSDstTable<1, opName#"_e32">,
+             VCMPVCMPXTable<opName#"_e32"> {
     let Defs = !if(DefExec, [VCC, EXEC], [VCC]);
     let SchedRW = P.Schedule;
     let isConvergent = DefExec;
     let isCompare = 1;
     let isCommutable = 1;
   }
 
   def _e64 : VOP3_Pseudo<opName, P, getVOPCPat64<cond, P>.ret>,
     Commutable_REV<revOp#"_e64", !eq(revOp, opName)>,
-    VCMPXNoSDstTable<1, opName#"_e64"> {
+    VCMPXNoSDstTable<1, opName#"_e64">,
+    VCMPVCMPXTable<opName#"_e64"> {
     let Defs = !if(DefExec, [EXEC], []);
     let SchedRW = P.Schedule;
     let isCompare = 1;
     let isCommutable = 1;
   }
 
   foreach _ = BoolToList<P.HasExtSDWA>.ret in
   def _sdwa : VOPC_SDWA_Pseudo <opName, P> {
     let Defs = !if(DefExec, [EXEC], []);
     let SchedRW = P.Schedule;
     let isConvergent = DefExec;
     let isCompare = 1;
   }
 }
 
 let SubtargetPredicate = HasSdstCMPX in {
 multiclass VOPCX_Pseudos <string opName,
                           VOPC_Profile P, VOPC_Profile P_NoSDst,
                           SDPatternOperator cond = COND_NULL,
                           string revOp = opName> :
            VOPC_Pseudos <opName, P, cond, revOp, 1> {
 
   def _nosdst_e32 : VOPC_Pseudo <opName#"_nosdst", P_NoSDst, [], 0>,
              Commutable_REV<revOp#"_nosdst_e32", !eq(revOp, opName)>,
-             VCMPXNoSDstTable<0, opName#"_e32"> {
+             VCMPXNoSDstTable<0, opName#"_e32">,
+             VCMPVCMPXTable<!subst("v_cmpx", "v_cmp", opName#"_e32")> {
     let Defs = [EXEC];
     let SchedRW = P_NoSDst.Schedule;
     let isConvergent = 1;
     let isCompare = 1;
     let isCommutable = 1;
     let SubtargetPredicate = HasNoSdstCMPX;
+    let IsVCMPX = 1;
   }
 
   def _nosdst_e64 : VOP3_Pseudo<opName#"_nosdst", P_NoSDst>,
     Commutable_REV<revOp#"_nosdst_e64", !eq(revOp, opName)>,
-    VCMPXNoSDstTable<0, opName#"_e64"> {
+    VCMPXNoSDstTable<0, opName#"_e64">,
+    VCMPVCMPXTable<!subst("v_cmpx", "v_cmp", opName#"_e64")> {
     let Defs = [EXEC];
     let SchedRW = P_NoSDst.Schedule;
     let isCompare = 1;
     let isCommutable = 1;
     let SubtargetPredicate = HasNoSdstCMPX;
+    let IsVCMPX = 1;
   }
 
   foreach _ = BoolToList<P_NoSDst.HasExtSDWA>.ret in
   def _nosdst_sdwa : VOPC_SDWA_Pseudo <opName#"_nosdst", P_NoSDst> {
     let Defs = [EXEC];
     let SchedRW = P_NoSDst.Schedule;
     let isConvergent = 1;
     let isCompare = 1;

llvm/test/CodeGen/AMDGPU/branch-relaxation-gfx10-branch-offset-bug.ll

 ; RUN: llc -march=amdgcn -mcpu=gfx1030 -verify-machineinstrs -amdgpu-s-branch-bits=7 < %s | FileCheck -enable-var-scope -check-prefixes=GCN,GFX1030 %s
 ; RUN: llc -march=amdgcn -mcpu=gfx1010 -verify-machineinstrs -amdgpu-s-branch-bits=7 < %s | FileCheck -enable-var-scope -check-prefixes=GCN,GFX1010 %s
 
 ; For gfx1010, overestimate the branch size in case we need to insert
 ; a nop for the buggy offset.
 
 ; GCN-LABEL: long_forward_scc_branch_3f_offset_bug:
 ; GFX1030: s_cmp_lg_u32
-; GFX1030-NEXT: s_cbranch_scc1  [[ENDBB:.LBB[0-9]+_[0-9]+]]
+; GFX1030: s_cbranch_scc1  [[ENDBB:.LBB[0-9]+_[0-9]+]]
 
 ; GFX1010: s_cmp_lg_u32
 ; GFX1010-NEXT: s_cbranch_scc0  [[RELAX_BB:.LBB[0-9]+_[0-9]+]]
 ; GFX1010: s_getpc_b64
 ; GFX1010-NEXT: [[POST_GETPC:.Lpost_getpc[0-9]+]]:{{$}}
 ; GFX1010-NEXT: s_add_u32 s{{[0-9]+}}, s{{[0-9]+}}, ([[ENDBB:.LBB[0-9]+_[0-9]+]]-[[POST_GETPC]])&4294967295
 ; GFX1010-NEXT: s_addc_u32 s{{[0-9]+}}, s{{[0-9]+}}, ([[ENDBB:.LBB[0-9]+_[0-9]+]]-[[POST_GETPC]])>>32
 ; GFX1010: [[RELAX_BB]]:
bb2:
   br i1 %cmp1, label %bb2, label %bb3   ; +4 (gfx1030), +8 with workaround (gfx1010)
 
 bb3:
   store volatile i32 %cnd0, i32 addrspace(1)* %arg
   ret void
 }
 
 ; GCN-LABEL: {{^}}long_forward_exec_branch_3f_offset_bug:
-; GFX1030: v_cmp_eq_u32
-; GFX1030: s_and_saveexec_b32
-; GFX1030-NEXT: s_cbranch_execnz [[RELAX_BB:.LBB[0-9]+_[0-9]+]]
+; GFX1030: s_mov_b32
+; GFX1030: v_cmpx_eq_u32
+; GFX1030: s_cbranch_execnz [[RELAX_BB:.LBB[0-9]+_[0-9]+]]
 
 ; GFX1010: v_cmp_eq_u32
 ; GFX1010: s_and_saveexec_b32
 ; GFX1010-NEXT: s_cbranch_execnz  [[RELAX_BB:.LBB[0-9]+_[0-9]+]]
 
 ; GCN: s_getpc_b64
 ; GCN-NEXT: [[POST_GETPC:.Lpost_getpc[0-9]+]]:{{$}}
 ; GCN-NEXT: s_add_u32 s{{[0-9]+}}, s{{[0-9]+}}, ([[ENDBB:.LBB[0-9]+_[0-9]+]]-[[POST_GETPC]])&4294967295

llvm/test/CodeGen/AMDGPU/vcmp-saveexec-to-vcmpx.ll

+; RUN: llc -march=amdgcn -mcpu=gfx1010 -verify-machineinstrs < %s | FileCheck -check-prefixes=GCN,GFX1010 %s
+; RUN: llc -march=amdgcn -mcpu=gfx1030 -verify-machineinstrs < %s | FileCheck -check-prefixes=GCN,GFX1030 %s
+
+; GCN-LABEL: {{^}}test_insert_vcmpx_pattern_lt:
+; GFX1010: v_cmp_lt_i32_e32 vcc_lo, 15, v{{.*}}
+; GFX1010-NEXT: s_and_saveexec_b32 s{{.*}}, vcc_lo
+; GFX1030: s_mov_b32 s{{.*}}, exec_lo
+; GFX1030-NEXT: v_cmpx_lt_i32_e32 15, v{{.*}}
+define i32 @test_insert_vcmpx_pattern_lt(i32 %x) {
+entry:
+  %bc = icmp slt i32 %x, 16
+  br i1 %bc, label %endif, label %if
+
+if:
+  %ret = shl i32 %x, 2
+  ret i32 %ret
+
+endif:
+  ret i32 %x
+}
+
+; GCN-LABEL: {{^}}test_insert_vcmpx_pattern_gt:
+; GFX1010: v_cmp_gt_i32_e32 vcc_lo, 17, v{{.*}}
+; GFX1010-NEXT: s_and_saveexec_b32 s{{.*}}, vcc_lo
+; GFX1030: s_mov_b32 s{{.*}}, exec_lo
+; GFX1030-NEXT: v_cmpx_gt_i32_e32 17, v{{.*}}
+define i32 @test_insert_vcmpx_pattern_gt(i32 %x) {
+entry:
+  %bc = icmp sgt i32 %x, 16
+  br i1 %bc, label %endif, label %if
+
+if:
+  %ret = shl i32 %x, 2
+  ret i32 %ret
+
+endif:
+  ret i32 %x
+}
+
+; GCN-LABEL: {{^}}test_insert_vcmpx_pattern_eq:
+; GFX1010: v_cmp_ne_u32_e32 vcc_lo, 16, v{{.*}}
+; GFX1010-NEXT: s_and_saveexec_b32 s{{.*}}, vcc_lo
+; GFX1030: s_mov_b32 s{{.*}}, exec_lo
+; GFX1030-NEXT: v_cmpx_ne_u32_e32 16, v{{.*}}
+define i32 @test_insert_vcmpx_pattern_eq(i32 %x) {
+entry:
+  %bc = icmp eq i32 %x, 16
+  br i1 %bc, label %endif, label %if
+
+if:
+  %ret = shl i32 %x, 2
+  ret i32 %ret
+
+endif:
+  ret i32 %x
+}
+
+; GCN-LABEL: {{^}}test_insert_vcmpx_pattern_ne:
+; GFX1010: v_cmp_eq_u32_e32 vcc_lo, 16, v{{.*}}
+; GFX1010-NEXT: s_and_saveexec_b32 s{{.*}}, vcc_lo
+; GFX1030: s_mov_b32 s{{.*}}, exec_lo
+; GFX1030-NEXT: v_cmpx_eq_u32_e32 16, v{{.*}}
+define i32 @test_insert_vcmpx_pattern_ne(i32 %x) {
+entry:
+  %bc = icmp ne i32 %x, 16
+  br i1 %bc, label %endif, label %if
+
+if:
+  %ret = shl i32 %x, 2
+  ret i32 %ret
+
+endif:
+  ret i32 %x
+}
+
+; GCN-LABEL: {{^}}test_insert_vcmpx_pattern_le:
+; GFX1010: v_cmp_lt_i32_e32 vcc_lo, 16, v{{.*}}
+; GFX1010-NEXT: s_and_saveexec_b32 s{{.*}}, vcc_lo
+; GFX1030: s_mov_b32 s{{.*}}, exec_lo
+; GFX1030-NEXT: v_cmpx_lt_i32_e32 16, v{{.*}}
+define i32 @test_insert_vcmpx_pattern_le(i32 %x) {
+entry:
+  %bc = icmp sle i32 %x, 16
+  br i1 %bc, label %endif, label %if
+
+if:
+  %ret = shl i32 %x, 2
+  ret i32 %ret
+
+endif:
+  ret i32 %x
+}
+
+; GCN-LABEL: {{^}}test_insert_vcmpx_pattern_ge:
+; GFX1010: v_cmp_gt_i32_e32 vcc_lo, 16, v{{.*}}
+; GFX1010-NEXT: s_and_saveexec_b32 s{{.*}}, vcc_lo
+; GFX1030: s_mov_b32 s{{.*}}, exec_lo
+; GFX1030-NEXT: v_cmpx_gt_i32_e32 16, v{{.*}}
+define i32 @test_insert_vcmpx_pattern_ge(i32 %x) {
+entry:
+  %bc = icmp sge i32 %x, 16
+  br i1 %bc, label %endif, label %if
+
+if:
+  %ret = shl i32 %x, 2
+  ret i32 %ret
+
+endif:
+  ret i32 %x
+}
+
+declare amdgpu_gfx void @check_live_outs_helper(i64) #0
+
+; In cases where the output operand cannot be safely removed,
+; don't apply the v_cmpx transformation.
+
+; GCN-LABEL: {{^}}check_live_outs:
+; GFX1010: v_cmp_eq_u32_e64 s{{.*}}, v{{.*}}, v{{.*}}
+; GFX1010: s_and_saveexec_b32 s{{.*}}, s{{.*}}
+; GFX1030: v_cmp_eq_u32_e64 s{{.*}}, v{{.*}}, v{{.*}}
+; GFX1030: s_and_saveexec_b32 s{{.*}}, s{{.*}}
+define amdgpu_cs void @check_live_outs(i32 %a, i32 %b) {
+  %cond = icmp eq i32 %a, %b
+  %result = call i64 @llvm.amdgcn.icmp.i32(i32 %a, i32 %b, i32 32)
+  br i1 %cond, label %l1, label %l2
+l1:
+  call amdgpu_gfx void @check_live_outs_helper(i64 %result)
+  br label %l2
+l2:
+  ret void
+}
+
+; Omit the transformation if the s_and_saveexec instruction overwrites
+; any of the v_cmp source operands.
+
+; GCN-LABEL: check_saveexec_overwrites_vcmp_source:
+; GCN:  ; %bb.1: ; %then
+; GFX1010:          v_cmp_ge_i32_e32 vcc_lo, s[[A:[0-9]+]], v{{.*}}
+; GFX1010-NEXT:     v_mov_b32_e32 {{.*}}, s[[A]]
+; GFX1010-NEXT:     s_and_saveexec_b32 s[[A]], vcc_lo
+; GFX1030:          v_cmp_ge_i32_e32 vcc_lo, s[[A:[0-9]+]], v{{.*}}
+; GFX1030-NEXT:     v_mov_b32_e32 {{.*}}, s[[A]]
+; GFX1030-NEXT:     s_and_saveexec_b32 s[[A]], vcc_lo
+define i32 @check_saveexec_overwrites_vcmp_source(i32 inreg %a, i32 inreg %b) {
+entry:
+  %0 = icmp sge i32 %a, 0
+  br i1 %0, label %if, label %then
+
+if:
+  %1 = shl i32 %a, 2
+  %2 = or i32 %1, %b
+  ret i32 %2
+
+then:
+  %3 = call i64 @llvm.amdgcn.icmp.i32(i32 %a, i32 %b, i32 32)
+  %4 = trunc i64 %3 to i32
+  %5 = icmp slt i32 %4, %b
+  br i1 %5, label %after, label %end
+
+after:
+  ret i32 %4
+
+end:
+  ret i32 %a
+}
+
+declare i64 @llvm.amdgcn.icmp.i32(i32, i32, i32) #0

llvm/test/CodeGen/AMDGPU/vcmp-saveexec-to-vcmpx.mir

+# RUN: llc -march=amdgcn -mcpu=gfx1010 -mattr=-wavefrontsize32,+wavefrontsize64 %s -o - | FileCheck -check-prefix=GCN %s
+# RUN: llc -march=amdgcn -mcpu=gfx1030 -mattr=-wavefrontsize32,+wavefrontsize64 %s -o - | FileCheck -check-prefix=GCN %s
+
+---
+
+# After the Optimize exec masking (post-RA) pass, there's a change of having v_cmpx instructions
+# being introduced whenever there's a sequence of v_cmp and s_and_saveexec instructions
+# which can be safely replaced in various cases.
+# However, it is not safe to do so when the generated code sequence would omit part of the EXEC mask
+# which could occur when a subset of EXEC is used as input operand in the v_cmp instruction.
+# The idea behind this test is to check if the subregisters are correctly handled here.
+
+# GCN-LABEL: vcmp_saveexec_to_mov_vcmpx_exec_subreg:
+# GCN: v_cmp_gt_u32_e64 s[[[SDST_LO:[0-9]+]]:[[SDST_HI:[0-9]+]]], exec_lo, v{{.*}}
+# GCN: s_and_saveexec_b64 s[[[EXEC_LO:[0-9]+]]:[[EXEC_HI:[0-9]+]]], s[[[SDST_LO]]:[[SDST_HI]]]
+name: vcmp_saveexec_to_mov_vcmpx_exec_subreg
+tracksRegLiveness: true
+body: |
+  bb.0:
+    liveins: $vgpr0
+    renamable $sgpr0_sgpr1 = V_CMP_GT_U32_e64 $exec_lo, killed $vgpr0, implicit $exec
+    $sgpr2_sgpr3 = COPY $exec, implicit-def $exec
+    $sgpr2_sgpr3 = S_AND_B64 killed renamable $sgpr2_sgpr3, killed renamable $sgpr0_sgpr1, implicit-def dead $scc
+    $exec = S_MOV_B64_term killed renamable $sgpr2_sgpr3

llvm/test/CodeGen/AMDGPU/wqm.ll

 ; GFX9-W64-NEXT:  .LBB9_2: ; %endif
 ; GFX9-W64-NEXT:    s_or_b64 exec, exec, s[2:3]
 ; GFX9-W64-NEXT:    ; return to shader part epilog
 ;
 ; GFX10-W32-LABEL: test_wwm3:
 ; GFX10-W32:       ; %bb.0: ; %main_body
 ; GFX10-W32-NEXT:    v_mbcnt_lo_u32_b32 v0, -1, 0
 ; GFX10-W32-NEXT:    v_mbcnt_hi_u32_b32 v0, -1, v0
 ; GFX10-W32-NEXT:    v_cmp_gt_u32_e32 vcc_lo, 16, v0
 ; GFX10-W32-NEXT:    v_mov_b32_e32 v0, 0
 ; GFX10-W32-NEXT:    s_and_saveexec_b32 s1, vcc_lo
 ; GFX10-W32-NEXT:    s_cbranch_execz .LBB9_2
 ; GFX10-W32-NEXT:  ; %bb.1: ; %if
 ; GFX10-W32-NEXT:    s_or_saveexec_b32 s2, -1
 ; GFX10-W32-NEXT:    v_mov_b32_e32 v1, s0
 ; GFX10-W32-NEXT:    buffer_load_dword v1, v1, s[0:3], 0 idxen
 ; GFX10-W32-NEXT:    s_waitcnt vmcnt(0)
 ; GFX10-W32-NEXT:    v_add_f32_e32 v2, v1, v1
 ; GFX10-W32-NEXT:    s_mov_b32 exec_lo, s2

 ; GFX9-W64-NEXT:    s_waitcnt vmcnt(0)
 ; GFX9-W64-NEXT:    v_mov_b32_e32 v0, v2
 ; GFX9-W64-NEXT:    ; return to shader part epilog
 ;
 ; GFX10-W32-LABEL: test_control_flow_0:
 ; GFX10-W32:       ; %bb.0: ; %main_body
 ; GFX10-W32-NEXT:    s_mov_b32 s12, exec_lo
 ; GFX10-W32-NEXT:    s_wqm_b32 exec_lo, exec_lo
-; GFX10-W32-NEXT:    v_cmp_ne_u32_e32 vcc_lo, 0, v1
-; GFX10-W32-NEXT:    s_and_saveexec_b32 s13, vcc_lo
+; GFX10-W32-NEXT:    s_mov_b32 s13, exec_lo
+; GFX10-W32-NEXT:    v_cmpx_ne_u32_e32 0, v1
 ; GFX10-W32-NEXT:    s_xor_b32 s13, exec_lo, s13
 ; GFX10-W32-NEXT:    s_cbranch_execz .LBB23_2
 ; GFX10-W32-NEXT:  ; %bb.1: ; %ELSE
 ; GFX10-W32-NEXT:    s_and_saveexec_b32 s14, s12
 ; GFX10-W32-NEXT:    buffer_store_dword v2, v0, s[0:3], 0 idxen
 ; GFX10-W32-NEXT:    ; implicit-def: $vgpr0
 ; GFX10-W32-NEXT:    s_mov_b32 exec_lo, s14
 ; GFX10-W32-NEXT:  .LBB23_2: ; %Flow

 ; GFX9-W64-NEXT:    s_waitcnt vmcnt(0)
 ; GFX9-W64-NEXT:    v_mov_b32_e32 v0, v2
 ; GFX9-W64-NEXT:    ; return to shader part epilog
 ;
 ; GFX10-W32-LABEL: test_control_flow_1:
 ; GFX10-W32:       ; %bb.0: ; %main_body
 ; GFX10-W32-NEXT:    s_mov_b32 s12, exec_lo
 ; GFX10-W32-NEXT:    s_wqm_b32 exec_lo, exec_lo
-; GFX10-W32-NEXT:    v_cmp_ne_u32_e32 vcc_lo, 0, v1
-; GFX10-W32-NEXT:    s_and_saveexec_b32 s13, vcc_lo
+; GFX10-W32-NEXT:    s_mov_b32 s13, exec_lo
+; GFX10-W32-NEXT:    v_cmpx_ne_u32_e32 0, v1
 ; GFX10-W32-NEXT:    s_xor_b32 s13, exec_lo, s13
 ; GFX10-W32-NEXT:    s_cbranch_execz .LBB24_2
 ; GFX10-W32-NEXT:  ; %bb.1: ; %IF
 ; GFX10-W32-NEXT:    image_sample v0, v0, s[0:7], s[8:11] dmask:0x1 dim:SQ_RSRC_IMG_1D
 ; GFX10-W32-NEXT:    s_waitcnt vmcnt(0)
 ; GFX10-W32-NEXT:    image_sample v2, v0, s[0:7], s[8:11] dmask:0x1 dim:SQ_RSRC_IMG_1D
 ; GFX10-W32-NEXT:    ; implicit-def: $vgpr0
 ; GFX10-W32-NEXT:  .LBB24_2: ; %Flow

 ; GFX10-W32:       ; %bb.0: ; %main_body
 ; GFX10-W32-NEXT:    s_mov_b32 s12, exec_lo
 ; GFX10-W32-NEXT:    s_wqm_b32 exec_lo, exec_lo
 ; GFX10-W32-NEXT:    image_sample v1, v1, s[0:7], s[8:11] dmask:0x1 dim:SQ_RSRC_IMG_1D
 ; GFX10-W32-NEXT:    s_and_b32 exec_lo, exec_lo, s12
 ; GFX10-W32-NEXT:    s_waitcnt vmcnt(0)
 ; GFX10-W32-NEXT:    image_sample v1, v1, s[0:7], s[8:11] dmask:0x1 dim:SQ_RSRC_IMG_1D
 ; GFX10-W32-NEXT:    s_waitcnt vmcnt(0)
-; GFX10-W32-NEXT:    v_cmp_nlt_f32_e32 vcc_lo, 0, v1
 ; GFX10-W32-NEXT:    buffer_store_dword v1, v0, s[0:3], 0 idxen
+; GFX10-W32-NEXT:    s_mov_b32 s0, exec_lo
 ; GFX10-W32-NEXT:    ; implicit-def: $vgpr0
-; GFX10-W32-NEXT:    s_and_saveexec_b32 s0, vcc_lo
+; GFX10-W32-NEXT:    v_cmpx_nlt_f32_e32 0, v1
 ; GFX10-W32-NEXT:    s_xor_b32 s0, exec_lo, s0
 ; GFX10-W32-NEXT:  ; %bb.1: ; %ELSE
 ; GFX10-W32-NEXT:    v_mul_f32_e32 v0, 4.0, v1
 ; GFX10-W32-NEXT:    ; implicit-def: $vgpr1
 ; GFX10-W32-NEXT:  ; %bb.2: ; %Flow
 ; GFX10-W32-NEXT:    s_or_saveexec_b32 s0, s0
 ; GFX10-W32-NEXT:    s_xor_b32 exec_lo, exec_lo, s0
 ; GFX10-W32-NEXT:  ; %bb.3: ; %IF

 ; GFX9-W64-NEXT:    image_sample v[0:3], v0, s[0:7], s[8:11] dmask:0xf
 ; GFX9-W64-NEXT:    s_waitcnt vmcnt(0)
 ; GFX9-W64-NEXT:    ; return to shader part epilog
 ;
 ; GFX10-W32-LABEL: test_control_flow_4:
 ; GFX10-W32:       ; %bb.0: ; %main_body
 ; GFX10-W32-NEXT:    s_mov_b32 s12, exec_lo
 ; GFX10-W32-NEXT:    s_wqm_b32 exec_lo, exec_lo
-; GFX10-W32-NEXT:    v_cmp_eq_u32_e32 vcc_lo, 0, v1
-; GFX10-W32-NEXT:    s_and_saveexec_b32 s13, vcc_lo
+; GFX10-W32-NEXT:    s_mov_b32 s13, exec_lo
+; GFX10-W32-NEXT:    v_cmpx_eq_u32_e32 0, v1
 ; GFX10-W32-NEXT:    s_cbranch_execz .LBB27_2
 ; GFX10-W32-NEXT:  ; %bb.1: ; %IF
 ; GFX10-W32-NEXT:    s_and_saveexec_b32 s14, s12
 ; GFX10-W32-NEXT:    buffer_load_dword v1, off, s[0:3], 0
 ; GFX10-W32-NEXT:    v_mov_b32_e32 v2, 1
 ; GFX10-W32-NEXT:    s_waitcnt vmcnt(0)
 ; GFX10-W32-NEXT:    buffer_store_dword v1, v2, s[0:3], 0 idxen
 ; GFX10-W32-NEXT:    s_mov_b32 exec_lo, s14

 ;
 ; GFX10-W32-LABEL: test_strict_wqm_within_wqm:
 ; GFX10-W32:       ; %bb.0: ; %main_body
 ; GFX10-W32-NEXT:    s_mov_b32 s12, exec_lo
 ; GFX10-W32-NEXT:    s_wqm_b32 exec_lo, exec_lo
 ; GFX10-W32-NEXT:    s_mov_b32 s13, exec_lo
 ; GFX10-W32-NEXT:    s_wqm_b32 exec_lo, exec_lo
 ; GFX10-W32-NEXT:    v_mov_b32_e32 v2, v0
 ; GFX10-W32-NEXT:    s_mov_b32 exec_lo, s13
-; GFX10-W32-NEXT:    v_cmp_eq_u32_e32 vcc_lo, 0, v1
 ; GFX10-W32-NEXT:    v_mov_b32_e32 v0, 0
-; GFX10-W32-NEXT:    s_and_saveexec_b32 s13, vcc_lo
+; GFX10-W32-NEXT:    s_mov_b32 s13, exec_lo
+; GFX10-W32-NEXT:    v_cmpx_eq_u32_e32 0, v1
 ; GFX10-W32-NEXT:    s_cbranch_execz .LBB46_2
 ; GFX10-W32-NEXT:  ; %bb.1: ; %IF
 ; GFX10-W32-NEXT:    s_mov_b32 s14, exec_lo
 ; GFX10-W32-NEXT:    s_wqm_b32 exec_lo, exec_lo
 ; GFX10-W32-NEXT:    image_sample v2, v2, s[0:7], s[8:11] dmask:0x1 dim:SQ_RSRC_IMG_1D
 ; GFX10-W32-NEXT:    s_waitcnt vmcnt(0)
 ; GFX10-W32-NEXT:    image_sample v2, v2, s[0:7], s[8:11] dmask:0x1 dim:SQ_RSRC_IMG_1D
 ; GFX10-W32-NEXT:    s_waitcnt vmcnt(0)