Skip to content

Commit

Permalink
[x86] Teach the x86 backend that it can fold between TCRETURNm* and T…
Browse files Browse the repository at this point in the history
…CRETURNr* and fix latent bugs with register class updates.

Summary:
Enabling this fully exposes a latent bug in the instruction folding: we
never update the register constraints for the register operands when
fusing a load into another operation. The fused form could, in theory,
have different register constraints on its operands. And in fact,
TCRETURNm* needs its memory operands to use tailcall compatible
registers.

I've updated the folding code to re-constrain all the registers after
they are mapped onto their new instruction.

However, we still can't enable folding in the general case from
TCRETURNr* to TCRETURNm* because doing so may require more registers to
be available during the tail call. If the call itself uses all but one
register, and the folded load would require both a base and index
register, there will not be enough registers to allocate the tail call.

It would be better, IMO, to teach the register allocator to *unfold*
TCRETURNm* when it runs out of registers (or specifically check the
number of registers available during the TCRETURNr*) but I'm not going
to try and solve that for now. Instead, I've just blocked the forward
folding from r -> m, leaving LLVM free to unfold from m -> r as that
doesn't introduce new register pressure constraints.

The down side is that I don't have anything that will directly exercise
this. Instead, I will be immediately using this it my SLH patch. =/

Still worse, without allowing the TCRETURNr* -> TCRETURNm* fold, I don't
have any tests that demonstrate the failure to update the memory operand
register constraints. This patch still seems correct, but I'm nervous
about the degree of testing due to this.

Suggestions?

Reviewers: craig.topper

Subscribers: sanjoy, mcrosier, hiraditya, llvm-commits

Differential Revision: https://reviews.llvm.org/D49717

llvm-svn: 337845
  • Loading branch information
chandlerc committed Jul 24, 2018
1 parent 1d504f7 commit c9313a9
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 0 deletions.
2 changes: 2 additions & 0 deletions llvm/lib/Target/X86/X86InstrFoldTables.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,8 @@ static const X86MemoryFoldTableEntry MemoryFoldTable0[] = {
{ X86::TAILJMPr, X86::TAILJMPm, TB_FOLDED_LOAD },
{ X86::TAILJMPr64, X86::TAILJMPm64, TB_FOLDED_LOAD },
{ X86::TAILJMPr64_REX, X86::TAILJMPm64_REX, TB_FOLDED_LOAD },
{ X86::TCRETURNri, X86::TCRETURNmi, TB_FOLDED_LOAD | TB_NO_FORWARD },
{ X86::TCRETURNri64, X86::TCRETURNmi64, TB_FOLDED_LOAD | TB_NO_FORWARD },
{ X86::TEST16ri, X86::TEST16mi, TB_FOLDED_LOAD },
{ X86::TEST16rr, X86::TEST16mr, TB_FOLDED_LOAD },
{ X86::TEST32ri, X86::TEST32mi, TB_FOLDED_LOAD },
Expand Down
31 changes: 31 additions & 0 deletions llvm/lib/Target/X86/X86InstrInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "X86Subtarget.h"
#include "X86TargetMachine.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/Sequence.h"
#include "llvm/CodeGen/LivePhysRegs.h"
#include "llvm/CodeGen/LiveVariables.h"
#include "llvm/CodeGen/MachineConstantPool.h"
Expand Down Expand Up @@ -4652,6 +4653,32 @@ static void addOperands(MachineInstrBuilder &MIB, ArrayRef<MachineOperand> MOs,
}
}

static void updateOperandRegConstraints(MachineFunction &MF,
MachineInstr &NewMI,
const TargetInstrInfo &TII) {
MachineRegisterInfo &MRI = MF.getRegInfo();
const TargetRegisterInfo &TRI = *MRI.getTargetRegisterInfo();

for (int Idx : llvm::seq<int>(0, NewMI.getNumOperands())) {
MachineOperand &MO = NewMI.getOperand(Idx);
// We only need to update constraints on virtual register operands.
if (!MO.isReg())
continue;
unsigned Reg = MO.getReg();
if (!TRI.isVirtualRegister(Reg))
continue;

auto *NewRC = MRI.constrainRegClass(
Reg, TII.getRegClass(NewMI.getDesc(), Idx, &TRI, MF));
if (!NewRC) {
LLVM_DEBUG(
dbgs() << "WARNING: Unable to update register constraint for operand "
<< Idx << " of instruction:\n";
NewMI.dump(); dbgs() << "\n");
}
}
}

static MachineInstr *FuseTwoAddrInst(MachineFunction &MF, unsigned Opcode,
ArrayRef<MachineOperand> MOs,
MachineBasicBlock::iterator InsertPt,
Expand All @@ -4675,6 +4702,8 @@ static MachineInstr *FuseTwoAddrInst(MachineFunction &MF, unsigned Opcode,
MIB.add(MO);
}

updateOperandRegConstraints(MF, *NewMI, TII);

MachineBasicBlock *MBB = InsertPt->getParent();
MBB->insert(InsertPt, NewMI);

Expand All @@ -4701,6 +4730,8 @@ static MachineInstr *FuseInst(MachineFunction &MF, unsigned Opcode,
}
}

updateOperandRegConstraints(MF, *NewMI, TII);

MachineBasicBlock *MBB = InsertPt->getParent();
MBB->insert(InsertPt, NewMI);

Expand Down

0 comments on commit c9313a9

Please sign in to comment.