[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
Thu Oct 15 15:03:53 PDT 2020


MaskRay added a comment.

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

> If the answer is that we consider (2) to be incorrect, but that this is only a partial step in that direction, I think that's fine -- that's enough that we can know where the bug is if / when people complain that we still generate calls to `memcpy` even when we used an `asm` label to indicate the library symbol was renamed to something else. In that light, this is really a workaround rather than a fix: we can't easily stop the middle-end from synthesizing calls to library functions with the wrong symbol, because we don't have a good way to tell it those library functions were renamed, but we can at least reduce the rate of incidence by avoiding generating the intrinsic calls that are likely to get lowered to library functions.
>
> Given that understanding, should this:
>
>   double sin(double x) asm("real_sin");
>   double f(double d) { return __builtin_sin(d); }
>
> ... call the `sin` symbol or the `real_sin` symbol? I think with the current patch it'll call the `sin` symbol, but calling `real_sin` seems correct.
>
> In any case, assuming the above understanding is right, this looks like a step in the right direction.

Thanks for the review! For this example, GCC emits `real_sin` but we fail to do so. Added it to the test.


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