[X86][ConstraintFP] Model `MXCSR` for function call

[pengfei]

Summary

This patch is inspired by D111433. It would affect the performance under
strict FP mode. But it preserves the correct rounding behavior accross
function calls.

Fixes #59305

[craig.topper]

llvm/test/CodeGen/X86/fp-strict-scalar-inttofp-fp16.ll

384

What happened here?

[pengfei]

llvm/test/CodeGen/X86/fp-strict-scalar-inttofp-fp16.ll

384

Before this change, addss can be sunk into %bb.1. But it fails now because of https://github.com/llvm/llvm-project/blob/main/llvm/lib/CodeGen/MachineSink.cpp#L918
due to MXCSR has a def in this function.

[sepavloff]

llvm/include/llvm/CodeGen/TargetLowering.h

4491–4495

The function name is a bit misleading, maybe getFPEnvironmentRegisters or some thing like that?

If this function lists FP environment registers that can be changed by a call, then any FP control mode register, not only rounding control (for example, denormal treatment or precision control) also can be modified. FP status information, like raised exception, also may be changed in the called procedure, so it is reasonable to include relevant registers as well.

MXCSR contains all this information but other architectures may have different registers for such information.

[pengfei]

llvm/include/llvm/CodeGen/TargetLowering.h

4491–4495

Thanks for the inputs! Modeling all the FP environment registers sounds attractive. However, it is not what the current strict FP required.

1. Denormal treatment is a good point. We haven't modeled it, but I think it's not a problem if we sort it into rounding control.
2. Precision as well as exception control registers do nothing with strict FP, because we just use MIFlag to ensure exception correctness.
3. The exception status registers are the same. We never use this information in strict FP.

In a word, only the rounding behaviors in strict FP mode relies on register modeling. So I think the name reflects the requirments correctly.

[sepavloff]

llvm/include/llvm/CodeGen/TargetLowering.h

4491–4495

Current definition of strictfp in https://llvm.org/docs/LangRef.html#function-attributes explicitly mentions status flags:

LLVM will not attempt any optimizations that require assumptions about the floating-point rounding mode or that might alter the state of floating-point status flags ...

As for other control modes, like denormal mode, they behave similar to rounding direction. If a function call (like C2x library function fesetmode) modifies them, nothing prevents from moving the call across the users of the modified control register.

Listing all FP environment registers saves us from undesired code motion.

[sepavloff]

Reverse ping.

[pengfei]

llvm/include/llvm/CodeGen/TargetLowering.h

4491–4495

It is how it works across the whole compiler pipeline. But I tend to take rounding mode and floating-point status flags as 2 different aspects of strictfp due to their different implementation in backend.
To prevent backend optimizations over FP exceptions, we introduced an MI flag NoFPExcept, which will be set to false for FP instructions that may raise exception under strictfp mode and check mayRaiseFPException before doing optimizations. This is different from we preventing optimizations through modeling rounding control registers.
For example, preventing instruction moving in MachineInstr::isSafeToMove, preventing CSE we are facing here in MachineCSE::isCSECandidate, where isCall is checked too.
That said we don't need to model any FP status flags for call instructions again. Thus we should avoid to use FPEnvironmentRegisters for confusion.
As for denormal mode control, I think it's OK we take them as rounding control class and return them with the same method in future.

[sepavloff]

llvm/include/llvm/CodeGen/TargetLowering.h

4491–4495

Thank you for the detailed explanation!

You are right, separation of status flags from control modes has benefits. It is OK to limit affected registers to control modes only in this patch, status flags anyway are in the same register on X86.

What about using something like getFPControlRegisters as a name?

[pengfei]

llvm/include/llvm/CodeGen/TargetLowering.h

4491–4495

status flags anyway are in the same register on X86.

Actually not, X87 has separate FPSW too.

What about using something like getFPControlRegisters as a name?

I don't think so. The exception mask register is also a FP control register. We don't want to model them too. Actually we have a huge space for performance improvement under strict exception mode if we can correctly model these mask in bits. It needs a large refactor and I don't have a clear idea at the moment.

[pengfei]

llvm/include/llvm/CodeGen/TargetLowering.h

4491–4495

If you like, we may expand it in future with an enum argument RoundingControl, DenormalControl, ExceptionControl, ExceptionStatus etc. for different requirments, then we can change is to getFPEnvironmentRegisters. I think it's sufficient for this patch to go with it.

[sepavloff]

LGTM.

Thanks!

llvm/include/llvm/CodeGen/TargetLowering.h

public:
   }
 
   /// Returns a 0 terminated array of registers that can be safely used as
   /// scratch registers.
   virtual const MCPhysReg *getScratchRegisters(CallingConv::ID CC) const {
     return nullptr;
   }
 
