[PATCH] D158053: [Legalizer] Expand fmaximum and fminimum

[Thomas Lively via Phabricator via llvm-commits]

tlively added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:8161
+        *DAG.getContext(), APFloat::getNaN(DAG.EVTToAPFloatSemantics(VT)));
+    MinMax = DAG.getSelect(DL, VT, DAG.getSetCC(DL, CCVT, LHS, RHS, ISD::SETUO),
+                           DAG.getConstantFP(*FPNaN, DL, VT), MinMax);
----------------
tra wrote:
> qiucf wrote:
> > tra wrote:
> > > Are we expected to return *a* NaN or the specific NaN value of one of the arguments? 
> > > APfloat's implementation returns one of the argument values.
> > > X86ISelLowering.cpp does the same.
> > > 
> > > 
> > Does the standard specify if the exact NaN should be returned? Choosing one NaN will produce more complex code. 
> I don't have access to the standard text. All I could find online are these notes https://grouper.ieee.org/groups/msc/ANSI_IEEE-Std-754-2019/background/minNum_maxNum_Removal_Demotion_v3.pdf oulining the state of the min/max and the recommeendations for the standard changes. They do seem to care about NaN associativity/commutativity, but I have no idea what exactly ended up specified by the standard.
> 
> I assume that the authors of the implementations in APFloat and X86 did implement it according to the standard.
> 
> @tlively -- would you happen to know the details?
Looked into it, and yes, the standard says (in section 6.2) that the output NaN should have the same payload as one of the input NaNs, explicitly not specifying which input NaN the payload is from if there are multiple. Furthermore, the output NaN must be canonical even if the input is a non-canonical NaN.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158053