[X86] Preserve FPSW when popping x87 stack

When compiler converts x87 operations to stack model, it may insert
instructions that pop top stack element. To do it the compiler inserts
instruction FSTP right after the instruction that calculates value on
the stack. It can break the code that uses FPSW set by the last
instruction. For example, an instruction FXAM is usually followed by
FNSTSW, but FSTP is inserted after FXAM. As FSTP leaves condition code
in FPSW undefined, the compiler produces incorrect code.

With this change FSTP in inserted after the FPSW consumer if the last
instruction sets FPSW.

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

llvm/lib/Target/X86/X86FloatingPoint.cpp

@@ -832,6 +832,24 @@ static const TableEntry PopTable[] = {
   { X86::UCOM_Fr   , X86::UCOM_FPr    },
 };
 
+static bool doesInstructionSetFPSW(MachineInstr &MI) {
+  if (const MachineOperand *MO = MI.findRegisterDefOperand(X86::FPSW))
+    if (!MO->isDead())
+      return true;
+  return false;
+}
+
+static MachineBasicBlock::iterator
+getNextFPInstruction(MachineBasicBlock::iterator I) {
+  MachineBasicBlock &MBB = *I->getParent();
+  while (++I != MBB.end()) {
+    MachineInstr &MI = *I;
+    if (X86::isX87Instruction(MI))
+      return I;
+  }
+  return MBB.end();
+}
+
 /// popStackAfter - Pop the current value off of the top of the FP stack after
 /// the specified instruction.  This attempts to be sneaky and combine the pop
 /// into the instruction itself if possible.  The iterator is left pointing to
@@ -853,6 +871,14 @@ void FPS::popStackAfter(MachineBasicBlock::iterator &I) {
       I->RemoveOperand(0);
     MI.dropDebugNumber();
   } else {    // Insert an explicit pop
+    // If this instruction sets FPSW, which is read in following instruction,
+    // insert pop after that reader.
+    if (doesInstructionSetFPSW(MI)) {
+      MachineBasicBlock &MBB = *MI.getParent();
+      MachineBasicBlock::iterator Next = getNextFPInstruction(I);
+      if (Next != MBB.end() && Next->readsRegister(X86::FPSW))
+        I = Next;
+    }
     I = BuildMI(*MBB, ++I, dl, TII->get(X86::ST_FPrr)).addReg(X86::ST0);
   }
 }

llvm/lib/Target/X86/X86InsertWait.cpp

@@ -55,23 +55,6 @@ char WaitInsert::ID = 0;
 
 FunctionPass *llvm::createX86InsertX87waitPass() { return new WaitInsert(); }
 
