Index: lib/Target/X86/X86FixupBWInsts.cpp =================================================================== --- lib/Target/X86/X86FixupBWInsts.cpp +++ lib/Target/X86/X86FixupBWInsts.cpp @@ -166,48 +166,77 @@ 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 %RDI, 1, %noreg, 0, %noreg, %EAX; mem:LD2[%p] -/// No %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, %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 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 %RDI, 1, %noreg, 0, %noreg, %EAX + // ; No %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, %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. @@ -216,65 +245,28 @@ "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; } Index: test/CodeGen/X86/fixup-bw-inst.mir =================================================================== --- test/CodeGen/X86/fixup-bw-inst.mir +++ 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 @@ 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 +...