[PATCH] D102995: errorUnsupported should be non-fatal

[MJ via Phabricator via llvm-commits]

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