[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

[Sanjay Patel via Phabricator via cfe-commits]

spatel added a comment.

In D88712#2413877 <https://reviews.llvm.org/D88712#2413877>, @venkataramanan.kumar.llvm wrote:

> In D88712#2413688 <https://reviews.llvm.org/D88712#2413688>, @spatel wrote:
>
>> In D88712#2412874 <https://reviews.llvm.org/D88712#2412874>, @venkataramanan.kumar.llvm wrote:
>>
>>> In D88712#2412366 <https://reviews.llvm.org/D88712#2412366>, @MaskRay wrote:
>>>
>>>> In D88712#2411841 <https://reviews.llvm.org/D88712#2411841>, @venkataramanan.kumar.llvm wrote:
>>>>
>>>>> 
>>>>
>>>> For your example:
>>>>
>>>>   extern double log (double) asm ("" "log_finite") __attribute__ ((nothrow));
>>>>   
>>>>   double mylog (double d) { return log(d); }
>>>>
>>>> The intention is to emit `log_finite`, no matter the `-ffast-math`. Both GCC and Clang (after this patch) emit `jmp log_finite`.
>>>>
>>>> The previous behavior (according to your comment, with an older glibc) was undesired. So I think the right suggestion is to upgrade glibc.
>>>
>>> Then there optimizations like vectorization, inst combine which works on the LLVM intrinsics.  But they will be not happening now  with  log_finite calls.
>>
>> I'm not sure about the expected semantics/lowering for the finite calls, but can you add something under LibCallSimplifier::optimizeFloatingPointLibCall() that would turn it into the LLVM log intrinsic with appropriate FMF? Codegen would need to be responsible for converting it back to finite call(s) if those are available?
>
> Hi Sanjay I thought codegen no longer lowers them back to finite calls 
> https://reviews.llvm.org/rGcd0926d087a85c5ee1222ca80980b4440214a822

There was a comment in D74712 <https://reviews.llvm.org/D74712> that we might be moving too fast. If you would like to revert/adjust that patch, raise a bug or post a patch to discuss? If the goal is to be able to vectorize the finite calls, then we will need some way to represent those. Alternatively, we could change something in the cost models to enable more unrolling, etc?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88712