[PATCH] D96568: [CFE, SystemZ] Emit s390.tdc instrincic for __builtin_isnan in Constrained FP mode.

[Jonas Paulsson via Phabricator via cfe-commits]

jonpa added a comment.

In D96568#2559476 <https://reviews.llvm.org/D96568#2559476>, @thopre wrote:

> In D96568#2559475 <https://reviews.llvm.org/D96568#2559475>, @uweigand wrote:
>
>> In D96568#2559336 <https://reviews.llvm.org/D96568#2559336>, @thopre wrote:
>>
>>> That's interesting. I presume that can be used to implement isinf as well? Perhaps better call the hook fpclassify or similar?
>>
>> Hmm, the instruction doesn't really implement fpclassify in itself, it is more like a combined check for fpclassify() == <some constant>.   Specifically, the TEST DATA CLASS instruction takes an immediate operand that represents a bit mask, which each bit standing for one type of floating-point value (zero, normal, subnormal, infinity, QNaN, SNaN -- each in positive and negative versions).  The instruction sets the condition code depending on whether the input FP number is in one of the classes selected by the bit mask, or not.
>>
>> This is why Jonas' patch uses a bit mask of 0x0F -- this has the bits for the four types of NaN set (pos/neg QNaN/SNan).   The instruction could indeed also be used to implement an isinf check (bit mask 0x30) or many other checks.   We actually have a SystemZ back-end pass that tries to multiple combine FP checks into a single TEST DATA CLASS instruction.
>>
>> However, the instruction does not directly implement the fpclassify semantics.  To implement fpclassify, you'd still have to use multiple invocations of the instruction with different flags to determine the fpclassify output value.
>
> I see. I'm not sure whether it's better to have several target hooks or a single one like testFPKind that would take a flag saying what do we want to test (NaN, Inf, etc.).

Personally I think testFPKind should work well - it gives a single target hook for related purposes which should be more readable, and it is also natural for SystemZ since we will be using the same (Test Data Class) instruction for differnet checks only with different flag bits... Any one else has an opinion? Should I go ahead and change the patch to this end?


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

https://reviews.llvm.org/D96568