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

Jonas Paulsson via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 17 15:49:14 PST 2021


jonpa added a comment.

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

> In D96568#2569296 <https://reviews.llvm.org/D96568#2569296>, @jonpa wrote:
>
>>> Sounds good to me. Hopefully I'll get round to __builtin_isinf soon and a single hook will make the patch slightly smaller.
>>
>> Patch updated to call the new hook testFPKind() and make it take a BuiltinID as argument (that seems to work at least for the moment - maybe an enum type will become necessary at some point per your suggestion..?)
>>
>> I am not sure if this is "only" or "typically" used in constrained FP mode, or if the mode should be independent of calling this hook. The patch as it is asserts that it is called for an FP type but leaves it to the target to decide based on the FP mode, where SystemZ opts out unless it is constrained (which I think is what is wanted...).
>
> LGTM, we can adapt the hook later if needed. I do not know whether allowing the hook to be used for non constrained FP will prove useful but it is easy enough to ignore it for non FP so why not. Thanks for changing that!

Thanks for review!

@uweigand: looks good on the SystemZ parts?


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

https://reviews.llvm.org/D96568



More information about the cfe-commits mailing list