Skip to content

Commit

Permalink
Handle the case of live 16-bit subregisters in X86FixupBWInsts
Browse files Browse the repository at this point in the history 
Differential Revision: https://reviews.llvm.org/D40524

Change-Id: Ie3a405b28503ceae999f5f3ba07a68fa733a2400
llvm-svn: 321674
  • Loading branch information
@andykaylor
andykaylor committed Jan 2, 2018
1 parent 24e6a8b commit e12e08c
Show file tree
Hide file tree
Showing 2 changed files with 121 additions and 82 deletions.
153 changes: 71 additions & 82 deletions llvm/lib/Target/X86/X86FixupBWInsts.cpp
View file
Expand Up @@ -166,49 +166,75 @@ bool FixupBWInstPass::runOnMachineFunction(MachineFunction &MF) {
return true;
}

/// Check if register \p Reg is live after the \p MI.
///
/// \p LiveRegs should be in a state describing liveness information in
/// that exact place as this function tries to precise analysis made
/// by \p LiveRegs by exploiting the information about particular
/// instruction \p MI. \p MI is expected to be one of the MOVs handled
/// by the x86FixupBWInsts pass.
/// Note: similar to LivePhysRegs::contains this would state that
/// super-register is not used if only some part of it is used.
///
/// X86 backend does not have subregister liveness tracking enabled,
/// so liveness information might be overly conservative. However, for
/// some specific instructions (this pass only cares about MOVs) we can
/// produce more precise results by analysing that MOV's operands.
///
/// Indeed, if super-register is not live before the mov it means that it
/// was originally <read-undef> and so we are free to modify these
/// undef upper bits. That may happen in case where the use is in another MBB
/// and the vreg/physreg corresponding to the move has higher width than
/// necessary (e.g. due to register coalescing with a "truncate" copy).
/// So, it handles pattern like this:
///
/// %bb.2: derived from LLVM BB %if.then
/// Live Ins: %rdi
/// Predecessors according to CFG: %bb.0
/// %ax = MOV16rm killed %rdi, 1, %noreg, 0, %noreg, implicit-def %eax;
/// mem:LD2[%p]
/// No implicit %eax
/// Successors according to CFG: %bb.3(?%)
/// \brief Check if after \p OrigMI the only portion of super register
/// of the destination register of \p OrigMI that is alive is that
/// destination register.
///
/// %bb.3: derived from LLVM BB %if.end
/// Live Ins: %eax Only %ax is actually live
/// Predecessors according to CFG: %bb.2 %bb.1
/// %ax = KILL %ax, implicit killed %eax
/// RET 0, %ax
static bool isLive(const MachineInstr &MI,
const LivePhysRegs &LiveRegs,
const TargetRegisterInfo *TRI,
unsigned Reg) {
if (!LiveRegs.contains(Reg))
/// If so, return that super register in \p SuperDestReg.
bool FixupBWInstPass::getSuperRegDestIfDead(MachineInstr *OrigMI,
unsigned &SuperDestReg) const {
auto *TRI = &TII->getRegisterInfo();

unsigned OrigDestReg = OrigMI->getOperand(0).getReg();
SuperDestReg = getX86SubSuperRegister(OrigDestReg, 32);

const auto SubRegIdx = TRI->getSubRegIndex(SuperDestReg, OrigDestReg);

// Make sure that the sub-register that this instruction has as its
// destination is the lowest order sub-register of the super-register.
// If it isn't, then the register isn't really dead even if the
// super-register is considered dead.
if (SubRegIdx == X86::sub_8bit_hi)
return false;

unsigned Opc = MI.getOpcode(); (void)Opc;
// If neither the destination-super register nor any applicable subregisters
// are live after this instruction, then the super register is safe to use.
if (!LiveRegs.contains(SuperDestReg)) {
// If the original destination register was not the low 8-bit subregister
// then the super register check is sufficient.
if (SubRegIdx != X86::sub_8bit)
return true;
// If the original destination register was the low 8-bit subregister and
// we also need to check the 16-bit subregister and the high 8-bit
// subregister.
if (!LiveRegs.contains(getX86SubSuperRegister(OrigDestReg, 16)) &&
!LiveRegs.contains(getX86SubSuperRegister(SuperDestReg, 8,
/*High=*/true)))
return true;
// Otherwise, we have a little more checking to do.
}

// If we get here, the super-register destination (or some part of it) is
// marked as live after the original instruction.
//
// The X86 backend does not have subregister liveness tracking enabled,
// so liveness information might be overly conservative. Specifically, the
// super register might be marked as live because it is implicitly defined
// by the instruction we are examining.
//
// However, for some specific instructions (this pass only cares about MOVs)
// we can produce more precise results by analysing that MOV's operands.
//
// Indeed, if super-register is not live before the mov it means that it
// was originally <read-undef> and so we are free to modify these
// undef upper bits. That may happen in case where the use is in another MBB
// and the vreg/physreg corresponding to the move has higher width than
// necessary (e.g. due to register coalescing with a "truncate" copy).
// So, we would like to handle patterns like this:
//
// %bb.2: derived from LLVM BB %if.then
// Live Ins: %rdi
// Predecessors according to CFG: %bb.0
// %ax<def> = MOV16rm killed %rdi, 1, %noreg, 0, %noreg, implicit-def %eax
// ; No implicit %eax
// Successors according to CFG: %bb.3(?%)
//
// %bb.3: derived from LLVM BB %if.end
// Live Ins: %eax Only %ax is actually live
// Predecessors according to CFG: %bb.2 %bb.1
// %ax = KILL %ax, implicit killed %eax
// RET 0, %ax
unsigned Opc = OrigMI->getOpcode(); (void)Opc;
// These are the opcodes currently handled by the pass, if something
// else will be added we need to ensure that new opcode has the same
// properties.
Expand All @@ -217,65 +243,28 @@ static bool isLive(const MachineInstr &MI,
"Unexpected opcode.");

bool IsDefined = false;
for (auto &MO: MI.implicit_operands()) {
for (auto &MO: OrigMI->implicit_operands()) {
if (!MO.isReg())
continue;

assert((MO.isDef() || MO.isUse()) && "Expected Def or Use only!");

for (MCSuperRegIterator Supers(Reg, TRI, true); Supers.isValid(); ++Supers) {
for (MCSuperRegIterator Supers(OrigDestReg, TRI, true); Supers.isValid();
++Supers) {
if (*Supers == MO.getReg()) {
if (MO.isDef())
IsDefined = true;
else
return true; // SuperReg Imp-used' -> live before the MI
return false; // SuperReg Imp-used' -> live before the MI
}
}
}
// Reg is not Imp-def'ed -> it's live both before/after the instruction.
if (!IsDefined)
return true;
return false;

// Otherwise, the Reg is not live before the MI and the MOV can't
// make it really live, so it's in fact dead even after the MI.
return false;
}

/// \brief Check if after \p OrigMI the only portion of super register
/// of the destination register of \p OrigMI that is alive is that
/// destination register.
///
/// If so, return that super register in \p SuperDestReg.
bool FixupBWInstPass::getSuperRegDestIfDead(MachineInstr *OrigMI,
unsigned &SuperDestReg) const {
auto *TRI = &TII->getRegisterInfo();

unsigned OrigDestReg = OrigMI->getOperand(0).getReg();
SuperDestReg = getX86SubSuperRegister(OrigDestReg, 32);

const auto SubRegIdx = TRI->getSubRegIndex(SuperDestReg, OrigDestReg);

// Make sure that the sub-register that this instruction has as its
// destination is the lowest order sub-register of the super-register.
// If it isn't, then the register isn't really dead even if the
// super-register is considered dead.
if (SubRegIdx == X86::sub_8bit_hi)
return false;

if (isLive(*OrigMI, LiveRegs, TRI, SuperDestReg))
return false;

if (SubRegIdx == X86::sub_8bit) {
// In the case of byte registers, we also have to check that the upper
// byte register is also dead. That is considered to be independent of
// whether the super-register is dead.
unsigned UpperByteReg =
getX86SubSuperRegister(SuperDestReg, 8, /*High=*/true);

if (isLive(*OrigMI, LiveRegs, TRI, UpperByteReg))
return false;
}

return true;
}

Expand Down
50 changes: 50 additions & 0 deletions llvm/test/CodeGen/X86/fixup-bw-inst.mir
View file
Expand Up @@ -26,6 +26,12 @@
ret i16 %i.0
}

define i16 @test4() {
entry:
%t1 = zext i1 undef to i16
%t2 = or i16 undef, %t1
ret i16 %t2
}
...
---
# CHECK-LABEL: name: test1
Expand Down Expand Up @@ -149,3 +155,47 @@ body: |
RETQ %ax
...
---
# CHECK-LABEL: name: test4
name: test4
alignment: 4
exposesReturnsTwice: false
legalized: false
regBankSelected: false
selected: false
tracksRegLiveness: true
registers:
liveins:
- { reg: '%r9d' }
frameInfo:
isFrameAddressTaken: false
isReturnAddressTaken: false
hasStackMap: false
hasPatchPoint: false
stackSize: 0
offsetAdjustment: 0
maxAlignment: 0
adjustsStack: false
hasCalls: false
stackProtector: ''
maxCallFrameSize: 0
hasOpaqueSPAdjustment: false
hasVAStart: false
hasMustTailInVarArgFunc: false
savePoint: ''
restorePoint: ''
fixedStack:
stack:
constants:
# This code copies r10b into r9b and then uses r9w. We would like to promote
# the copy to a 32-bit copy, but because r9w is used this is not acceptable.
body: |
bb.0.entry:
successors:
liveins: %r9d
%r9b = MOV8rr undef %r10b, implicit-def %r9d, implicit killed %r9d, implicit-def %eflags
; CHECK-NOT: MOV32rr
%ax = OR16rr undef %ax, %r9w, implicit-def %eflags
RETQ %ax
...

0 comments on commit e12e08c

Please sign in to comment.