thopre added a comment. In D96568#2567145 <https://reviews.llvm.org/D96568#2567145>, @jonpa wrote:
> 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? Sounds good to me. Hopefully I'll get round to __builtin_isinf soon and a single hook will make the patch slightly smaller. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96568/new/ https://reviews.llvm.org/D96568 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
