[PATCH] D62017: [CodeGen] Add lrint/llrint builtins

Tue Jul 28 19:42:05 PDT 2020

craig.topper added a comment.

In D62017#2180856 <https://reviews.llvm.org/D62017#2180856>, @arsenm wrote:

> In D62017#2180852 <https://reviews.llvm.org/D62017#2180852>, @craig.topper wrote:
>
>> In D62017#2180847 <https://reviews.llvm.org/D62017#2180847>, @arsenm wrote:
>>
>>> What is the point of these intrinsics exactly? It seems like it's just copying the libm function names, despite them only differing in the return integer type. Why can't this be accomplished with adding type mangling to the regular rint intrinsic?
>>
>> I think the primary complication is that we need to know what libcall to emit if the target doesn't natively support them. The backend doesn't know how to map from IR type "long" or "long long" to generate the right libcall.
>
> But does that matter? What does the right library call really mean? I don't think the purpose of the math intrinsics is to wrap libm. Intrinsics should provide functionality independently of whatever the platform libm does. The backend is responsible for figuring out how to codegen any intrinsics which may or may not involve a platform provided library call.

I'm not sure they should have been intrinsics versus just matching LibFunc_lrint and LibFunc_llrint in SelectionDAGBuilder. Most targets are using the default Expand operation in LegalizeDAG so that still needs the ISD node to carry the function name to reverse back to library call name. The mapping from IR type to long vs long long is target and OS dependent. Unless we make SelectionDAGBuilder only use an ISD node if the target has the operation as Legal/Custom.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62017