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

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

I worry that we're chasing after the implementation details of GCC here. It seems self-contradictory to say all three of these things at once:

1. The frontend function `foo` has known, builtin semantics X. (Eg, we'll use those semantics in the frontend.)
2. The symbol `foo` has known, builtin semantics X. (Eg, we'll synthesize calls to `foo` from the middle-end.)
3. It's not correct to lower a call to the frontend function `foo` to the symbol `foo`.

So, from a principled standpoint, which of the above do we not consider to be correct in this situation?

- If we don't consider (1) to be correct, then we need to stop treating `foo` as a builtin everywhere -- we shouldn't be constant-evaluating calls to it, in particular. That's problematic in principle, because the `asm` label might be added after we've already seen the first declaration and added a `BuiltinAttr` to it. If we want to go in this direction, I think we should require a build that attaches an `asm` label to a builtin to also pass `-fno-builtin-foo` rather than trying to un-builtin a builtin function after the fact.
- If we don't consider (2) to be correct, then we need to stop the middle-end from inserting calls to the symbol `foo`, and get it to use the new symbol instead. That'd be the "teach the target libcall info about this" approach, which (as you point out) would be quite complex.
- The status quo is that we don't consider (3) to be correct. If we take this path, I suppose we could say that we make a best effort to use the renamed symbol, but provide no guarantees. That seems unprincipled and will likely continue to cause problems down the line, but maybe this gets us closest to the observed GCC behavior?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88712