[PATCH] D102995: errorUnsupported should be non-fatal
MJ via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed May 26 02:06:13 PDT 2021
majiang31312 added a comment.
Thanks for the information.
Well, the key problem is x64 abi. But we can do better here for sure.
First of all, we can fallback to use fpu instructions. I can get the following binary which should work correctly(after this fix, of course)
0000000000000000 <test>:
0: 55 push %rbp
1: 48 89 e5 mov %rsp,%rbp
4: d9 ee fldz
6: 5d pop %rbp
7: c3 retq
8: 0f 1f 84 00 00 00 00 nopl 0x0(%rax,%rax,1)
f: 00
0000000000000010 <main>:
10: 55 push %rbp
11: 48 89 e5 mov %rsp,%rbp
14: 48 83 ec 20 sub $0x20,%rsp
18: 31 c0 xor %eax,%eax
1a: c7 45 fc 00 00 00 00 movl $0x0,-0x4(%rbp)
21: 89 45 ec mov %eax,-0x14(%rbp)
24: e8 00 00 00 00 callq 29 <main+0x19>
25: R_X86_64_PLT32 test-0x4
29: dd 5d f0 fstpl -0x10(%rbp)
Although it might be incompatible with other libraries because of ABI problems, it is the responsibility of the linker to prevent possible errors. The compiler should just produce the binary as requested.
There have been similar situations in https://reviews.llvm.org/D70465.
Second, even if we decided not to support the fallback feature for the moment, we certainly could give a more clear message (maybe something like "float/double arguments under -mno-sse are not supported yet").
In all, using float/double is not a error in code(even with -mno-sse) in my opinion.
However, to make -mno-sse(with float/double) working gracefully need quite a work, so I agree it's reasonable to keep current state as there are no serious users yet. Thanks again for the clarification!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D102995/new/
https://reviews.llvm.org/D102995
More information about the cfe-commits
mailing list