This is an archive of the discontinued LLVM Phabricator instance.

[GlobalISel] Implement lowering for G_ISNAN + use it in AArch64
ClosedPublic

Authored by paquette on Aug 17 2021, 11:11 AM.

Details

Summary

GlobalISel equivalent to TargetLowering::expandISNAN.

Use it in AArch64 and add a testcase.

Diff Detail

Event Timeline

paquette created this revision.Aug 17 2021, 11:11 AM
paquette requested review of this revision.Aug 17 2021, 11:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 17 2021, 11:11 AM
Herald added a subscriber: wdng. · View Herald Transcript
aemerson accepted this revision.Aug 17 2021, 11:46 PM
This revision is now accepted and ready to land.Aug 17 2021, 11:46 PM
This revision was landed with ongoing or failed builds.Aug 18 2021, 10:54 AM
This revision was automatically updated to reflect the committed changes.
simon_tatham added inline comments.
llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
7378

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.

paquette added inline comments.Aug 31 2021, 9:41 AM
llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
7378

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.

Yes, that seems to have fixed it for me – thanks for the quick response!