[PATCH] D64662: [FPEnv] [PowerPC] Lower ppc_fp128 StrictFP Nodes to libcalls

[Kit Barton via Phabricator via llvm-commits]

kbarton accepted this revision.
kbarton marked 2 inline comments as done.
kbarton added a comment.
This revision is now accepted and ready to land.

LGTM.



================
Comment at: lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp:1140
+  case ISD::FMINNUM:            ExpandFloatRes_FMINNUM(N, Lo, Hi); break;
+  case ISD::STRICT_FMINNUM:     ExpandFloatRes_STRICT_FMINNUM(N, Lo, Hi); break;
+  case ISD::FMAXNUM:            ExpandFloatRes_FMAXNUM(N, Lo, Hi); break;
----------------
ajwock wrote:
> kbarton wrote:
> > Similar comment here with the choice of _STRICT_ vs Strict_ (i.e., ExpandFloatResStrict_FMINNUM(...)).
> > I think the use of Strict_ follows the coding guidelines, while _STRICT_ deviates from it. 
> So, each of these functions capitalized names corresponds to an instruction opcode.  In order to stay consistent with the rest of these functions, I have simply named these functions after their opcode rather than arbitrarily prepending _STRICT.
> 
> I believe that if I did not name these functions as such, it would either be inconsistent with existent code or require significant refactoring.
Yes, I see now. 
I think this is OK. And it's consistent with the way it's done in SystemZ too. 



================
Comment at: lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp:1242
+                                              SDValue &Hi) {
+  SDValue Call = LibCallify_StrictFP(GetFPLibCall(N->getValueType(0),
+                                         RTLIB::FMIN_F32, RTLIB::FMIN_F64,
----------------
ajwock wrote:
> kbarton wrote:
> > Was this formatted with clang-format? 
> Fixed?  Reformatted with clang-format.
Yes, this looks better now (although I honestly don't remeber what it looked like before :( ). 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64662