[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
Tue Jun 22 15:58:42 PDT 2021
jonpa added a comment.
In D96568#2832781 <https://reviews.llvm.org/D96568#2832781>, @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...).
>
> I forgot to ask at the time, why do you restrict this code to the constrained case? Presumably it's a lot faster than fabs+cmp and should be faster in all cases?
There are later optimizations that does this already in place (SystemZTDCPass). I checked now and it seems to make no difference at all to do this in the front-end always in non-constrained FP mode... (SPEC)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D96568/new/
https://reviews.llvm.org/D96568
More information about the cfe-commits
mailing list