[SystemZ] Merge SystemZExpandPseudo pass into SystemZPostRewrite.

[jonpa]

Summary

SystemZExpandPseudo:s only job was to expand LOCRMux instructions into jump sequences. This needs to be done if expandLOCRPseudo() or expandSELRPseudo() fails to find a legal opcode (all registers "high" or "low"). This task has now been moved to SystemZPostRewrite while removing the SystemZExpandPseudo pass.

It is in fact preferred to expand these pseudos directly after register allocation in SystemZPostRewrite since the hinted register combinations are then not subject to later optimizations. A few more handled cases on SPEC 2006 can be seen with this: "Number of LOCRMux jump-sequences": z14: 5 -> 0, arch13: 5 -> 4.

I thought it was a bit simpler and cleaner to keep the expandLOCRPseudo() and expandSELRPseudo() in SystemZInstrInfo. Is it ok to use "public:" and "private:" like this, or should they be moved instead?

If the number of missed cases is a problem (4 on arch13), a next step to try would be the allowHintRecoloring() method as used in D58923, but perhaps that is not quite motivated given the near 0 number of cases left.

[uweigand]

I thought it was a bit simpler and cleaner to keep the expandLOCRPseudo() and expandSELRPseudo() in SystemZInstrInfo. Is it ok to use "public:" and "private:" like this, or should they be moved instead?

I'd prefer to move everything then.

lib/Target/SystemZ/SystemZPostRewrite.cpp

51

What does this change have to do with the rest of the patch?

[jonpa]

I moved the functions and renamed them trySelectLOCRMux() and trySelectSELRMux() to avoid confusion (they do not "expand" the LOCRMux). Is there a point in passing the opcodes as arguments to them?

This in turn made some other methods of SystemZInstrInfo required to be "public" in order to use them from SytemZPostRewrite. I made isHighReg() a static method of SystemZRegisterInfo instead, though.

Removed this from expandPostRAPseudo(), since it seemed a bit redundant:

case SystemZ::LOCRMux:
 case SystemZ::SELRMux:
   llvm_unreachable("Should have been handled in SystemZPostRewrite!");
   return true;

[jonpa]

Wait - is it safe to have SystemZRegisterInfo::isHighReg() be a static function? I remember that you said that static functions were bad in multi-threaded programs. Is the "Reg" argument safe if two threads call this method simultaneously?

lib/Target/SystemZ/SystemZPostRewrite.cpp

51

I figured that since expandLOCRMux() may create new basic blocks, we can't call setPreservesAll(), unless we would be willing to update any analysis passes as needed. I haven't looked into that in detail, but I imagine things like the Dominator Tree and LoopInfo should be recomputed / updated as needed in case expandLOCRMux() is called.

[uweigand]

I agree with making isHighReg a static function in SystemZRegisterInfo. I don't see why there would be any issue with a static member function, as long as it has no persistent state. The only issue with multi-threaded programs would be persistant state, e.g. a static member variable or a static variable within the member function. Arguments and (nonstatic) local variables are fine.

I'm not sure we need to expose emitGRX32Move though. In SystemZPostRewrite, can't we just use a COPY to copy values, like we already do elsewhere in this file?

Also, now that selection to existing ISA instruction and expansion to an if-then-else block happen at the same place, I think we can therefore simplify the logic a bit. For example, make expandLOCRMux into a helper routine that creates the if-then-else expansion, and rename trySelectLOCRMux and trySelectSELRMux to selectLOCRMux and selectSELRMux, and have then call the helper if they cannot select to an existing ISA routine directly. This will also avoid first replacing a SELRMux with a LOCRMux just to have the latter being deleted immediately afterwards. (As an additional potential simplication, if the if-then-else expansion helper were to directly support three-address moves, then some of logic in selectSELRMux could be simplified even further.)

lib/Target/SystemZ/SystemZPostRewrite.cpp

51

Ah, this makes sense. Thanks!

[jonpa]

The only issue with multi-threaded programs would be persistant state...

OK, thanks for the explanation.

I'm not sure we need to expose emitGRX32Move though. In SystemZPostRewrite, can't we just use a COPY to copy values, like we already do elsewhere in this file?

Ah, sorry - of course... I also changed the call TII->copyPhysReg() to build a COPY (which was NFC on SPEC 2006).

(As an additional potential simplication, if the if-then-else expansion helper were to directly support three-address moves, then some of logic in selectSELRMux could be simplified even further.)

Since the copying into dest in selectSELRMux() serves both the purpose of increasing the number of selr/selfhr instructions as well as to prepare for expandCondMove(), it seems like the best place to have it to me. I first thought that the commutation should be moved to only be done if needed before calling expandCondMove(). However, I then one case appeared on SPEC 2006 where the commutation had made two SELRs look the same and therefore the CFG optimizer could transform the function to just have one SELR. So I am thinking that it probably is slightly preferred to "canonicalize" the SELRs and increase the chances of them becoming identical. This means I am not sure how this function could actually be simplified...

[uweigand]

In D67432#1669132, @jonpa wrote:

Since the copying into dest in selectSELRMux() serves both the purpose of increasing the number of selr/selfhr instructions as well as to prepare for expandCondMove(), it seems like the best place to have it to me. I first thought that the commutation should be moved to only be done if needed before calling expandCondMove(). However, I then one case appeared on SPEC 2006 where the commutation had made two SELRs look the same and therefore the CFG optimizer could transform the function to just have one SELR. So I am thinking that it probably is slightly preferred to "canonicalize" the SELRs and increase the chances of them becoming identical. This means I am not sure how this function could actually be simplified...

