[llvm-commits] [llvm] r63495 - in /llvm/trunk: lib/Target/X86/X86CallingConv.td lib/Target/X86/X86ISelLowering.cpp lib/Target/X86/X86Subtarget.cpp test/CodeGen/X86/2009-01-25-NoSSE.ll test/CodeGen/X86/nosse-varargs.ll

[Török Edwin]

On 2009-02-02 22:39, Dan Gohman wrote:
> Hello Edwin,
>   

Hi Dan
> On Feb 1, 2009, at 10:15 AM, Torok Edwin wrote:
>
>   
>> ======================================================================
>> --- llvm/trunk/lib/Target/X86/X86Subtarget.cpp (original)
>> +++ llvm/trunk/lib/Target/X86/X86Subtarget.cpp Sun Feb  1 12:15:56  
>> 2009
>> @@ -331,7 +331,7 @@
>>   // are enabled.  These are available on all x86-64 CPUs.
>>   if (Is64Bit) {
>>     HasX86_64 = true;
>> -#if 1
>> +#if 0
>>      if (X86SSELevel < SSE2)
>>        X86SSELevel = SSE2;
>> #endif
>>     
>
>
> Can you fix the comment immediately above this to reflect
> what the code now does?
>   

See below for patch.

> Also, without this code, does -march=x86-64 default to having
> SSE2 disabled (or dependent on the host)? 

In my tests it defaulted to being enabled, but I think that is due to
CPU features autodetection.
If you supply it with a CPU it should default to SSE2, because there is
something in parsesubtargetfeatures that turns on implied features of a CPU.

I have a testcase that checks that by default llc -march=x86-64
generates SSE2, and with -mattr=-sse it doesn't (sse-novarargs.ll).

> Since all known
> x86-64 hardware supports SSE2, it's convenient to have SSE2
> enabled by default on x86-64.
>   

To be safe [1], we can force the SSE level to be 2 only when
autodetecting features. If user supplied CPU/features we don't force:

[1] if you're cross-compiling from a non-SSE capable x86-32 CPU to x86-64

Is this patch OK to go in?

Index: lib/Target/X86/X86Subtarget.cpp
===================================================================
--- lib/Target/X86/X86Subtarget.cpp     (revision 63542)
+++ lib/Target/X86/X86Subtarget.cpp     (working copy)
@@ -322,13 +322,19 @@
     // If feature string is not empty, parse features string.
     std::string CPU = GetCurrentX86CPU();
     ParseSubtargetFeatures(FS, CPU);
+    // All X86-64 CPUs also have SSE2, however user might request no
SSE via
+    // -mattr, so don't force SSELevel here.
   } else {
     // Otherwise, use CPUID to auto-detect feature set.
     AutoDetectSubtargetFeatures();
+    if (Is64Bit) {
+      // Make sure SSE2  is enabled, it is available on all X86-64 CPUs.
+      X86SSELevel = SSE2;
+    }
   }

-  // If requesting codegen for X86-64, make sure that 64-bit and SSE2
features
-  // are enabled.  These are available on all x86-64 CPUs.
+  // If requesting codegen for X86-64, make sure that 64-bit features
+  // are enabled.
   if (Is64Bit) {
     HasX86_64 = true;
   }