[PATCH] D120327: compiler-rt: Add udivmodei5 to builtins and add bitint library

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: compiler-rt/lib/builtins/udivmodei5.c:19
+#ifdef IS_BITINT_LIBRARY
+#define WEAK_IF_BITINT_LIBRARY __attribute__((weak))
+#else
----------------
mgehre-amd wrote:
> MaskRay wrote:
> > Can you elaborate how `WEAK_IF_BITINT_LIBRARY` is intended to be used with libgcc, libgcc.a or libgcc_s.so.1? If libgcc_s.so.1 => STB_WEAK does not affect symbol resolution and is not different from STB_GLOBAL.
> > 
> The idea is that clang unconditionally links to libbitint, which contains __udivmodei5 (etc) with weak linkage.
> 
> In addition, clang either links to libgcc or compiler-rt builtins, depending on the user's choice.
> 
> If the user selected an "older" libgcc.a (i.e. without __udivmodei5), then __udivmodei5 from libbitint is used by the linker.
> If the user selected a "newer" libgcc.a (i.e. with __udivmodei5), then __udivmodei5 from libgcc is used by the linker. (There are no patches yet to add __udivmodei5 to libgcc).
> If the user links against "newer" libgcc_s.so.1, I think the __udivmodei5  from libbitint is used by the linker, but at least there is no linker error.
> If the user selected compiler-rt (i.e. with __udivmodei5), then __udivmodei5 from compiler-rt is used by the linker.
> 
> After libgcc has adopted __udivmodei5 for a while, we can phase out libbitint.
So libbitint is linked even in --rtlib=libgcc mode? That may be a bit weird.

I am not sure the proposed libgcc.a member shadowing scheme will work.
See https://maskray.me/blog/2021-06-20-symbol-processing#archive-processing

Say these functions are provided in libgcc.a(ei5.o). If after libbitint is processed, all symbols defined by libgcc.a(ei5.o) are either defined or unreferenced, libgcc.a(ei5.o) won't be extracted, even if it provides STB_GLOBAL definitions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120327