Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 4 Oct 2022 16:02:05 +0100
From:      Alexander Richardson <arichardson@freebsd.org>
To:        Brooks Davis <brooks@freebsd.org>
Cc:        freebsd-arch@freebsd.org
Subject:   Re: enabling float128 support in clang?
Message-ID:  <CA%2BZ_v8qVQhUpnB2aca0Wkuxnjz32MGTdbzhP4NdAY6-fnfCiCw@mail.gmail.com>
In-Reply-To: <20221003184310.GA20951@spindle.one-eyed-alien.net>
References:  <20221003184310.GA20951@spindle.one-eyed-alien.net>

next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 3 Oct 2022 at 19:43, Brooks Davis <brooks@freebsd.org> wrote:
>
> TL;DR: Is there a reason not to enable float128 support in clang for
> x86?
>
> Due to a build breakage with the flang runtime, I recently noticed that
> unlike most other OSes we don't support float128 in clang.  It's enable
> in modern GCC and other OSes have it enable in clang:
>  - Hakiu 2018: https://reviews.llvm.org/D54901
>  - Solaris 2018: https://reviews.llvm.org/D41240
>  - WASM 2019: https://reviews.llvm.org/D57154
>  - Dragonfly 2021: https://reviews.llvm.org/D111760
>  - OpenBSD (history hard to find...)
>
> Is there a known reason not to enable this?
>
> Patch to enable below (I'm a bit skeptical of the __FLOAT128__ part as
> GCC doesn't define it...)
>
> -- Brooks
>
> diff --git a/clang/lib/Basic/Targets/OSTargets.h b/clang/lib/Basic/Targets/OSTargets.h
> index c75f7d9fbafe..ea95f40e81a0 100644
> --- a/clang/lib/Basic/Targets/OSTargets.h
> +++ b/clang/lib/Basic/Targets/OSTargets.h
> @@ -232,6 +232,9 @@ protected:
>      // setting this to 1 is conforming even if all the basic source
>      // character literals have the same encoding as char and wchar_t.
>      Builder.defineMacro("__STDC_MB_MIGHT_NEQ_WC__", "1");
> +
> +    if (this->HasFloat128)
> +      Builder.defineMacro("__FLOAT128__");
>    }
>
>  public:
> @@ -241,6 +244,7 @@ public:
>      default:
>      case llvm::Triple::x86:
>      case llvm::Triple::x86_64:
> +      this->HasFloat128 = true;
>        this->MCountName = ".mcount";
>        break;
>      case llvm::Triple::mips:
>

I think this makes sense and may even motivate me to try and get the
necessary library calls merged to compiler-rt
(https://reviews.llvm.org/D98261).
One thing to note is that your current patch will also enable float128
for AArch64 (and other targets), which I don't believe any other OS
does.
I think it makes sense to restrict it to just x86 to avoid executing
code in the compiler that has not been well tested. I believe for all
other supported architectures long double is the same as float128 so
it probably doesn't make too much sense to enable it there anyway.

Alex



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CA%2BZ_v8qVQhUpnB2aca0Wkuxnjz32MGTdbzhP4NdAY6-fnfCiCw>