errorUnsupported should be non-fatal

[majiang31312]

Summary

DiagnosticInfoUnsupported use DS_Error as the default severity, but we want errorUnsupported to be non-fatal.
So we should pass DS_Warning explicitly in errorUnsupported.

before fix, the following code could not be built with ‘clang -mno-sse’

double test() {return 0.0;}
int main()
{
  double a = test();
  return 0;
}

[majiang31312]

update related tests

[lebedev.ri]

*Why* do we want that?

[majiang31312]

In D102995#2776986, @lebedev.ri wrote:

*Why* do we want that?

I am compiling skia with -mno-sse , because I want to use it in graalvm(sulong) which does not support float SSE instructions for the moment.
And, I think the comment before errorUnsupported has clearly state the reason.

[majiang31312]

CI failed to patch? Retry with Repository 'rG LLVM Github Monorepo'

[majiang31312]

Try Arc

[craig.topper]

"fatal" in the comment means don't diagnose any additional errors and immediately stop. We attempt to recover to detect more errors, but the emitted binary code is likely incorrect. I don't think we can just emit a warning.

The function name says "error" in its name, it should be an error.

[majiang31312]

In D102995#2778674, @craig.topper wrote:

"fatal" in the comment means don't diagnose any additional errors and immediately stop. We attempt to recover to detect more errors, but the emitted binary code is likely incorrect. I don't think we can just emit a warning.

The function name says "error" in its name, it should be an error.

Thanks for the explanations, it's reasonable now.
Although “SSE register return with SSE disabled” seems strange as the code did not use SSE directly, it's another problem. I'll close this one now.

[craig.topper]

X86-64 ABI requires float/double arguments to use XMM registers. All x86-64 CPUs have SSE2.

[majiang31312]

Thanks for the information.
Well, the key problem is X86-64 ABI indeed. But we could do better here for sure.
First of all, we could 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!

clang/test/CodeGen/x86_64-mno-sse.c

 // RUN: %clang_cc1 -triple x86_64-linux -target-feature -sse -target-feature -sse2 -S -o /dev/null -verify %s
 // REQUIRES: x86-registered-target
 
-double f1(void) { // expected-error {{SSE register return with SSE disabled}}
+double f1(void) { // expected-warning {{SSE register return with SSE disabled}}
   return 1.4;
 }
 extern double g;
-void f2(void) { // expected-error {{SSE register return with SSE disabled}}
+void f2(void) { // expected-warning {{SSE register return with SSE disabled}}
   g = f1();
 }
 void take_double(double);
 void pass_double(void) {
-  // FIXME: Still asserts.
-  //take_double(1.5);
+  take_double(1.5);
 }

clang/test/CodeGen/x86_64-mno-sse2.c

 // RUN: %clang_cc1 -triple x86_64-linux -target-feature -sse2 -S -o /dev/null -verify %s
 // REQUIRES: x86-registered-target
 
-double f1(void) { // expected-error {{SSE2 register return with SSE2 disabled}}
+double f1(void) { // expected-warning {{SSE2 register return with SSE2 disabled}}
   return 1.4;
 }
 extern double g;
-void f2(void) { // expected-error {{SSE2 register return with SSE2 disabled}}
+void f2(void) { // expected-warning {{SSE2 register return with SSE2 disabled}}
   g = f1();
 }
 void take_double(double);
 void pass_double(void) {
   // FIXME: Still asserts.
   //take_double(1.5);
 }
 
 double return_double();
-void call_double(double *a) { // expected-error {{SSE2 register return with SSE2 disabled}}
+void call_double(double *a) { // expected-warning {{SSE2 register return with SSE2 disabled}}
   *a = return_double();
 }

llvm/lib/Target/X86/X86ISelLowering.cpp

 
 /// Call this when the user attempts to do something unsupported, like
 /// returning a double without SSE2 enabled on x86_64. This is not fatal, unlike
 /// report_fatal_error, so calling code should attempt to recover without
 /// crashing.
 static void errorUnsupported(SelectionDAG &DAG, const SDLoc &dl,
                              const char *Msg) {
   MachineFunction &MF = DAG.getMachineFunction();
-  DAG.getContext()->diagnose(
-      DiagnosticInfoUnsupported(MF.getFunction(), Msg, dl.getDebugLoc()));
+  DAG.getContext()->diagnose(DiagnosticInfoUnsupported(
+      MF.getFunction(), Msg, dl.getDebugLoc(), DS_Warning));
 }
 
 X86TargetLowering::X86TargetLowering(const X86TargetMachine &TM,
                                      const X86Subtarget &STI)
     : TargetLowering(TM), Subtarget(STI) {
   bool UseX87 = !Subtarget.useSoftFloat() && Subtarget.hasX87();
   X86ScalarSSEf64 = Subtarget.hasSSE2();
   X86ScalarSSEf32 = Subtarget.hasSSE1();