thopre added a comment.

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!


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

Reply via email to