Re: enabling float128 support in clang?

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