From 656d44c5dadd3a0878221aafc5416369514cf4ec Mon Sep 17 00:00:00 2001 From: Andrew Kaylor Date: Tue, 2 Jan 2018 21:04:38 +0000 Subject: [PATCH] Handle the case of live 16-bit subregisters in X86FixupBWInsts Differential Revision: https://reviews.llvm.org/D40524 Change-Id: Ie3a405b28503ceae999f5f3ba07a68fa733a2400 llvm-svn: 321674 --- lib/Target/X86/X86FixupBWInsts.cpp | 163 ++++++++++++++--------------- test/CodeGen/X86/fixup-bw-inst.mir | 50 +++++++++ 2 files changed, 126 insertions(+), 87 deletions(-) diff --git a/lib/Target/X86/X86FixupBWInsts.cpp b/lib/Target/X86/X86FixupBWInsts.cpp index 01d10fe4cae..855ea683a8a 100644 --- a/lib/Target/X86/X86FixupBWInsts.cpp +++ b/lib/Target/X86/X86FixupBWInsts.cpp @@ -166,81 +166,6 @@ 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 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(?%) -/// -/// %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)) - return false; - - unsigned Opc = MI.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. - assert((Opc == X86::MOV8rm || Opc == X86::MOV16rm || Opc == X86::MOV8rr || - Opc == X86::MOV16rr) && - "Unexpected opcode."); - - bool IsDefined = false; - for (auto &MO: MI.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) { - if (*Supers == MO.getReg()) { - if (MO.isDef()) - IsDefined = true; - else - return true; // 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; - - // 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. @@ -262,20 +187,84 @@ bool FixupBWInstPass::getSuperRegDestIfDead(MachineInstr *OrigMI, 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; + // 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 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 = 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. + assert((Opc == X86::MOV8rm || Opc == X86::MOV16rm || Opc == X86::MOV8rr || + Opc == X86::MOV16rr) && + "Unexpected opcode."); + + bool IsDefined = false; + for (auto &MO: OrigMI->implicit_operands()) { + if (!MO.isReg()) + continue; + + assert((MO.isDef() || MO.isUse()) && "Expected Def or Use only!"); + + for (MCSuperRegIterator Supers(OrigDestReg, TRI, true); Supers.isValid(); + ++Supers) { + if (*Supers == MO.getReg()) { + if (MO.isDef()) + IsDefined = true; + else + 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 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 true; } diff --git a/test/CodeGen/X86/fixup-bw-inst.mir b/test/CodeGen/X86/fixup-bw-inst.mir index cea483e1b9b..e5a5e16108f 100644 --- a/test/CodeGen/X86/fixup-bw-inst.mir +++ b/test/CodeGen/X86/fixup-bw-inst.mir @@ -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 @@ -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 +...