[PATCH] D111433: [PowerPC] Respect rounding mode in the back end

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai added a comment.

In D111433#3061400 <https://reviews.llvm.org/D111433#3061400>, @qiucf wrote:

> Thanks for fixing this!
>
> I guess the reason to restrict them only undef strict-fp is performance?

Right, preventing optimization around calls due to rounding mode without `-frounding-math` would be unnecessarily restrictive for most compilations.

> Since FPSCR is not modeled yet, if so, we may need to re-define all float instructions like this patch when implementing FPSCR model?

There are only a handful of instructions that actually set the rounding mode (or other fields in the FPSCR) and they should be defined as such. The instructions that just use the rounding mode already have it as an implicit use. While there may be other, more esoteric interactions with the FPSCR, I believe that handling the rounding mode and exceptions correctly should suffice for a vast majority of uses. And I believe that we are pretty close to all of that working correctly.



================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:5228
+    RetOpc = PPCISD::CALL;
+  if (IsStrictFPCall) {
+    switch (RetOpc) {
----------------
qiucf wrote:
> Better to move this into a static function?
I don't have a particularly strong opinion on this since we are already in a static function and the code would read exactly the same. I already separated it into code that computes the node to use and then code that updates the opcode for strictfp, so it can very easily be two separate functions.

If you have a preference for moving it out, please let me know and I'll update it.


================
Comment at: llvm/lib/Target/PowerPC/PPCInstr64Bit.td:181
 
+let isCall = 1, PPC970_Unit = 7, Defs = [LR8, RM], hasSideEffects = 0,
+    isCodeGenOnly = 1, Uses = [RM] in {
----------------
qiucf wrote:
> Can they be implemented through `multiclass/defm`?
Absolutely, but I don't think we'd make it any more readable or concise since we'd need 4 new multiclasses for:
```
IForm
IForm_and_DForm_4_zero
XLForm_2_ext
XLForm_2_ext_and_DSForm_1
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111433