[PATCH] D108227: [GlobalISel] Implement lowering for G_ISNAN + use it in AArch64

[Jessica Paquette via Phabricator via llvm-commits]

paquette added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:7378
+  // isnan(V) == exp mask < abs(V)
+  auto FPToSI = MIRBuilder.buildFPTOSI(SrcTy, Src);
+  auto Mask = APInt::getSignedMaxValue(SrcTy.getScalarSizeInBits());
----------------
simon_tatham wrote:
> Sorry to be late spotting this, but I think this `fptosi` can't be right. 
> 
> Surely what we need here is the integer that represents the //encoding// of the FP number, with sign, exponent and mantissa fields?
> 
> But this is computing the integer that represents its //numerical value//, as if from a source-level float→int conversion.
> 
> In the AArch64 output code I'm seeing, this `fptosi` is coming out as an `FCVTZS w8,s0` instruction, and with a sensible single-precision NaN in s0 (encoding 0x7fc00000), that instruction is generating zero in w8, which leads to isnan(NaN) returning false.
> 
> I don't speak good MIR, but surely something like a bitcast would be the right operation.
Maybe it could be just done with a copy? We can't use `G_BITCAST` because GlobalISel doesn't distinguish between floating point and integer types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108227