[PATCH] D78513: [hip] Claim builtin type `__float128` supported if the host target supports it.

Thu Jan 28 22:45:34 PST 2021

jdoerfert added a comment.

In D78513#1993210 <https://reviews.llvm.org/D78513#1993210>, @hliao wrote:

> In D78513#1993115 <https://reviews.llvm.org/D78513#1993115>, @yaxunl wrote:
>
>> Currently if instructions of float128 get to amdgpu backend, are we going to crash?
>
> As `Float128Format` is re-defined as `double`, there won't be any issue in the backend. But, it won't function as the developer expects. That's quite similar to `long double` in clang.

I'd argue that it is quite similarly broken. Silently miscompiling the program is arguably the worst. Are you sure the OpenMP/Sycl way to deal with this is not better? Long story short,
allow the presence of a type that the aux-triple supports but do not allow the use of such a type. The implementation is not perfect yet, e.g., it doesn't allow the above typedef and sometimes a `long double` is manifested in IR, but for the most part it's functional: `clang/test/OpenMP/nvptx_unsupported_type_messages.cpp` and `clang/test/SemaSYCL/float128.cpp` after D74387 <https://reviews.llvm.org/D74387>. TBH, I believe an error is still better than treating `long double` and `fp128` on the device side different and then hope that it somehow works.

FWIW, I landed here after I wrote D95665 <https://reviews.llvm.org/D95665> to address PR48923. I think the ideas there and here are contrary and we should pick one.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78513