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

Fangrui Song via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 12 15:39:34 PDT 2020


MaskRay added a comment.

In D88712#2326223 <https://reviews.llvm.org/D88712#2326223>, @rsmith wrote:

> In D88712#2325823 <https://reviews.llvm.org/D88712#2325823>, @MaskRay wrote:
>
>> In D88712#2324105 <https://reviews.llvm.org/D88712#2324105>, @rsmith wrote:
>>
>>> What are the expected semantics in this case? Is this:
>>>
>>> - the function is still the builtin, but if it ends up being a libcall, call a function with a different asm name, or
>>> - the function is not considered to be the builtin any more, or
>>> - something else?
>>>
>>> I think this patch is approximately the first thing, but it's also cutting off emission for cases where we wouldn't emit a libcall. Should we make that distinction?
>>
>> Yes, we do the first one. I mentioned the limitation in the description. This happens with some functions like `abs` which clang has customized emit without using a library call.
>
> What should happen for a case where the LLVM mid-level optimizers insert a libcall (for example, inventing a call to `memcpy`), and `memcpy` was renamed at the source level to another name? Do we still call the original name?

In that case, I believe GCC does not handle it as well. glibc's approach is to ... let GNU as handle it (D88625 <https://reviews.llvm.org/D88625> ports the behavior to MC).

> Generally I wonder if we should instead be renaming the function in LLVM's `TargetLibraryInfo`, rather than getting the frontend to try to set up a situation where LLVM is unlikely to generate a call to the "wrong" name for the builtin.

Ideally, probably yes. Then clang does asm mangling for no-builtin functions and LLVM does asm manging for builtin functions... There needs to be a new IR level feature. There is complexity in the overloaded builtins...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88712



More information about the cfe-commits mailing list