On Thu, 22 Jun 2023 at 15:35, Daniel P. Berrangé <[email protected]> wrote: > > On Thu, Jun 22, 2023 at 02:08:23PM +0100, Peter Maydell wrote: > > We use __builtin_subcll() to do a 64-bit subtract with borrow-in and > > borrow-out when the host compiler supports it. Unfortunately some > > versions of Apple Clang have a bug in their implementation of this > > intrinsic which means it returns the wrong value. The effect is that > > a QEMU built with the affected compiler will hang when emulating x86 > > float80 division. > > > > The upstream LLVM issue is: > > https://github.com/llvm/llvm-project/issues/55253 > > > > The commit that introduced the bug apparently never made it into an > > upstream LLVM release without the subsequent fix > > https://github.com/llvm/llvm-project/commit/fffb6e6afdbaba563189c1f715058ed401fbc88d > > but unfortunately it did make it into Apple Clang 14.0, as shipped > > in Xcode 14.3 (14.2 is reported to be OK). The Apple bug number is > > FB12210478. > > > > Add ifdefs to avoid use of __builtin_subcll() on Apple Clang version > > 14 or greater. There is not currently a version of Apple Clang which > > has the bug fix -- when one appears we should be able to add an upper > > bound to the ifdef condition so we can start using the builtin again. > > We make the lower bound a conservative "any Apple clang with major > > version 14 or greater" because the consequences of incorrectly > > disabling the builtin when it would work are pretty small and the > > consequences of not disabling it when we should are pretty bad. > > > > Many thanks to those users who both reported this bug and also > > did a lot of work in identifying the root cause; in particular > > to Daniel Bertalan and osy. > > > > Cc: [email protected] > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1631 > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1659 > > Signed-off-by: Peter Maydell <[email protected]> > > --- > > I don't have a setup to test this, so this needs testing by the > > people who've encountered this compiler bug to confirm it does > > the right thing... > > --- > > include/qemu/compiler.h | 13 +++++++++++++ > > include/qemu/host-utils.h | 2 +- > > 2 files changed, 14 insertions(+), 1 deletion(-) > > > > diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h > > index c2f49df1f91..a309f90c768 100644 > > --- a/include/qemu/compiler.h > > +++ b/include/qemu/compiler.h > > @@ -184,4 +184,17 @@ > > #define QEMU_DISABLE_CFI > > #endif > > > > +/* > > + * Apple clang version 14 has a bug in its __builtin_subcll(); define > > + * BUILTIN_SUBCLL_BROKEN for the offending versions so we can avoid it. > > + * When a version of Apple clang which has this bug fixed is released > > + * we can add an upper bound to this check. > > + * See https://gitlab.com/qemu-project/qemu/-/issues/1631 > > + * and https://gitlab.com/qemu-project/qemu/-/issues/1659 for details. > > + * The bug never made it into any upstream LLVM releases, only Apple ones. > > Perhaps add a reminder: > > * TODO: put a max cap on __clang_major__/__clang_minor once > * Apple have released a version with the fix
That was what I intended the "When..." sentence to be. We can add "TODO:" on the front to make it stand out a bit more I guess. -- PMM