Ah, I see. This makes sense. The patch looks good to me then we with a just a couple of minor cleanups.

lib/Target/SystemZ/SystemZPostRewrite.cpp

82

Comment should be updated to specify that otherwise, a branch sequence is created.

103

Likewise. Also, MixedOpcode is no longer used and should be removed.

248

I guess this now just be "return true" and we no longer need the Modified variable.

lib/Target/SystemZ/SystemZRegisterInfo.cpp

76 ↗

(On Diff #220063)

Maybe this small enough so it would sense to move the definition to the header file, so it can be marked inline again.

[jonpa]

Updated per review.

lib/Target/SystemZ/SystemZRegisterInfo.cpp

76 ↗

(On Diff #220063)

I put it next to the other inline:d query methods in SystemZRegisterInfo.h. This makes the total name more manageable as well for the callers, right?

[uweigand]

This LGTM now, thanks!

[jonpa]

r371959

lib/Target/SystemZ/CMakeLists.txt

 
 add_public_tablegen_target(SystemZCommonTableGen)
 
 add_llvm_target(SystemZCodeGen
   SystemZAsmPrinter.cpp
   SystemZCallingConv.cpp
   SystemZConstantPoolValue.cpp
   SystemZElimCompare.cpp
-  SystemZExpandPseudo.cpp
   SystemZFrameLowering.cpp
   SystemZHazardRecognizer.cpp
   SystemZISelDAGToDAG.cpp
   SystemZISelLowering.cpp
   SystemZInstrInfo.cpp
   SystemZLDCleanup.cpp
   SystemZLongBranch.cpp
   SystemZMachineFunctionInfo.cpp

lib/Target/SystemZ/SystemZ.h

 static inline bool isImmHF(uint64_t Val) {
   return (Val & ~0xffffffff00000000ULL) == 0;
 }
 } // end namespace SystemZ
 
 FunctionPass *createSystemZISelDag(SystemZTargetMachine &TM,
                                    CodeGenOpt::Level OptLevel);
 FunctionPass *createSystemZElimComparePass(SystemZTargetMachine &TM);
-FunctionPass *createSystemZExpandPseudoPass(SystemZTargetMachine &TM);
 FunctionPass *createSystemZShortenInstPass(SystemZTargetMachine &TM);
 FunctionPass *createSystemZLongBranchPass(SystemZTargetMachine &TM);
 FunctionPass *createSystemZLDCleanupPass(SystemZTargetMachine &TM);
 FunctionPass *createSystemZPostRewritePass(SystemZTargetMachine &TM);
 FunctionPass *createSystemZTDCPass();
 } // end namespace llvm
 
 #endif

lib/Target/SystemZ/SystemZExpandPseudo.cpp

-//==-- SystemZExpandPseudo.cpp - Expand pseudo instructions -------*- C++ -*-=//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-//
-// This file contains a pass that expands pseudo instructions into target
-// instructions to allow proper scheduling and other late optimizations.  This
-// pass should be run after register allocation but before the post-regalloc
-// scheduling pass.
-//
-//===----------------------------------------------------------------------===//
-
-#include "SystemZ.h"
-#include "SystemZInstrInfo.h"
-#include "SystemZSubtarget.h"
-#include "llvm/CodeGen/LivePhysRegs.h"
-#include "llvm/CodeGen/MachineFunctionPass.h"
-#include "llvm/CodeGen/MachineInstrBuilder.h"
-using namespace llvm;
-
-#define SYSTEMZ_EXPAND_PSEUDO_NAME "SystemZ pseudo instruction expansion pass"
-
-namespace llvm {
-  void initializeSystemZExpandPseudoPass(PassRegistry&);
-}
-
-namespace {
-class SystemZExpandPseudo : public MachineFunctionPass {
-public:
-  static char ID;
-  SystemZExpandPseudo() : MachineFunctionPass(ID) {
-    initializeSystemZExpandPseudoPass(*PassRegistry::getPassRegistry());
-  }
-
-  const SystemZInstrInfo *TII;
-
-  bool runOnMachineFunction(MachineFunction &Fn) override;
-
-  StringRef getPassName() const override { return SYSTEMZ_EXPAND_PSEUDO_NAME; }
-
-private:
-  bool expandMBB(MachineBasicBlock &MBB);
-  bool expandMI(MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI,
-                MachineBasicBlock::iterator &NextMBBI);
-  bool expandLOCRMux(MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI,
-                     MachineBasicBlock::iterator &NextMBBI);
-};
-char SystemZExpandPseudo::ID = 0;
-}
-
-INITIALIZE_PASS(SystemZExpandPseudo, "systemz-expand-pseudo",
-                SYSTEMZ_EXPAND_PSEUDO_NAME, false, false)
-
-/// Returns an instance of the pseudo instruction expansion pass.
-FunctionPass *llvm::createSystemZExpandPseudoPass(SystemZTargetMachine &TM) {
-  return new SystemZExpandPseudo();
-}
-
-// MI is a load-register-on-condition pseudo instruction that could not be
-// handled as a single hardware instruction.  Replace it by a branch sequence.
-bool SystemZExpandPseudo::expandLOCRMux(MachineBasicBlock &MBB,
-                                        MachineBasicBlock::iterator MBBI,
-                                        MachineBasicBlock::iterator &NextMBBI) {
-  MachineFunction &MF = *MBB.getParent();
-  const BasicBlock *BB = MBB.getBasicBlock();
-  MachineInstr &MI = *MBBI;
-  DebugLoc DL = MI.getDebugLoc();
-  Register DestReg = MI.getOperand(0).getReg();
-  Register SrcReg = MI.getOperand(2).getReg();
-  unsigned CCValid = MI.getOperand(3).getImm();
-  unsigned CCMask = MI.getOperand(4).getImm();
-
-  LivePhysRegs LiveRegs(TII->getRegisterInfo());
-  LiveRegs.addLiveOuts(MBB);
-  for (auto I = std::prev(MBB.end()); I != MBBI; --I)
-    LiveRegs.stepBackward(*I);
-
-  // Splice MBB at MI, moving the rest of the block into RestMBB.
-  MachineBasicBlock *RestMBB = MF.CreateMachineBasicBlock(BB);
-  MF.insert(std::next(MachineFunction::iterator(MBB)), RestMBB);
-  RestMBB->splice(RestMBB->begin(), &MBB, MI, MBB.end());
-  RestMBB->transferSuccessors(&MBB);
-  for (auto I = LiveRegs.begin(); I != LiveRegs.end(); ++I)
-    RestMBB->addLiveIn(*I);
-
-  // Create a new block MoveMBB to hold the move instruction.
-  MachineBasicBlock *MoveMBB = MF.CreateMachineBasicBlock(BB);
-  MF.insert(std::next(MachineFunction::iterator(MBB)), MoveMBB);
-  MoveMBB->addLiveIn(SrcReg);
-  for (auto I = LiveRegs.begin(); I != LiveRegs.end(); ++I)
-    MoveMBB->addLiveIn(*I);
-
-  // At the end of MBB, create a conditional branch to RestMBB if the
-  // condition is false, otherwise fall through to MoveMBB.
-  BuildMI(&MBB, DL, TII->get(SystemZ::BRC))
-    .addImm(CCValid).addImm(CCMask ^ CCValid).addMBB(RestMBB);
-  MBB.addSuccessor(RestMBB);
-  MBB.addSuccessor(MoveMBB);
-
-  // In MoveMBB, emit an instruction to move SrcReg into DestReg,
-  // then fall through to RestMBB.
-  TII->copyPhysReg(*MoveMBB, MoveMBB->end(), DL, DestReg, SrcReg,
-                   MI.getOperand(2).isKill());
-  MoveMBB->addSuccessor(RestMBB);
-
-  NextMBBI = MBB.end();
-  MI.eraseFromParent();
-  return true;
-}
-
-/// If MBBI references a pseudo instruction that should be expanded here,
-/// do the expansion and return true.  Otherwise return false.
-bool SystemZExpandPseudo::expandMI(MachineBasicBlock &MBB,
-                                   MachineBasicBlock::iterator MBBI,
-                                   MachineBasicBlock::iterator &NextMBBI) {
-  MachineInstr &MI = *MBBI;
-  switch (MI.getOpcode()) {
-  case SystemZ::LOCRMux:
-    return expandLOCRMux(MBB, MBBI, NextMBBI);
-  default:
-    break;
-  }
-  return false;
-}
-
-/// Iterate over the instructions in basic block MBB and expand any
-/// pseudo instructions.  Return true if anything was modified.
-bool SystemZExpandPseudo::expandMBB(MachineBasicBlock &MBB) {
-  bool Modified = false;
-
-  MachineBasicBlock::iterator MBBI = MBB.begin(), E = MBB.end();
-  while (MBBI != E) {
-    MachineBasicBlock::iterator NMBBI = std::next(MBBI);
-    Modified |= expandMI(MBB, MBBI, NMBBI);
-    MBBI = NMBBI;
-  }
-
-  return Modified;
-}
-
-bool SystemZExpandPseudo::runOnMachineFunction(MachineFunction &MF) {
-  TII = static_cast<const SystemZInstrInfo *>(MF.getSubtarget().getInstrInfo());
-
-  bool Modified = false;
-  for (auto &MBB : MF)
-    Modified |= expandMBB(MBB);
-  return Modified;
-}
-

lib/Target/SystemZ/SystemZInstrInfo.h

class SystemZInstrInfo : public SystemZGenInstrInfo {
   void expandRIPseudo(MachineInstr &MI, unsigned LowOpcode, unsigned HighOpcode,
                       bool ConvertHigh) const;
   void expandRIEPseudo(MachineInstr &MI, unsigned LowOpcode,
                        unsigned LowOpcodeK, unsigned HighOpcode) const;
   void expandRXYPseudo(MachineInstr &MI, unsigned LowOpcode,
                        unsigned HighOpcode) const;
   void expandLOCPseudo(MachineInstr &MI, unsigned LowOpcode,
                        unsigned HighOpcode) const;
-  void expandLOCRPseudo(MachineInstr &MI, unsigned LowOpcode,
-                        unsigned HighOpcode) const;
-  void expandSELRPseudo(MachineInstr &MI, unsigned LowOpcode,
-                        unsigned HighOpcode, unsigned MixedOpcode) const;
   void expandZExtPseudo(MachineInstr &MI, unsigned LowOpcode,
                         unsigned Size) const;
   void expandLoadStackGuard(MachineInstr *MI) const;
 
   MachineInstrBuilder
   emitGRX32Move(MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI,
                 const DebugLoc &DL, unsigned DestReg, unsigned SrcReg,
                 unsigned LowLowOpcode, unsigned Size, bool KillSrc,

lib/Target/SystemZ/SystemZInstrInfo.cpp

 
 using namespace llvm;
 
 #define GET_INSTRINFO_CTOR_DTOR
 #define GET_INSTRMAP_INFO
 #include "SystemZGenInstrInfo.inc"
 
 #define DEBUG_TYPE "systemz-II"
-STATISTIC(LOCRMuxJumps, "Number of LOCRMux jump-sequences (lower is better)");
 
 // Return a mask with Count low bits set.
 static uint64_t allOnes(unsigned int Count) {
   return Count == 0 ? 0 : (uint64_t(1) << (Count - 1) << 1) - 1;
 }
 
-// Reg should be a 32-bit GPR.  Return true if it is a high register rather
-// than a low register.
-static bool isHighReg(unsigned int Reg) {
-  if (SystemZ::GRH32BitRegClass.contains(Reg))
-    return true;
-  assert(SystemZ::GR32BitRegClass.contains(Reg) && "Invalid GRX32");
-  return false;
-}
-
 // Pin the vtable to this file.
 void SystemZInstrInfo::anchor() {}
 
 SystemZInstrInfo::SystemZInstrInfo(SystemZSubtarget &sti)
   : SystemZGenInstrInfo(SystemZ::ADJCALLSTACKDOWN, SystemZ::ADJCALLSTACKUP),
     RI(), STI(sti) {
 }
 

 // is a high GR32.  ConvertHigh is true if LowOpcode takes a signed operand
 // and HighOpcode takes an unsigned 32-bit operand.  In those cases,
 // MI has the same kind of operand as LowOpcode, so needs to be converted
 // if HighOpcode is used.
 void SystemZInstrInfo::expandRIPseudo(MachineInstr &MI, unsigned LowOpcode,
                                       unsigned HighOpcode,
                                       bool ConvertHigh) const {
   Register Reg = MI.getOperand(0).getReg();
-  bool IsHigh = isHighReg(Reg);
+  bool IsHigh = SystemZ::isHighReg(Reg);
   MI.setDesc(get(IsHigh ? HighOpcode : LowOpcode));
   if (IsHigh && ConvertHigh)
     MI.getOperand(1).setImm(uint32_t(MI.getOperand(1).getImm()));
 }
 
 // MI is a three-operand RIE-style pseudo instruction.  Replace it with
 // LowOpcodeK if the registers are both low GR32s, otherwise use a move
 // followed by HighOpcode or LowOpcode, depending on whether the target
 // is a high or low GR32.
 void SystemZInstrInfo::expandRIEPseudo(MachineInstr &MI, unsigned LowOpcode,
                                        unsigned LowOpcodeK,
                                        unsigned HighOpcode) const {
   Register DestReg = MI.getOperand(0).getReg();
   Register SrcReg = MI.getOperand(1).getReg();
-  bool DestIsHigh = isHighReg(DestReg);
-  bool SrcIsHigh = isHighReg(SrcReg);
+  bool DestIsHigh = SystemZ::isHighReg(DestReg);
+  bool SrcIsHigh = SystemZ::isHighReg(SrcReg);
   if (!DestIsHigh && !SrcIsHigh)
     MI.setDesc(get(LowOpcodeK));
   else {
     if (DestReg != SrcReg) {
       emitGRX32Move(*MI.getParent(), MI, MI.getDebugLoc(), DestReg, SrcReg,
                     SystemZ::LR, 32, MI.getOperand(1).isKill(),
                     MI.getOperand(1).isUndef());
       MI.getOperand(1).setReg(DestReg);
     }
     MI.setDesc(get(DestIsHigh ? HighOpcode : LowOpcode));
     MI.tieOperands(0, 1);
   }
 }
 
 // MI is an RXY-style pseudo instruction.  Replace it with LowOpcode
 // if the first operand is a low GR32 and HighOpcode if the first operand
 // is a high GR32.
 void SystemZInstrInfo::expandRXYPseudo(MachineInstr &MI, unsigned LowOpcode,
                                        unsigned HighOpcode) const {
   Register Reg = MI.getOperand(0).getReg();
-  unsigned Opcode = getOpcodeForOffset(isHighReg(Reg) ? HighOpcode : LowOpcode,
-                                       MI.getOperand(2).getImm());
+  unsigned Opcode = getOpcodeForOffset(
+      SystemZ::isHighReg(Reg) ? HighOpcode : LowOpcode,
   MI.setDesc(get(Opcode));
 }
 
 // MI is a load-on-condition pseudo instruction with a single register
 // (source or destination) operand.  Replace it with LowOpcode if the
 // register is a low GR32 and HighOpcode if the register is a high GR32.
 void SystemZInstrInfo::expandLOCPseudo(MachineInstr &MI, unsigned LowOpcode,
                                        unsigned HighOpcode) const {
   Register Reg = MI.getOperand(0).getReg();
-  unsigned Opcode = isHighReg(Reg) ? HighOpcode : LowOpcode;
+  unsigned Opcode = SystemZ::isHighReg(Reg) ? HighOpcode : LowOpcode;
   MI.setDesc(get(Opcode));
 }
 
-// MI is a load-register-on-condition pseudo instruction.  Replace it with
-// LowOpcode if source and destination are both low GR32s and HighOpcode if
-// source and destination are both high GR32s.
-void SystemZInstrInfo::expandLOCRPseudo(MachineInstr &MI, unsigned LowOpcode,
-                                        unsigned HighOpcode) const {
-  Register DestReg = MI.getOperand(0).getReg();
-  Register SrcReg = MI.getOperand(2).getReg();
-  bool DestIsHigh = isHighReg(DestReg);
-  bool SrcIsHigh = isHighReg(SrcReg);
-
-  if (!DestIsHigh && !SrcIsHigh)
-    MI.setDesc(get(LowOpcode));
-  else if (DestIsHigh && SrcIsHigh)
-    MI.setDesc(get(HighOpcode));
-  else
-    LOCRMuxJumps++;
-
-  // If we were unable to implement the pseudo with a single instruction, we
-  // need to convert it back into a branch sequence.  This cannot be done here
-  // since the caller of expandPostRAPseudo does not handle changes to the CFG
-  // correctly.  This change is defered to the SystemZExpandPseudo pass.
-}
-
-// MI is a select pseudo instruction.  Replace it with LowOpcode if source
-// and destination are all low GR32s and HighOpcode if source and destination
-// are all high GR32s.  Otherwise, use the two-operand MixedOpcode.
-void SystemZInstrInfo::expandSELRPseudo(MachineInstr &MI, unsigned LowOpcode,
-                                        unsigned HighOpcode,
-                                        unsigned MixedOpcode) const {
-  Register DestReg = MI.getOperand(0).getReg();
-  Register Src1Reg = MI.getOperand(1).getReg();
-  Register Src2Reg = MI.getOperand(2).getReg();
-  bool DestIsHigh = isHighReg(DestReg);
-  bool Src1IsHigh = isHighReg(Src1Reg);
-  bool Src2IsHigh = isHighReg(Src2Reg);
-
-  // If sources and destination aren't all high or all low, we may be able to
-  // simplify the operation by moving one of the sources to the destination
-  // first.  But only if this doesn't clobber the other source.
-  if (DestReg != Src1Reg && DestReg != Src2Reg) {
-    if (DestIsHigh != Src1IsHigh) {
-      emitGRX32Move(*MI.getParent(), MI, MI.getDebugLoc(), DestReg, Src1Reg,
-                    SystemZ::LR, 32, MI.getOperand(1).isKill(),
-                    MI.getOperand(1).isUndef());
-      MI.getOperand(1).setReg(DestReg);
-      Src1Reg = DestReg;
-      Src1IsHigh = DestIsHigh;
-    } else if (DestIsHigh != Src2IsHigh) {
-      emitGRX32Move(*MI.getParent(), MI, MI.getDebugLoc(), DestReg, Src2Reg,
-                    SystemZ::LR, 32, MI.getOperand(2).isKill(),
-                    MI.getOperand(2).isUndef());
-      MI.getOperand(2).setReg(DestReg);
-      Src2Reg = DestReg;
-      Src2IsHigh = DestIsHigh;
-    }
-  }
-
-  // If the destination (now) matches one source, prefer this to be first.
-  if (DestReg != Src1Reg && DestReg == Src2Reg) {
-    commuteInstruction(MI, false, 1, 2);
-    std::swap(Src1Reg, Src2Reg);
-    std::swap(Src1IsHigh, Src2IsHigh);
-  }
-
-  if (!DestIsHigh && !Src1IsHigh && !Src2IsHigh)
-    MI.setDesc(get(LowOpcode));
-  else if (DestIsHigh && Src1IsHigh && Src2IsHigh)
-    MI.setDesc(get(HighOpcode));
-  else {
-    // Given the simplifcation above, we must already have a two-operand case.
-    assert (DestReg == Src1Reg);
-    MI.setDesc(get(MixedOpcode));
-    MI.tieOperands(0, 1);
-    LOCRMuxJumps++;
-  }
-
-  // If we were unable to implement the pseudo with a single instruction, we
-  // need to convert it back into a branch sequence.  This cannot be done here
-  // since the caller of expandPostRAPseudo does not handle changes to the CFG
-  // correctly.  This change is defered to the SystemZExpandPseudo pass.
-}
-
 // MI is an RR-style pseudo instruction that zero-extends the low Size bits
 // of one GRX32 into another.  Replace it with LowOpcode if both operands
 // are low registers, otherwise use RISB[LH]G.
 void SystemZInstrInfo::expandZExtPseudo(MachineInstr &MI, unsigned LowOpcode,
                                         unsigned Size) const {
   MachineInstrBuilder MIB =
     emitGRX32Move(*MI.getParent(), MI, MI.getDebugLoc(),
                MI.getOperand(0).getReg(), MI.getOperand(1).getReg(), LowOpcode,

 MachineInstrBuilder
 SystemZInstrInfo::emitGRX32Move(MachineBasicBlock &MBB,
                                 MachineBasicBlock::iterator MBBI,
                                 const DebugLoc &DL, unsigned DestReg,
                                 unsigned SrcReg, unsigned LowLowOpcode,
                                 unsigned Size, bool KillSrc,
                                 bool UndefSrc) const {
   unsigned Opcode;
-  bool DestIsHigh = isHighReg(DestReg);
-  bool SrcIsHigh = isHighReg(SrcReg);
+  bool DestIsHigh = SystemZ::isHighReg(DestReg);
+  bool SrcIsHigh = SystemZ::isHighReg(SrcReg);
   if (DestIsHigh && SrcIsHigh)
     Opcode = SystemZ::RISBHH;
   else if (DestIsHigh && !SrcIsHigh)
     Opcode = SystemZ::RISBHL;
   else if (!DestIsHigh && SrcIsHigh)
     Opcode = SystemZ::RISBLH;
   else {
     return BuildMI(MBB, MBBI, DL, get(LowLowOpcode), DestReg)
bool SystemZInstrInfo::expandPostRAPseudo(MachineInstr &MI) const {
   case SystemZ::LOCMux:
     expandLOCPseudo(MI, SystemZ::LOC, SystemZ::LOCFH);
     return true;
 
   case SystemZ::LOCHIMux:
     expandLOCPseudo(MI, SystemZ::LOCHI, SystemZ::LOCHHI);
     return true;
 
-  case SystemZ::LOCRMux:
-    expandLOCRPseudo(MI, SystemZ::LOCR, SystemZ::LOCFHR);
-    return true;
-
-  case SystemZ::SELRMux:
-    expandSELRPseudo(MI, SystemZ::SELR, SystemZ::SELFHR,
-                         SystemZ::LOCRMux);
-    return true;
-
   case SystemZ::STCMux:
     expandRXYPseudo(MI, SystemZ::STC, SystemZ::STCH);
     return true;
 
   case SystemZ::STHMux:
     expandRXYPseudo(MI, SystemZ::STH, SystemZ::STHH);
     return true;
 
case SystemZ::CMux:
     expandRXYPseudo(MI, SystemZ::C, SystemZ::CHF);
     return true;
 
   case SystemZ::CLMux:
     expandRXYPseudo(MI, SystemZ::CL, SystemZ::CLHF);
     return true;
 
   case SystemZ::RISBMux: {
-    bool DestIsHigh = isHighReg(MI.getOperand(0).getReg());
-    bool SrcIsHigh = isHighReg(MI.getOperand(2).getReg());
+    bool DestIsHigh = SystemZ::isHighReg(MI.getOperand(0).getReg());
+    bool SrcIsHigh = SystemZ::isHighReg(MI.getOperand(2).getReg());
     if (SrcIsHigh == DestIsHigh)
       MI.setDesc(get(DestIsHigh ? SystemZ::RISBHH : SystemZ::RISBLL));
     else {
       MI.setDesc(get(DestIsHigh ? SystemZ::RISBHL : SystemZ::RISBLH));
       MI.getOperand(5).setImm(MI.getOperand(5).getImm() ^ 32);
     }
     return true;
   }

lib/Target/SystemZ/SystemZPostRewrite.cpp

 #include "llvm/CodeGen/MachineFunctionPass.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
 using namespace llvm;
 
 #define SYSTEMZ_POSTREWRITE_NAME "SystemZ Post Rewrite pass"
 
 #define DEBUG_TYPE "systemz-postrewrite"
 STATISTIC(MemFoldCopies, "Number of copies inserted before folded mem ops.");
+STATISTIC(LOCRMuxJumps, "Number of LOCRMux jump-sequences (lower is better)");
 
 namespace llvm {
   void initializeSystemZPostRewritePass(PassRegistry&);
 }
 
 namespace {
 
 class SystemZPostRewrite : public MachineFunctionPass {
 public:
   static char ID;
   SystemZPostRewrite() : MachineFunctionPass(ID) {
     initializeSystemZPostRewritePass(*PassRegistry::getPassRegistry());
   }
 
   const SystemZInstrInfo *TII;
 
   bool runOnMachineFunction(MachineFunction &Fn) override;
 
   StringRef getPassName() const override { return SYSTEMZ_POSTREWRITE_NAME; }
 
-  void getAnalysisUsage(AnalysisUsage &AU) const override {
-    AU.setPreservesAll();
-    MachineFunctionPass::getAnalysisUsage(AU);
-  }
-
 private:
+  void selectLOCRMux(MachineBasicBlock &MBB,
+                     MachineBasicBlock::iterator MBBI,
+                     MachineBasicBlock::iterator &NextMBBI,
+                     unsigned LowOpcode,
+                     unsigned HighOpcode);
+  void selectSELRMux(MachineBasicBlock &MBB,
+                     MachineBasicBlock::iterator MBBI,
+                     MachineBasicBlock::iterator &NextMBBI,
+                     unsigned LowOpcode,
+                     unsigned HighOpcode);
+  bool expandCondMove(MachineBasicBlock &MBB,
+                      MachineBasicBlock::iterator MBBI,
+                      MachineBasicBlock::iterator &NextMBBI);
   bool selectMI(MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI,
                 MachineBasicBlock::iterator &NextMBBI);
   bool selectMBB(MachineBasicBlock &MBB);
 };
 
 char SystemZPostRewrite::ID = 0;
 
 } // end anonymous namespace
 
 INITIALIZE_PASS(SystemZPostRewrite, "systemz-post-rewrite",
                 SYSTEMZ_POSTREWRITE_NAME, false, false)
 
 /// Returns an instance of the Post Rewrite pass.
 FunctionPass *llvm::createSystemZPostRewritePass(SystemZTargetMachine &TM) {
   return new SystemZPostRewrite();
 }
 
+// MI is a load-register-on-condition pseudo instruction.  Replace it with
+// LowOpcode if source and destination are both low GR32s and HighOpcode if
+// source and destination are both high GR32s. Otherwise, a branch sequence
+// is created.
+void SystemZPostRewrite::selectLOCRMux(MachineBasicBlock &MBB,
+                                       MachineBasicBlock::iterator MBBI,
+                                       MachineBasicBlock::iterator &NextMBBI,
+                                       unsigned LowOpcode,
+                                       unsigned HighOpcode) {
+  Register DestReg = MBBI->getOperand(0).getReg();
+  Register SrcReg = MBBI->getOperand(2).getReg();
+  bool DestIsHigh = SystemZ::isHighReg(DestReg);
+  bool SrcIsHigh = SystemZ::isHighReg(SrcReg);
+
+  if (!DestIsHigh && !SrcIsHigh)
+    MBBI->setDesc(TII->get(LowOpcode));
+  else if (DestIsHigh && SrcIsHigh)
+    MBBI->setDesc(TII->get(HighOpcode));
+  else
+    expandCondMove(MBB, MBBI, NextMBBI);
+}
+
+// MI is a select pseudo instruction.  Replace it with LowOpcode if source
+// and destination are all low GR32s and HighOpcode if source and destination
+// are all high GR32s. Otherwise, a branch sequence is created.
+void SystemZPostRewrite::selectSELRMux(MachineBasicBlock &MBB,
+                                       MachineBasicBlock::iterator MBBI,
+                                       MachineBasicBlock::iterator &NextMBBI,
+                                       unsigned LowOpcode,
+                                       unsigned HighOpcode) {
+  Register DestReg = MBBI->getOperand(0).getReg();
+  Register Src1Reg = MBBI->getOperand(1).getReg();
+  Register Src2Reg = MBBI->getOperand(2).getReg();
+  bool DestIsHigh = SystemZ::isHighReg(DestReg);
+  bool Src1IsHigh = SystemZ::isHighReg(Src1Reg);
+  bool Src2IsHigh = SystemZ::isHighReg(Src2Reg);
+
+  // If sources and destination aren't all high or all low, we may be able to
+  // simplify the operation by moving one of the sources to the destination
+  // first.  But only if this doesn't clobber the other source.
+  if (DestReg != Src1Reg && DestReg != Src2Reg) {
+    if (DestIsHigh != Src1IsHigh) {
+      BuildMI(*MBBI->getParent(), MBBI, MBBI->getDebugLoc(),
+              TII->get(SystemZ::COPY), DestReg)
+        .addReg(MBBI->getOperand(1).getReg(), getRegState(MBBI->getOperand(1)));
+      MBBI->getOperand(1).setReg(DestReg);
+      Src1Reg = DestReg;
+      Src1IsHigh = DestIsHigh;
+    } else if (DestIsHigh != Src2IsHigh) {
+      BuildMI(*MBBI->getParent(), MBBI, MBBI->getDebugLoc(),
+              TII->get(SystemZ::COPY), DestReg)
+        .addReg(MBBI->getOperand(2).getReg(), getRegState(MBBI->getOperand(2)));
+      MBBI->getOperand(2).setReg(DestReg);
+      Src2Reg = DestReg;
+      Src2IsHigh = DestIsHigh;
+    }
+  }
+
+  // If the destination (now) matches one source, prefer this to be first.
+  if (DestReg != Src1Reg && DestReg == Src2Reg) {
+    TII->commuteInstruction(*MBBI, false, 1, 2);
+    std::swap(Src1Reg, Src2Reg);
+    std::swap(Src1IsHigh, Src2IsHigh);
+  }
+
+  if (!DestIsHigh && !Src1IsHigh && !Src2IsHigh)
+    MBBI->setDesc(TII->get(LowOpcode));
+  else if (DestIsHigh && Src1IsHigh && Src2IsHigh)
+    MBBI->setDesc(TII->get(HighOpcode));
+  else
+    // Given the simplification above, we must already have a two-operand case.
+    expandCondMove(MBB, MBBI, NextMBBI);
+}
+
+// Replace MBBI by a branch sequence that performs a conditional move of
+// operand 2 to the destination register. Operand 1 is expected to be the
+// same register as the destination.
+bool SystemZPostRewrite::expandCondMove(MachineBasicBlock &MBB,
+                                        MachineBasicBlock::iterator MBBI,
+                                        MachineBasicBlock::iterator &NextMBBI) {
+  MachineFunction &MF = *MBB.getParent();
+  const BasicBlock *BB = MBB.getBasicBlock();
+  MachineInstr &MI = *MBBI;
+  DebugLoc DL = MI.getDebugLoc();
+  Register DestReg = MI.getOperand(0).getReg();
+  Register SrcReg = MI.getOperand(2).getReg();
+  unsigned CCValid = MI.getOperand(3).getImm();
+  unsigned CCMask = MI.getOperand(4).getImm();
+  assert(DestReg == MI.getOperand(1).getReg() &&
+         "Expected destination and first source operand to be the same.");
+
+  LivePhysRegs LiveRegs(TII->getRegisterInfo());
+  LiveRegs.addLiveOuts(MBB);
+  for (auto I = std::prev(MBB.end()); I != MBBI; --I)
+    LiveRegs.stepBackward(*I);
+
+  // Splice MBB at MI, moving the rest of the block into RestMBB.
+  MachineBasicBlock *RestMBB = MF.CreateMachineBasicBlock(BB);
+  MF.insert(std::next(MachineFunction::iterator(MBB)), RestMBB);
+  RestMBB->splice(RestMBB->begin(), &MBB, MI, MBB.end());
+  RestMBB->transferSuccessors(&MBB);
+  for (auto I = LiveRegs.begin(); I != LiveRegs.end(); ++I)
+    RestMBB->addLiveIn(*I);
+
+  // Create a new block MoveMBB to hold the move instruction.
+  MachineBasicBlock *MoveMBB = MF.CreateMachineBasicBlock(BB);
+  MF.insert(std::next(MachineFunction::iterator(MBB)), MoveMBB);
+  MoveMBB->addLiveIn(SrcReg);
+  for (auto I = LiveRegs.begin(); I != LiveRegs.end(); ++I)
+    MoveMBB->addLiveIn(*I);
+
+  // At the end of MBB, create a conditional branch to RestMBB if the
+  // condition is false, otherwise fall through to MoveMBB.
+  BuildMI(&MBB, DL, TII->get(SystemZ::BRC))
+    .addImm(CCValid).addImm(CCMask ^ CCValid).addMBB(RestMBB);
+  MBB.addSuccessor(RestMBB);
+  MBB.addSuccessor(MoveMBB);
+
+  // In MoveMBB, emit an instruction to move SrcReg into DestReg,
+  // then fall through to RestMBB.
+  BuildMI(*MoveMBB, MoveMBB->end(), DL, TII->get(SystemZ::COPY), DestReg)
+      .addReg(MI.getOperand(2).getReg(), getRegState(MI.getOperand(2)));
+  MoveMBB->addSuccessor(RestMBB);
+
+  NextMBBI = MBB.end();
+  MI.eraseFromParent();
+  LOCRMuxJumps++;
+  return true;
+}
+
 /// If MBBI references a pseudo instruction that should be selected here,
 /// do it and return true.  Otherwise return false.
 bool SystemZPostRewrite::selectMI(MachineBasicBlock &MBB,
                                   MachineBasicBlock::iterator MBBI,
                                   MachineBasicBlock::iterator &NextMBBI) {
   MachineInstr &MI = *MBBI;
   unsigned Opcode = MI.getOpcode();
 
   // Note: If this could be done during regalloc in foldMemoryOperandImpl()
   // while also updating the LiveIntervals, there would be no need for the
   // MemFoldPseudo to begin with.
   int TargetMemOpcode = SystemZ::getTargetMemOpcode(Opcode);
   if (TargetMemOpcode != -1) {
     MI.setDesc(TII->get(TargetMemOpcode));
     MI.tieOperands(0, 1);
     Register DstReg = MI.getOperand(0).getReg();
     MachineOperand &SrcMO = MI.getOperand(1);
     if (DstReg != SrcMO.getReg()) {
       BuildMI(MBB, &MI, MI.getDebugLoc(), TII->get(SystemZ::COPY), DstReg)
         .addReg(SrcMO.getReg());
       SrcMO.setReg(DstReg);
       MemFoldCopies++;
     }
     return true;
   }
 
+  switch (Opcode) {
+  case SystemZ::LOCRMux:
+    selectLOCRMux(MBB, MBBI, NextMBBI, SystemZ::LOCR, SystemZ::LOCFHR);
+    return true;
+  case SystemZ::SELRMux:
+    selectSELRMux(MBB, MBBI, NextMBBI, SystemZ::SELR, SystemZ::SELFHR);
+    return true;
+  }
+
   return false;
 }
 
 /// Iterate over the instructions in basic block MBB and select any
 /// pseudo instructions.  Return true if anything was modified.
 bool SystemZPostRewrite::selectMBB(MachineBasicBlock &MBB) {
   bool Modified = false;
 
   MachineBasicBlock::iterator MBBI = MBB.begin(), E = MBB.end();
   while (MBBI != E) {
     MachineBasicBlock::iterator NMBBI = std::next(MBBI);
     Modified |= selectMI(MBB, MBBI, NMBBI);

lib/Target/SystemZ/SystemZRegisterInfo.h

 // Return the subreg to use for referring to the even and odd registers
 // in a GR128 pair.  Is32Bit says whether we want a GR32 or GR64.
 inline unsigned even128(bool Is32bit) {
   return Is32bit ? subreg_hl32 : subreg_h64;
 }
 inline unsigned odd128(bool Is32bit) {
   return Is32bit ? subreg_l32 : subreg_l64;
 }
+
+// Reg should be a 32-bit GPR.  Return true if it is a high register rather
+// than a low register.
+inline bool isHighReg(unsigned int Reg) {
+  if (SystemZ::GRH32BitRegClass.contains(Reg))
+    return true;
+  assert(SystemZ::GR32BitRegClass.contains(Reg) && "Invalid GRX32");
+  return false;
+}
 } // end namespace SystemZ
 
 struct SystemZRegisterInfo : public SystemZGenRegisterInfo {
 public:
   SystemZRegisterInfo();
 
   /// getPointerRegClass - Return the register class to use to hold pointers.
   /// This is currently only used by LOAD_STACK_GUARD, which requires a non-%r0

lib/Target/SystemZ/SystemZTargetMachine.cpp

 }
 
 void SystemZPassConfig::addPreSched2() {
   // PostRewrite needs to be run at -O0 also (in which case addPostRewrite()
   // is not called).
   if (getOptLevel() == CodeGenOpt::None)
     addPass(createSystemZPostRewritePass(getSystemZTargetMachine()));
 
-  addPass(createSystemZExpandPseudoPass(getSystemZTargetMachine()));
-
   if (getOptLevel() != CodeGenOpt::None)
     addPass(&IfConverterID);
 }
 
 void SystemZPassConfig::addPreEmitPass() {
   // Do instruction shortening before compare elimination because some
   // vector instructions will be shortened into opcodes that compare
   // elimination recognizes.