Page MenuHomePhabricator

Intrinsic for checking floating point class
Needs ReviewPublic

Authored by sepavloff on Oct 18 2021, 11:49 AM.

Details

Summary

This change introduces a new intrinsic, llvm.is.fclass, which checks
if the provided floating point number belongs to the specifies value
class. It implements the checks made by C standard library functions
isnan, isinf, isfinite, isnormal, issubnormal, issignaling
and corresponding IEEE-754 operations.

The primary motivation for this intrinsic is the support of strict FP
mode. In this mode using compare instructions or other FP operations is
not possible, because if the value is a signaling NaN, floating point
exception Invalid is raised, but the aforementioned functions must
never raise exceptions.

Currently there are two solutions for this problem, both are
implemented partially. One of them is using integer operations to
implement the check. It was implemented in https://reviews.llvm.org/D95948
for isnan. It solves the problem of exceptions, but offers one
solution for all targets, although some can do the check in more
efficient way.

The other, implemented in https://reviews.llvm.org/D96568, introduced a
hook 'clang::TargetCodeGenInfo::testFPKind', which injects a target
specific code into IR to implement isnan. It is convenient for
targets that has dedicated instruction to determine FP data class.
However using target-specific intrinsic complicates analysis and can
prevent some optimizations.

Using a special intrinsic to represent a value class check allows
representing data class test with enough flexibility. During IR
transformations it represents the check in target-independent way and
prevents it from undesired transformations. In the instruction selector
it allows efficient lowering depending on the target and used mode.

This implementation is an extended variant of llvm.isnan introduced
in https://reviews.llvm.org/D104854. It is limited to minimal intrinsic
support. Target-specific treatment will be implemented in separate
patches.

Diff Detail

Event Timeline

sepavloff created this revision.Oct 18 2021, 11:49 AM
sepavloff requested review of this revision.Oct 18 2021, 11:49 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 18 2021, 11:49 AM

The patch was tested in runtime using extended version of tests https://reviews.llvm.org/D106804 and temporary clang patch that implements classification builtins by calls to llvm.is.fclass.

arsenm added a subscriber: arsenm.Oct 18 2021, 1:31 PM
arsenm added inline comments.
llvm/docs/LangRef.rst
21984–21993

AMDGPU has a class instruction but the fields are a bit more expansive than this. Specifically, it has positive and negative versions of each of the non-nan fields. e.g. you can check specifically +inf separately from -inf, and the same for the rest.

I guess we could identify and (signbit check), (class %x, !foo)

Make test specification more flexible

Now test data class specification is not limited by the test necessary for
implementation of C classification functions, but is made more close to
the facility provided by data class inquiry instruction existing on many
targets.

The intrinsic is renamed to llvm.is_fpclass to use the similar naming as
in fpclassify.

The patch is rebased.

sepavloff added inline comments.Oct 22 2021, 12:02 AM
llvm/docs/LangRef.rst
21984–21993

Updated the patch to better represent targets with such instructions.

sepavloff updated this revision to Diff 383799.Nov 1 2021, 7:17 AM

Updated patch

  • Check for inverted test was moved into a separate function.
  • Fixed code generation for negative zeros and normals check.
  • Fixed code generation for snan checks.
  • Number of codegen tests were reduced.

This patch was tested using a patches for clang https://reviews.llvm.org/D112932 and test-suite https://reviews.llvm.org/D112933. Clang build with the patch were used to compile llvm-test-suite/SingleSource/UnitTests/Float/classify.c. Running the obtained executable executes the unit tests.

arsenm added inline comments.Thu, Nov 11, 6:19 AM
llvm/include/llvm/IR/Intrinsics.td
723

The word "is" here reads weird to me, and you dropped it from the ISD node name. How about just llvm.fpclass? Also the description says "fclass"

sepavloff added inline comments.Thu, Nov 11, 8:17 AM
llvm/include/llvm/IR/Intrinsics.td
723

I agree that this name is a bit awkward. Initially it was llvm.fclass, as you noticed, but then it was changed because:

  • it is inconsistent with C function fpclassify, which also may get a special llvm intrinsic,
  • llvm.fclass might be understood as if the intrinsic calculated the value class. Actually it only tests the value against the specified classes.