-/// Return true if the Reg is X87 register.
-static bool isX87Reg(unsigned Reg) {
-  return (Reg == X86::FPCW || Reg == X86::FPSW ||
-          (Reg >= X86::ST0 && Reg <= X86::ST7));
-}
-
-/// check if the instruction is X87 instruction
-static bool isX87Instruction(MachineInstr &MI) {
-  for (const MachineOperand &MO : MI.operands()) {
-    if (!MO.isReg())
-      continue;
-    if (isX87Reg(MO.getReg()))
-      return true;
-  }
-  return false;
-}
-
 static bool isX87ControlInstruction(MachineInstr &MI) {
   switch (MI.getOpcode()) {
   case X86::FNINIT:
@@ -121,7 +104,7 @@ bool WaitInsert::runOnMachineFunction(MachineFunction &MF) {
   for (MachineBasicBlock &MBB : MF) {
     for (MachineBasicBlock::iterator MI = MBB.begin(); MI != MBB.end(); ++MI) {
       // Jump non X87 instruction.
-      if (!isX87Instruction(*MI))
+      if (!X86::isX87Instruction(*MI))
         continue;
       // If the instruction instruction neither has float exception nor is
       // a load/store instruction, or the instruction is x87 control
@@ -132,7 +115,7 @@ bool WaitInsert::runOnMachineFunction(MachineFunction &MF) {
       // If the following instruction is an X87 instruction and isn't an X87
       // non-waiting control instruction, we can omit insert wait instruction.
       MachineBasicBlock::iterator AfterMI = std::next(MI);
-      if (AfterMI != MBB.end() && isX87Instruction(*AfterMI) &&
+      if (AfterMI != MBB.end() && X86::isX87Instruction(*AfterMI) &&
           !isX87NonWaitingControlInstruction(*AfterMI))
         continue;
 

llvm/lib/Target/X86/X86InstrFPStack.td

@@ -377,7 +377,7 @@ def TST_F  : FPI<0xD9, MRM_E4, (outs), (ins), "ftst">;
 } // SchedRW
 } // Uses = [FPCW], mayRaiseFPException = 1
 
-let SchedRW = [WriteFTest] in {
+let SchedRW = [WriteFTest], Defs = [FPSW] in {
 def XAM_Fp32  : FpIf32<(outs), (ins RFP32:$src), OneArgFP, []>;
 def XAM_Fp64  : FpIf64<(outs), (ins RFP64:$src), OneArgFP, []>;
 def XAM_Fp80  : FpI_<(outs), (ins RFP80:$src), OneArgFP, []>;

llvm/lib/Target/X86/X86InstrInfo.cpp

@@ -2948,6 +2948,23 @@ unsigned X86::getSwappedVCMPImm(unsigned Imm) {
   return Imm;
 }
 
+/// Return true if the Reg is X87 register.
+static bool isX87Reg(unsigned Reg) {
+  return (Reg == X86::FPCW || Reg == X86::FPSW ||
+          (Reg >= X86::ST0 && Reg <= X86::ST7));
+}
+
+/// check if the instruction is X87 instruction
+bool X86::isX87Instruction(MachineInstr &MI) {
+  for (const MachineOperand &MO : MI.operands()) {
+    if (!MO.isReg())
+      continue;
+    if (isX87Reg(MO.getReg()))
+      return true;
+  }
+  return false;
+}
+
 bool X86InstrInfo::isUnconditionalTailCall(const MachineInstr &MI) const {
   switch (MI.getOpcode()) {
   case X86::TCRETURNdi:

llvm/lib/Target/X86/X86InstrInfo.h

@@ -65,6 +65,8 @@ unsigned getSwappedVPCOMImm(unsigned Imm);
 /// Get the VCMP immediate if the opcodes are swapped.
 unsigned getSwappedVCMPImm(unsigned Imm);
 
+/// Check if the instruction is X87 instruction.
+bool isX87Instruction(MachineInstr &MI);
 } // namespace X86
 
 /// isGlobalStubReference - Return true if the specified TargetFlag operand is

llvm/test/CodeGen/X86/x87-stack-pop.mir

@@ -0,0 +1,67 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple=i686-- -run-pass x86-codegen -O2 -o - %s | FileCheck %s
+
+---
+name: func_fxam
+tracksRegLiveness: true
+fixedStack:
+  - { id: 0, type: default, offset: 0, size: 10, alignment: 16 }
+body:             |
+  bb.0.entry:
+    ; CHECK-LABEL: name: func_fxam
+    ; CHECK: nofpexcept LD_F80m %fixed-stack.0, 1, $noreg, 0, $noreg, implicit-def dead $fpsw, implicit $fpcw, implicit-def $st0 :: (load (s80) from %fixed-stack.0, align 16)
+    ; CHECK-NEXT: XAM_F implicit-def $fpsw, implicit $st0
+    ; CHECK-NEXT: FNSTSW16r implicit-def $ax, implicit-def dead $fpsw, implicit $fpsw
+    ; CHECK-NEXT: ST_FPrr $st0, implicit-def $fpsw, implicit $fpcw
+    ; CHECK-NEXT: renamable $ax = KILL $ax, implicit-def $eax
+    ; CHECK-NEXT: RET 0, $eax
+    renamable $fp0 = nofpexcept LD_Fp80m %fixed-stack.0, 1, $noreg, 0, $noreg, implicit-def dead $fpsw, implicit $fpcw :: (load (s80) from %fixed-stack.0, align 16)
+    XAM_Fp80 killed renamable $fp0, implicit-def $fpsw
+    FNSTSW16r implicit-def $ax, implicit-def dead $fpsw, implicit $fpsw
+    renamable $ax = KILL $ax, implicit-def $eax
+    RET 0, $eax
+
+
+...
+---
+name: func_ftst
+tracksRegLiveness: true
+fixedStack:
+  - { id: 0, type: default, offset: 0, size: 10, alignment: 16 }
+body:             |
+  bb.0.entry:
+    ; CHECK-LABEL: name: func_ftst
+    ; CHECK: nofpexcept LD_F80m %fixed-stack.0, 1, $noreg, 0, $noreg, implicit-def dead $fpsw, implicit $fpcw, implicit-def $st0 :: (load (s80) from %fixed-stack.0, align 16)
+    ; CHECK-NEXT: TST_F implicit-def $fpsw, implicit $fpcw, implicit $st0
+    ; CHECK-NEXT: FNSTSW16r implicit-def $ax, implicit-def dead $fpsw, implicit $fpsw
+    ; CHECK-NEXT: ST_FPrr $st0, implicit-def $fpsw, implicit $fpcw
+    ; CHECK-NEXT: renamable $ax = KILL $ax, implicit-def $eax
+    ; CHECK-NEXT: RET 0, $eax
+    renamable $fp0 = nofpexcept LD_Fp80m %fixed-stack.0, 1, $noreg, 0, $noreg, implicit-def dead $fpsw, implicit $fpcw :: (load (s80) from %fixed-stack.0, align 16)
+    TST_Fp80 killed renamable $fp0, implicit-def $fpsw, implicit $fpcw
+    FNSTSW16r implicit-def $ax, implicit-def dead $fpsw, implicit $fpsw
+    renamable $ax = KILL $ax, implicit-def $eax
+    RET 0, $eax
+
+...
+---
+name: func_deaddef
+tracksRegLiveness: true
+fixedStack:
+  - { id: 0, type: default, offset: 0, size: 10, alignment: 16 }
+body:             |
+  bb.0.entry:
+    ; CHECK-LABEL: name: func_deaddef
+    ; CHECK: nofpexcept LD_F80m %fixed-stack.0, 1, $noreg, 0, $noreg, implicit-def dead $fpsw, implicit $fpcw, implicit-def $st0 :: (load (s80) from %fixed-stack.0, align 16)
+    ; CHECK-NEXT: TST_F implicit-def dead $fpsw, implicit $fpcw, implicit $st0
+    ; CHECK-NEXT: ST_FPrr $st0, implicit-def $fpsw, implicit $fpcw
+    ; CHECK-NEXT: FNSTSW16r implicit-def $ax, implicit-def dead $fpsw, implicit $fpsw
+    ; CHECK-NEXT: renamable $ax = KILL $ax, implicit-def $eax
+    ; CHECK-NEXT: RET 0, $eax
+    renamable $fp0 = nofpexcept LD_Fp80m %fixed-stack.0, 1, $noreg, 0, $noreg, implicit-def dead $fpsw, implicit $fpcw :: (load (s80) from %fixed-stack.0, align 16)
+    TST_Fp80 killed renamable $fp0, implicit-def dead $fpsw, implicit $fpcw
+    FNSTSW16r implicit-def $ax, implicit-def dead $fpsw, implicit $fpsw
+    renamable $ax = KILL $ax, implicit-def $eax
+    RET 0, $eax
+
+...