Add "x87" in x86 target feature map

[aturetsk]

Summary

Add 'x87' in x86 target feature map

[rsmith]

lib/Basic/Targets.cpp

2597–2598

i386 doesn't necessarily have x87, does it? IIRC the floating point co-processor is only guaranteed to be available in the 486 DX and later.

[aturetsk]

Do not enable X87 for i386

[aturetsk]

Hello Richard,
Thank for the review.

lib/Basic/Targets.cpp

2597–2598

You are right. Fixed.

[echristo]

Are there any of the intrinsics in the headers that also depend on x87?

One inline comment.

-eric

lib/Basic/Targets.cpp

2597–2598

Mind only doing getCPUKind once here? :)

[aturetsk]

Use getCPUKind once

[aturetsk]

Are there any of the intrinsics in the headers that also depend on x87?

Not that I could find.

lib/Basic/Targets.cpp

2600–2601

Done.

[rsmith]

lib/Basic/Targets.cpp

2603

What about CK_Generic? Also, if CK_i486 can be used for the 486SX, we need to exclude it here too. It looks (from wikipedia) like all the "WinChip" flavours of 486 have an x87 unit, but it'd be nice if someone could confirm that. Maybe we should have separate CPUKinds for 486 SX versus 486 DX.

[aturetsk]

lib/Basic/Targets.cpp

2603

Before this changeset clang would generate x87 instructions for Generic, i386 and i486. So one can say they had FeatureX87 enabled implicitly. If that was ok formerly why do we want to change that now? Wouldn't it be better to keep clang's behavior unchanged?

[rsmith]

lib/Basic/Targets.cpp

2603

OK, if this means "x87 instructions probably work" (although they may be emulated in software) and not "x87 instructions definitely work", then we shouldn't be checking the CPU kind here at all. It looks like the former is how GCC handles this; it has -mno-80387 to specify that we're building for an i386 with no support for x87 instructions. Should we handle that flag?

[aturetsk]

Enable X87 back for all X86 processors.

[aturetsk]

lib/Basic/Targets.cpp

2603

"x87 instructions probably work" is more like it, and having feature x87 disabled would mean "x87 instructions definitely don't work". That's what I intended. If feature soft float is specified it takes precedence over feature x87 causing compiler to generate calls instead of X87 instructions.
Support of -m80387/-mno-80387 seems to be a good thing to have in Clang to be compatible with GCC (although briefly looking at GCC's sources these options just seem to be synonyms to -mhard-float/-msoft-float). I can add the flags, but I think that's better to be done in a separate patch.

[aturetsk]

Rebased.

[aturetsk]

Hi,
The related LLVM patch (http://reviews.llvm.org/D13979) was approved. Is this patch ok for commit?

[echristo]

Can you add an explicit test for soft/hard-float.

One inline comment as well, waiting on Richard to pipe up.

-eric

lib/Basic/Targets.cpp

2603

Waiting for rsmith to comment here.

[rsmith]

lib/Basic/Targets.cpp

2603

This seems fine to me.

[echristo]

LGTM.

Thanks!

-eric

[aturetsk]

The test Eric asked for was added.

lib/Basic/Targets.cpp

 bool X86TargetInfo::initFeatureMap(
     llvm::StringMap<bool> &Features, DiagnosticsEngine &Diags, StringRef CPU,
     const std::vector<std::string> &FeaturesVec) const {
   // FIXME: This *really* should not be here.
   // X86_64 always has SSE2.
   if (getTriple().getArch() == llvm::Triple::x86_64)
     setFeatureEnabledImpl(Features, "sse2", true);
 
+  // Enable X87 for all X86 processors.
+  setFeatureEnabledImpl(Features, "x87", true);
+
   switch (getCPUKind(CPU)) {
   case CK_Generic:
   case CK_i386:
   case CK_i486:
   case CK_i586:
   case CK_Pentium:
   case CK_i686:
   case CK_PentiumPro:
     break;
   case CK_PentiumMMX:
   case CK_Pentium2:
   case CK_K6:

test/CodeGen/attr-target-x86-mmx.c

void __attribute__((target("sse"))) shift(__m64 a, __m64 b, int c) {
   _mm_srli_pi16(a, c);
   _mm_srli_pi32(a, c);
   _mm_srli_si64(a, c);
 
   _mm_srai_pi16(a, c);
   _mm_srai_pi32(a, c);
 }
 
-// CHECK: "target-features"="+mmx,+sse"
+// CHECK: "target-features"="+mmx,+sse,+x87"

test/CodeGen/attr-target-x86.c

 // We're currently ignoring the fpmath attribute so koala should be identical to baz and bar.
 // CHECK: koala{{.*}} #0
 // CHECK: echidna{{.*}} #2
 // CHECK: panda{{.*}} #3
 // CHECK: bar{{.*}} #0
 // CHECK: qux{{.*}} #1
 // CHECK: qax{{.*}} #4
 // CHECK: qq{{.*}} #5
-// CHECK: #0 = {{.*}}"target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2"
-// CHECK: #1 = {{.*}}"target-cpu"="ivybridge" "target-features"="+aes,+avx,+cx16,+f16c,+fsgsbase,+fxsr,+mmx,+pclmul,+popcnt,+rdrnd,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+xsave,+xsaveopt"
-// CHECK: #2 = {{.*}}"target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,-aes,-avx,-avx2,-avx512bw,-avx512cd,-avx512dq,-avx512er,-avx512f,-avx512pf,-avx512vl,-f16c,-fma,-fma4,-pclmul,-sha,-sse2,-sse3,-sse4.1,-sse4.2,-sse4a,-ssse3,-xop,-xsave,-xsaveopt"
-// CHECK: #3 = {{.*}}"target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+popcnt,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3"
-// CHECK: #4 = {{.*}}"target-cpu"="ivybridge" "target-features"="+avx,+cx16,+f16c,+fsgsbase,+fxsr,+mmx,+pclmul,+popcnt,+rdrnd,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+xsave,+xsaveopt,-aes"
-// CHECK: #5 = {{.*}}"target-cpu"="x86-64" "target-features"="+fxsr,+sse,+sse2,-3dnow,-3dnowa,-mmx"
+// CHECK: #0 = {{.*}}"target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2,+x87"
+// CHECK: #1 = {{.*}}"target-cpu"="ivybridge" "target-features"="+aes,+avx,+cx16,+f16c,+fsgsbase,+fxsr,+mmx,+pclmul,+popcnt,+rdrnd,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87,+xsave,+xsaveopt"
+// CHECK: #2 = {{.*}}"target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+x87,-aes,-avx,-avx2,-avx512bw,-avx512cd,-avx512dq,-avx512er,-avx512f,-avx512pf,-avx512vl,-f16c,-fma,-fma4,-pclmul,-sha,-sse2,-sse3,-sse4.1,-sse4.2,-sse4a,-ssse3,-xop,-xsave,-xsaveopt"
+// CHECK: #3 = {{.*}}"target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+popcnt,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87"
+// CHECK: #4 = {{.*}}"target-cpu"="ivybridge" "target-features"="+avx,+cx16,+f16c,+fsgsbase,+fxsr,+mmx,+pclmul,+popcnt,+rdrnd,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87,+xsave,+xsaveopt,-aes"
+// CHECK: #5 = {{.*}}"target-cpu"="x86-64" "target-features"="+fxsr,+sse,+sse2,+x87,-3dnow,-3dnowa,-mmx"