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

Jinsong Ji via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 3 14:32:49 PDT 2019


jsji added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp:1577
 
+void DAGTypeLegalizer::ExpandFloatRes_STRICT_FP_EXTEND(SDNode *N, SDValue &Lo,
+                                                SDValue &Hi) {
----------------
ajwock wrote:
> jsji wrote:
> > Add comments about why this is special here, not lowering to calls. I know you have description in summary,
> > but it would be great to add comment here, so that readers don't have to look into commit message or patches.
> I didn't find it necessary to add such comments- libcallification is one of the possible ways one might expand a floating point result, but it isn't something that is implied.
> 
> Another reason I didn't do this is that the author of the analog ExpandFloatRes_FP_EXTEND (not strict and right above this) didn't comment on this, and when I came across this code in my problem solving, I could reasonably intuit why FP_EXTEND wouldn't be a libcall.
> 
> I could add documentation that explains this issue with respect to both this function and the above function, but wouldn't that be more appropriate for a ticket specifically about existing documentation?
Fair enough, maybe a NFC patch for documenting these in the future. Thanks.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64662





More information about the llvm-commits mailing list