[PATCH] D98895: [X86][clang] Disable long double type for -mno-x87 option

Tue Nov 9 10:26:20 PST 2021

erichkeane added a comment.

In D98895#3119104 <https://reviews.llvm.org/D98895#3119104>, @asavonic wrote:

> In D98895#3119064 <https://reviews.llvm.org/D98895#3119064>, @erichkeane wrote:
>
>> In D98895#3119027 <https://reviews.llvm.org/D98895#3119027>, @asavonic wrote:
>>
>>> In D98895#3118821 <https://reviews.llvm.org/D98895#3118821>, @eandrews wrote:
>>>
>>>> This patch causes a regression.
>>>>
>>>> To reproduce - `clang -cc1 -fsycl-is-device -triple spir64 test.cpp`
>>>>
>>>>   test.cpp:x:3: error: 'bar<__float128>' requires 128 bit size '__float128' type support, but target 'spir64' does not support it
>>>>   T bar() { return T(); };
>>>>     ^
>>>>
>>>> I looked at it briefly, and I believe the issue is call to `checkTypeSupport()` in `ActOnFinishFunctionBody()`. I tried deleting the call but it breaks tests (E.g. L26 in x86_64-no-x87.cpp). @asavonic Please take a look. I will be reverting the patch if this cannot be fixed soon.
>>>
>>> The diagnostic seems to be correct - this instance of `bar` returns an unsupported type. Why do you think it should not be diagnosed?
>>
>> I believe the problem is that there are now _3_ different diagnostics for the same thing, the one on 'bar', plus 2 more here:
>>
>> `auto malAutoTemp5 = bar<__float128>();`
>>
>> I think i would expect 1 error on 'bar', 1 error on the deduced 'auto', but the 3rd is superfluous.
>
> I will check if there is a way to filter it out. However, I don't think that it is a good reason to revert the patch.

This a pretty significant regression in our downstream, which is typically more than enough to get a revert.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98895