[PATCH] D139549: [X86][ConstraintFP] Model `MXCSR` for function call

[Phoebe Wang via Phabricator via llvm-commits]

pengfei added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/TargetLowering.h:4474-4478
+  /// Returns a 0 terminated array of rounding control registers that can be
+  /// attached into strict FP call.
+  virtual const MCPhysReg *getRoundingControlRegisters() const {
+    return nullptr;
+  }
----------------
sepavloff wrote:
> pengfei wrote:
> > sepavloff wrote:
> > > 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.
> > 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.
> 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.
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139549/new/

https://reviews.llvm.org/D139549