Page MenuHomePhabricator

[CodeGen] Add lrint/llrint builtins
ClosedPublic

Authored by zatrazz on May 16 2019, 11:24 AM.

Details

Summary

This patch add the ISD::LRINT and ISD::LLRINT along with new
intrinsics. The changes are straightforward as for other
floating-point rounding functions, with just some adjustments
required to handle the return value being an integer.

The idea is to optimize lrint/llrint generation for AArch64
in a subsequent patch. Current semantic is just route it to libm
symbol.

(This is patch is similar to D61390).

Diff Detail

Repository
rL LLVM

Event Timeline

zatrazz created this revision.May 16 2019, 11:24 AM

Can we do this with 2 intrinsics with overloaded result types as I've done for lround/llround in D62026?

Can we do this with 2 intrinsics with overloaded result types as I've done for lround/llround in D62026?

We can and I will update this patch accordingly.

zatrazz updated this revision to Diff 200289.May 20 2019, 8:12 AM

Updated patch based on D62026.

This revision is now accepted and ready to land.May 20 2019, 9:33 AM
This revision was automatically updated to reflect the committed changes.
arsenm added a subscriber: arsenm.Jul 28 2020, 7:16 PM

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?

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.

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?

Not the regular rint intrinsic, which returns float, but between lrint and llrint

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.

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.

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.

I don't understand why you can't derive the call name from the triple