[llvm] 3057e85 - [X86] Preserve FPSW when popping x87 stack

Serge Pavlov via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 11 21:56:10 PST 2021


Author: Serge Pavlov
Date: 2021-11-12T12:00:09+07:00
New Revision: 3057e850b88e8faf754db02293066e1182da2954

URL: https://github.com/llvm/llvm-project/commit/3057e850b88e8faf754db02293066e1182da2954
DIFF: https://github.com/llvm/llvm-project/commit/3057e850b88e8faf754db02293066e1182da2954.diff

LOG: [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

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

Modified: 
    llvm/lib/Target/X86/X86FloatingPoint.cpp
    llvm/lib/Target/X86/X86InsertWait.cpp
    llvm/lib/Target/X86/X86InstrFPStack.td
    llvm/lib/Target/X86/X86InstrInfo.cpp
    llvm/lib/Target/X86/X86InstrInfo.h

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/X86/X86FloatingPoint.cpp b/llvm/lib/Target/X86/X86FloatingPoint.cpp
index 8ee503e58e47..60e1b37ed61c 100644
--- a/llvm/lib/Target/X86/X86FloatingPoint.cpp
+++ b/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);
   }
 }

diff  --git a/llvm/lib/Target/X86/X86InsertWait.cpp b/llvm/lib/Target/X86/X86InsertWait.cpp
index 56d2709f5937..69a3d32a9314 100644
--- a/llvm/lib/Target/X86/X86InsertWait.cpp
+++ b/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;
 

diff  --git a/llvm/lib/Target/X86/X86InstrFPStack.td b/llvm/lib/Target/X86/X86InstrFPStack.td
index cda28d18f4aa..e310f369be08 100644
--- a/llvm/lib/Target/X86/X86InstrFPStack.td
+++ b/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, []>;

diff  --git a/llvm/lib/Target/X86/X86InstrInfo.cpp b/llvm/lib/Target/X86/X86InstrInfo.cpp
index 15a305df7f15..769874d1b740 100644
--- a/llvm/lib/Target/X86/X86InstrInfo.cpp
+++ b/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:

diff  --git a/llvm/lib/Target/X86/X86InstrInfo.h b/llvm/lib/Target/X86/X86InstrInfo.h
index 30b455d1724b..b07e82b64277 100644
--- a/llvm/lib/Target/X86/X86InstrInfo.h
+++ b/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

diff  --git a/llvm/test/CodeGen/X86/x87-stack-pop.mir b/llvm/test/CodeGen/X86/x87-stack-pop.mir
new file mode 100644
index 000000000000..1c4ffa54b150
--- /dev/null
+++ b/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
+
+...


        


More information about the llvm-commits mailing list