+  /// Returns a 0 terminated array of rounding control registers that can be
+  /// attached into strict FP call.
+  virtual const MCPhysReg *getRoundingControlRegisters() const {
+    return nullptr;
+  }
+
   /// This callback is used to prepare for a volatile or atomic load.
   /// It takes a chain node as input and returns the chain for the load itself.
   ///
   /// Having a callback like this is necessary for targets like SystemZ,
   /// which allows a CPU to reuse the result of a previous load indefinitely,
   /// even if a cache-coherent store is performed by another CPU.  The default
   /// implementation does nothing.
   virtual SDValue prepareVolatileOrAtomicLoad(SDValue Chain, const SDLoc &DL,

llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp

for (SDNode *F = Node->getGluedUser(); F; F = F->getGluedUser()) {
         if (RegisterSDNode *R = dyn_cast<RegisterSDNode>(F->getOperand(i))) {
           Register Reg = R->getReg();
           if (Reg.isPhysical())
             UsedRegs.push_back(Reg);
         }
     }
   }
 
+  // Add rounding control registers as implicit def for function call.
+  if (II.isCall() && MF->getFunction().hasFnAttribute(Attribute::StrictFP)) {
+    const MCPhysReg *RCRegs = TLI->getRoundingControlRegisters();
+    if (RCRegs)
+      for (; *RCRegs; ++RCRegs)
+        UsedRegs.push_back(*RCRegs);
+  }
+
   // Finally mark unused registers as dead.
   if (!UsedRegs.empty() || !II.implicit_defs().empty() || II.hasOptionalDef())
     MIB->setPhysRegsDeadExcept(UsedRegs, *TRI);
 
   // STATEPOINT is too 'dynamic' to have meaningful machine description.
   // We have to manually tie operands.
   if (Opc == TargetOpcode::STATEPOINT && NumDefs > 0) {
     assert(!HasPhysRegOuts && "STATEPOINT mishandled");

llvm/lib/Target/X86/X86ISelLowering.h

EVT getTypeForExtReturn(LLVMContext &Context, EVT VT,
                             ISD::NodeType ExtendKind) const override;
 
     bool CanLowerReturn(CallingConv::ID CallConv, MachineFunction &MF,
                         bool isVarArg,
                         const SmallVectorImpl<ISD::OutputArg> &Outs,
                         LLVMContext &Context) const override;
 
     const MCPhysReg *getScratchRegisters(CallingConv::ID CC) const override;
+    const MCPhysReg *getRoundingControlRegisters() const override;
 
     TargetLoweringBase::AtomicExpansionKind
     shouldExpandAtomicLoadInIR(LoadInst *LI) const override;
     TargetLoweringBase::AtomicExpansionKind
     shouldExpandAtomicStoreInIR(StoreInst *SI) const override;
     TargetLoweringBase::AtomicExpansionKind
     shouldExpandAtomicRMWInIR(AtomicRMWInst *AI) const override;
     TargetLoweringBase::AtomicExpansionKind

llvm/lib/Target/X86/X86ISelLowering.cpp

bool X86TargetLowering::CanLowerReturn(
   return CCInfo.CheckReturn(Outs, RetCC_X86);
 }
 
 const MCPhysReg *X86TargetLowering::getScratchRegisters(CallingConv::ID) const {
   static const MCPhysReg ScratchRegs[] = { X86::R11, 0 };
   return ScratchRegs;
 }
 
+const MCPhysReg *X86TargetLowering::getRoundingControlRegisters() const {
+  // FIXME: We should def X86::FPCW for x87 as well. But it affects a lot of lit
+  // tests at the moment, which is not what we expected.
+  static const MCPhysReg RCRegs[] = { X86::MXCSR, 0 };
+  return RCRegs;
+}
+
 /// Lowers masks values (v*i1) to the local register values
 /// \returns DAG node after lowering to register type
 static SDValue lowerMasksToReg(const SDValue &ValArg, const EVT &ValLoc,
                                const SDLoc &Dl, SelectionDAG &DAG) {
   EVT ValVT = ValArg.getValueType();
 
   if (ValVT == MVT::v1i1)
     return DAG.getNode(ISD::EXTRACT_VECTOR_ELT, Dl, ValLoc, ValArg,

llvm/test/CodeGen/X86/fp-strict-scalar-inttofp-fp16.ll

 ; SSE2:       # %bb.0:
 ; SSE2-NEXT:    movq %rdi, %rax
 ; SSE2-NEXT:    shrq %rax
 ; SSE2-NEXT:    movl %edi, %ecx
 ; SSE2-NEXT:    andl $1, %ecx
 ; SSE2-NEXT:    orq %rax, %rcx
 ; SSE2-NEXT:    testq %rdi, %rdi
 ; SSE2-NEXT:    cmovnsq %rdi, %rcx
-; SSE2-NEXT:    cvtsi2ss %rcx, %xmm0
-; SSE2-NEXT:    jns .LBB9_2
+; SSE2-NEXT:    cvtsi2ss %rcx, %xmm1
+; SSE2-NEXT:    movaps %xmm1, %xmm0
+; SSE2-NEXT:    addss %xmm1, %xmm0
+; SSE2-NEXT:    js .LBB9_2
 ; SSE2-NEXT:  # %bb.1:
-; SSE2-NEXT:    addss %xmm0, %xmm0
+; SSE2-NEXT:    movaps %xmm1, %xmm0
 ; SSE2-NEXT:  .LBB9_2:
 ; SSE2-NEXT:    pushq %rax
 ; SSE2-NEXT:    callq __truncsfhf2@PLT
 ; SSE2-NEXT:    popq %rax
 ; SSE2-NEXT:    retq
 ;
 ; F16C-LABEL: uitofp_i64tof16:
 ; F16C:       # %bb.0:

llvm/test/CodeGen/X86/pr59305.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -mtriple=x86_64-pc-linux < %s | FileCheck %s
 
 define double @foo(double %0) #0 {
 ; CHECK-LABEL: foo:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    pushq %rax
+; CHECK-NEXT:    subq $24, %rsp
 ; CHECK-NEXT:    movsd %xmm0, (%rsp) # 8-byte Spill
 ; CHECK-NEXT:    movl $1024, %edi # imm = 0x400
 ; CHECK-NEXT:    callq fesetround@PLT
-; CHECK-NEXT:    movsd {{.*#+}} xmm0 = mem[0],zero
-; CHECK-NEXT:    divsd (%rsp), %xmm0 # 8-byte Folded Reload
-; CHECK-NEXT:    movsd %xmm0, (%rsp) # 8-byte Spill
+; CHECK-NEXT:    movsd {{.*#+}} xmm1 = mem[0],zero
+; CHECK-NEXT:    divsd (%rsp), %xmm1 # 8-byte Folded Reload
+; CHECK-NEXT:    movsd %xmm1, {{[-0-9]+}}(%r{{[sb]}}p) # 8-byte Spill
 ; CHECK-NEXT:    movl $1024, %edi # imm = 0x400
 ; CHECK-NEXT:    callq fesetround@PLT
+; CHECK-NEXT:    movsd {{.*#+}} xmm0 = mem[0],zero
+; CHECK-NEXT:    divsd (%rsp), %xmm0 # 8-byte Folded Reload
+; CHECK-NEXT:    movsd %xmm0, {{[-0-9]+}}(%r{{[sb]}}p) # 8-byte Spill
 ; CHECK-NEXT:    movl $1024, %edi # imm = 0x400
 ; CHECK-NEXT:    callq fesetround@PLT
-; CHECK-NEXT:    movsd (%rsp), %xmm0 # 8-byte Reload
+; CHECK-NEXT:    movsd {{.*#+}} xmm2 = mem[0],zero
+; CHECK-NEXT:    divsd (%rsp), %xmm2 # 8-byte Folded Reload
+; CHECK-NEXT:    movsd {{[-0-9]+}}(%r{{[sb]}}p), %xmm0 # 8-byte Reload
 ; CHECK-NEXT:    # xmm0 = mem[0],zero
-; CHECK-NEXT:    movaps %xmm0, %xmm1
-; CHECK-NEXT:    movaps %xmm0, %xmm2
+; CHECK-NEXT:    movsd {{[-0-9]+}}(%r{{[sb]}}p), %xmm1 # 8-byte Reload
+; CHECK-NEXT:    # xmm1 = mem[0],zero
 ; CHECK-NEXT:    callq fma@PLT
-; CHECK-NEXT:    popq %rax
+; CHECK-NEXT:    addq $24, %rsp
 ; CHECK-NEXT:    retq
     %2 = call i32 @fesetround(i32 noundef 1024)
     %3 = call double @llvm.experimental.constrained.fdiv.f64(double 1.000000e+00, double %0, metadata !"round.dynamic", metadata !"fpexcept.ignore") #0
     %4 = call i32 @fesetround(i32 noundef 1024)
     %5 = call double @llvm.experimental.constrained.fdiv.f64(double 1.000000e+00, double %0, metadata !"round.dynamic", metadata !"fpexcept.ignore") #0
     %6 = call i32 @fesetround(i32 noundef 1024)
     %7 = call double @llvm.experimental.constrained.fdiv.f64(double 1.000000e+00, double %0, metadata !"round.dynamic", metadata !"fpexcept.ignore") #0
     %8 = call double @llvm.experimental.constrained.fma.f64(double %3, double %5, double %7, metadata !"round.dynamic", metadata !"fpexcept.ignore") #0
     ret double %8
 }
 
 declare i32 @fesetround(i32) #0
 declare double @llvm.experimental.constrained.fdiv.f64(double, double, metadata, metadata) #0
 declare double @llvm.experimental.constrained.fma.f64(double, double, double, metadata, metadata) #0
 
 attributes #0 = { nounwind strictfp }