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

[Matthias Gehre via Phabricator via llvm-commits]

mgehre-amd added inline comments.


================
Comment at: compiler-rt/lib/builtins/udivmodei5.c:19
+#ifdef IS_BITINT_LIBRARY
+#define WEAK_IF_BITINT_LIBRARY __attribute__((weak))
+#else
----------------
MaskRay wrote:
> 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.
> So libbitint is linked even in --rtlib=libgcc mode? That may be a bit weird.
This is exactly the use case for libbitint: To allow users of libgcc (which is the default on clang on Linux) to use _BitInt while libgcc / distributions have not catched up with __divmodei5 and similar.

> libgcc.a(ei5.o) won't be extracted, even if it provides STB_GLOBAL definitions.
My main driver for making the symbols "weak" in libbitint is to avoid multiple definitions errors once libgcc also provides __divmodei5. You are saying that
even with normal linkage in libbitint, no multiple definition error will occur? Then I can remove the weak linkage from libbitint.

Alternatively, I can also completely remove libbitint from this PR. Then users can only use `_BitInt` division in clang when they either use compiler-rt or a future version of libgcc.
What do you think?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120327