[PATCH] D15600: AArch64: Add option to use shared epilogues in compiler-rt

[Renato Golin via llvm-commits]

On 12 January 2016 at 19:26, Matthias Braun <matze at braunis.de> wrote:
> I looked into producing comdat functions and unfortunately I am not sure we can easily do this at the moment. All the codegen passes and the usual CodeGen/Passes.cpp pipeline is built from (Machine)FunctionPasses which are not allowed to create additional functions. I don't see an easy way out there yet.

Maybe a higher level IR pass could add them unconditionally and then
the back-end just puts the right calls in place?


> As for keeping the epilogues in compiler-rt: I do not see how this case is any worse than anything else we have in compiler-rt; If you link with an incompatible/older version there is always a chance that things won't work, this should be the same for epilogues as for example soft-float intrinsics.

Well, most soft-float functions are well known and have the same
meaning, even if not the same implementation, on all compatible
libraries. The extra functionality (sanitizers, some kludge) are
specific to cases where the behaviour is *explicitly* invoked. In
those cases, an error message on the linker complaining about missing
symbols would be easily solved.

You're proposing that we have to rely on a specific version of
Compiler-RT for *all* cases. That's a whole lot different.


> To avoid people accidentally changing the epilogue function I decidedly choose names that pretty much completely describe the content of the epilogue function: __epilogue_X19_X20_X21_X22 does what you would expect it to do: restore X19,X10,X21 and X22 in that order and return, I don't see how anyone would change the content of that function without also choosing a different function name.

True, it's only if you call it "epilogue" and change the
implementation that you'll have silent bugs. But when the name is a
perfect match to the implementation, you'll end up with too many
symbols, and will get undefined symbols if they change names, where
the user wasn't expecting (because that behaviour is default, not
intentional).

You're basically introducing library dependency and unexpected
toolchain behaviour for the sake of 1.7% size reduction in a
test-suite. The costs far outweigh the benefits IMO.

cheers,
--renato