From: Quentin Colombet Date: Tue, 10 May 2016 20:49:46 +0000 (+0000) Subject: [X86] Properly check that EAX is dead when copying EFLAGS. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=5e0f621a7bd1490c809dbb2dec765e469a9e554a;p=llvm [X86] Properly check that EAX is dead when copying EFLAGS. This fixes a bug introduced in r267623, where we got smarter and avoided to save EAX before using it. However, we failed to check if any of the subregister of EAX were alive and thus, missed cases where we have to save EAX before using it. The problem may happen on every X86/i386/... platform. This fixes llvm.org/PR27624 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@269115 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Target/X86/X86InstrInfo.cpp b/lib/Target/X86/X86InstrInfo.cpp index f6c11c80855..11c532c7d71 100644 --- a/lib/Target/X86/X86InstrInfo.cpp +++ b/lib/Target/X86/X86InstrInfo.cpp @@ -4527,8 +4527,9 @@ void X86InstrInfo::copyPhysReg(MachineBasicBlock &MBB, // first frame index. // See X86ISelLowering.cpp - X86::hasCopyImplyingStackAdjustment. + const TargetRegisterInfo *TRI = &getRegisterInfo(); MachineBasicBlock::LivenessQueryResult LQR = - MBB.computeRegisterLiveness(&getRegisterInfo(), AX, MI); + MBB.computeRegisterLiveness(TRI, AX, MI); // We do not want to save and restore AX if we do not have to. // Moreover, if we do so whereas AX is dead, we would need to set // an undef flag on the use of AX, otherwise the verifier will @@ -4536,15 +4537,19 @@ void X86InstrInfo::copyPhysReg(MachineBasicBlock &MBB, // We do not want to change the behavior of the machine verifier // as this is usually wrong to read an undef value. if (MachineBasicBlock::LQR_Unknown == LQR) { - LivePhysRegs LPR(&getRegisterInfo()); + LivePhysRegs LPR(TRI); LPR.addLiveOuts(MBB); MachineBasicBlock::iterator I = MBB.end(); while (I != MI) { --I; LPR.stepBackward(*I); } - LQR = LPR.contains(AX) ? MachineBasicBlock::LQR_Live - : MachineBasicBlock::LQR_Dead; + // AX contains the top most register in the aliasing hierarchy. + // It may not be live, but one of its aliases may be. + for (MCRegAliasIterator AI(AX, TRI, true); + AI.isValid() && LQR != MachineBasicBlock::LQR_Live; ++AI) + LQR = LPR.contains(*AI) ? MachineBasicBlock::LQR_Live + : MachineBasicBlock::LQR_Dead; } bool AXDead = (Reg == AX) || (MachineBasicBlock::LQR_Dead == LQR); if (!AXDead) diff --git a/test/CodeGen/X86/eflags-copy-expansion.mir b/test/CodeGen/X86/eflags-copy-expansion.mir new file mode 100644 index 00000000000..03f9072dc0e --- /dev/null +++ b/test/CodeGen/X86/eflags-copy-expansion.mir @@ -0,0 +1,67 @@ +# RUN: llc -run-pass postrapseudos -mtriple=i386-apple-macosx -o /dev/null %s 2>&1 | FileCheck %s + +# Verify that we correctly save and restore eax when copying eflags, +# even when only a smaller alias of eax is used. We used to check only +# eax and not its aliases. +# PR27624. + +--- | + target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128" + + define void @foo() { + entry: + br label %false + false: + ret void + } + +... + +--- +name: foo +allVRegsAllocated: true +isSSA: false +tracksRegLiveness: true +liveins: + - { reg: '%edi' } +body: | + bb.0.entry: + liveins: %edi + successors: %bb.1.false + NOOP implicit-def %al + + ; The bug was triggered only when LivePhysReg is used, which + ; happens only when the heuristic for the liveness computation + ; failed. The liveness computation heuristic looks at 10 instructions + ; before and after the copy. Make sure we do not reach the definition of + ; AL in 10 instructions, otherwise the heuristic will see that it is live. + NOOP + NOOP + NOOP + NOOP + NOOP + NOOP + NOOP + NOOP + NOOP + NOOP + NOOP + NOOP + NOOP + ; Save AL. + ; CHECK: PUSH32r killed %eax + + ; Copy EDI into EFLAGS + ; CHECK-NEXT: %eax = MOV32rr %edi + ; CHECK-NEXT: %al = ADD8ri %al, 127, implicit-def %eflags + ; CHECK-NEXT: SAHF implicit-def %eflags, implicit %ah + %eflags = COPY %edi + + ; Restore AL. + ; CHECK-NEXT: %eax = POP32r + bb.1.false: + liveins: %al + NOOP implicit %al + RETQ + +...