Bug 221733

Summary: SSE2 instructions emited in compiler-rt on AMD Sempron 3000+
Product: Base System Reporter: Bertrand Petit <bsdpr>
Component: binAssignee: Dimitry Andric <dim>
Status: Closed FIXED    
Severity: Affects Some People CC: arrowd, bsdpr, dim, emaste, kib, theraven
Priority: --- Keywords: patch
Version: 10.3-STABLEFlags: dim: mfc-stable11+
dim: mfc-stable10+
dim: mfc-stable9+
Hardware: i386   
OS: Any   
Attachments:
Description Flags
Patch reverting to C implementations on i386 none

Description Bertrand Petit 2017-08-23 04:02:26 UTC 
On an i386 host using an AMD Sempron 3000+ CPU the compiler-rt include at least one SSE2 instruction which is not supported by that CPU.

Boot log CPU identification:

CPU: AMD Sempron(tm)   3000+ (1999.82-MHz 686-class CPU)
  Origin="AuthenticAMD"  Id=0x6a0  Family=0x6  Model=0xa  Stepping=0
  Features=0x383fbff<FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,MMX,FXSR,SSE>
  AMD Features=0xc0480800<SYSCALL,MP,MMX+,3DNow!+,3DNow!>

Excerpt from objdump -d /usr/lib/libcompiler-rt.a:

floatdidf.o:     file format elf32-i386-freebsd

Disassembly of section .text:

00000000 <twop52>:
   0:   00 00                   add    %al,(%eax)
   2:   00 00                   add    %al,(%eax)
   4:   00 00                   add    %al,(%eax)
   6:   30 43 00                xor    %al,0x0(%ebx)

00000008 <twop32>:
   8:   00 00                   add    %al,(%eax)
   a:   00 00                   add    %al,(%eax)
   c:   00 00                   add    %al,(%eax)
   e:   f0 41                   lock inc %ecx

00000010 <__floatdidf>:
  10:   f2 0f 2a 4c 24 08       cvtsi2sd 0x8(%esp),%xmm1
  16:   f3 0f 10 44 24 04       movss  0x4(%esp),%xmm0
  1c:   e8 00 00 00 00          call   21 <__floatdidf+0x11>
  21:   58                      pop    %eax
  22:   f2 0f 59 88 e7 ff ff    mulsd  -0x19(%eax),%xmm1
  29:   ff
  2a:   f2 0f 10 90 df ff ff    movsd  -0x21(%eax),%xmm2
  31:   ff
  32:   f2 0f 5c ca             subsd  %xmm2,%xmm1
  36:   66 0f 56 c2             orpd   %xmm2,%xmm0
  3a:   f2 0f 58 c1             addsd  %xmm1,%xmm0
  3e:   f2 0f 11 44 24 04       movsd  %xmm0,0x4(%esp)
  44:   dd 44 24 04             fldl   0x4(%esp)
  48:   c3                      ret

The first instruction of __floatdidf, cvtsi2sd, is not supported by that CPU.
World and kernel were built on that same host from svn revision 320864.
Comment 1 Konstantin Belousov freebsd_committer freebsd_triage 2017-08-23 08:03:27 UTC 
i386 system libraries must not access %xmm registers at all.  At least not without checking for CPUID support first, and then checking for SSE and SSE2 features as needed.

lib32 libraries built on amd64 host are somewhat different.  We compile them with ppro arch set, which at least allows cmovX instructions, and this environment also can assume that SSE2 is present, since amd64 requires CPU with SSE2 (there is no long-mode capable CPU not supporting SSE2).

For i386, this is definitely bug.
Comment 2 Ed Maste freebsd_committer freebsd_triage 2017-08-23 12:12:38 UTC 
To be clear, this is not a compiler bug: floatdidf comes from an assembly routine in compiler-rt/lib/builtins/i386/floatdidf.S. I see there is a generic C version in compiler-rt/lib/builtins/floatdidf.c that we might be able to use, although preferably using the optimized one (via ifunc or such) when possible.
Comment 3 Dimitry Andric freebsd_committer freebsd_triage 2017-08-23 20:43:04 UTC 
Hm, there are more .S files under contrib/compiler-rt/lib/builtins/i386 which unconditionally use SSE, although for instance ashldi3.S has an explicit test for __SSE2__, and if that isn't defined, it uses GPRs.

I think this might be worth an upstream bug.
Comment 4 David Chisnall freebsd_committer freebsd_triage 2017-08-24 09:02:34 UTC 
I suspect it will see little priority upstream.  SSE2 is supported by most post-2003 CPUs (and some post-2001 CPUs), and none of the companies that fund most LLVM development care about processors more than a decade old.

If we still care about these targets, we should probably use the C versions for i386 and the asm versions only for lib32 on amd64.
Comment 5 Bertrand Petit 2017-08-29 09:39:12 UTC 
Created attachment 185862 [details]
Patch reverting to C implementations on i386

This patch, on i386, reverts to the C implementations of the functions affected by the unguarded use of SSE2 instructions in the assembler implementations.
Comment 6 Bertrand Petit 2017-08-29 09:43:23 UTC 
Here is a naïve patch that use the C implementation of the affected functions.

After buildworld and installworld:
$ objdump -d /usr/lib/libcompiler_rt.a | grep -c xmm
0
Comment 7 commit-hook freebsd_committer freebsd_triage 2017-08-29 21:45:52 UTC 
A commit references this bug:

Author: dim
Date: Tue Aug 29 21:45:00 UTC 2017
New revision: 323001
URL: https://svnweb.freebsd.org/changeset/base/323001

Log:
  In compiler-rt, a few assembler implementations for i386 floating point
  conversion functions use SSE2 instructions, but these are not guarded by
  #ifdef __SSE2__, and there is no implementation using general purpose
  registers.  For these functions, use the generic C variants instead,
  otherwise they will cause SIGILL on older processors.

  Reported by:	bsdpr@phoe.frmug.org
  PR:		221733
  MFC after:	1 week

Changes:
  head/lib/libcompiler_rt/Makefile.inc
Comment 8 commit-hook freebsd_committer freebsd_triage 2017-08-30 07:06:20 UTC 
A commit references this bug:

Author: dim
Date: Wed Aug 30 07:05:29 UTC 2017
New revision: 323014
URL: https://svnweb.freebsd.org/changeset/base/323014

Log:
  Follow-up to r323001: if the actually selected CPUTYPE is capable of
  SSE2 instructions, we can use them.

  Suggested by:	jkim
  PR:		221733
  MFC after:	1 week
  X-MFC-With:	r323001

Changes:
  head/lib/libcompiler_rt/Makefile.inc
Comment 9 commit-hook freebsd_committer freebsd_triage 2017-09-05 16:59:38 UTC 
A commit references this bug:

Author: dim
Date: Tue Sep  5 16:59:20 UTC 2017
New revision: 323187
URL: https://svnweb.freebsd.org/changeset/base/323187

Log:
  MFC r323001:

  In compiler-rt, a few assembler implementations for i386 floating point
  conversion functions use SSE2 instructions, but these are not guarded by
  #ifdef __SSE2__, and there is no implementation using general purpose
  registers.  For these functions, use the generic C variants instead,
  otherwise they will cause SIGILL on older processors.

  Reported by:	bsdpr@phoe.frmug.org
  PR:		221733

Changes:
_U  stable/11/
  stable/11/lib/libcompiler_rt/Makefile.inc
Comment 10 commit-hook freebsd_committer freebsd_triage 2017-09-05 17:13:52 UTC 
A commit references this bug:

Author: dim
Date: Tue Sep  5 17:12:12 UTC 2017
New revision: 323188
URL: https://svnweb.freebsd.org/changeset/base/323188

Log:
  MFC r323014:

  Follow-up to r323001: if the actually selected CPUTYPE is capable of
  SSE2 instructions, we can use them.

  Suggested by:	jkim
  PR:		221733

Changes:
_U  stable/11/
  stable/11/lib/libcompiler_rt/Makefile.inc
Comment 11 commit-hook freebsd_committer freebsd_triage 2017-09-05 17:33:12 UTC 
A commit references this bug:

Author: dim
Date: Tue Sep  5 17:32:15 UTC 2017
New revision: 323189
URL: https://svnweb.freebsd.org/changeset/base/323189

Log:
  MFC r323001:

  In compiler-rt, a few assembler implementations for i386 floating point
  conversion functions use SSE2 instructions, but these are not guarded by
  #ifdef __SSE2__, and there is no implementation using general purpose
  registers.  For these functions, use the generic C variants instead,
  otherwise they will cause SIGILL on older processors.

  Approved by:	re (kib)
  Reported by:	bsdpr@phoe.frmug.org
  PR:		221733

  MFC r323014:

  Follow-up to r323001: if the actually selected CPUTYPE is capable of
  SSE2 instructions, we can use them.

  Suggested by:	jkim
  PR:		221733

Changes:
_U  stable/10/
  stable/10/lib/libcompiler_rt/Makefile
Comment 12 commit-hook freebsd_committer freebsd_triage 2017-09-05 17:33:15 UTC 
A commit references this bug:

Author: dim
Date: Tue Sep  5 17:32:15 UTC 2017
New revision: 323189
URL: https://svnweb.freebsd.org/changeset/base/323189

Log:
  MFC r323001:

  In compiler-rt, a few assembler implementations for i386 floating point
  conversion functions use SSE2 instructions, but these are not guarded by
  #ifdef __SSE2__, and there is no implementation using general purpose
  registers.  For these functions, use the generic C variants instead,
  otherwise they will cause SIGILL on older processors.

  Approved by:	re (kib)
  Reported by:	bsdpr@phoe.frmug.org
  PR:		221733

  MFC r323014:

  Follow-up to r323001: if the actually selected CPUTYPE is capable of
  SSE2 instructions, we can use them.

  Suggested by:	jkim
  PR:		221733

Changes:
_U  stable/10/
  stable/10/lib/libcompiler_rt/Makefile
Comment 13 commit-hook freebsd_committer freebsd_triage 2017-09-05 17:36:19 UTC 
A commit references this bug:

Author: dim
Date: Tue Sep  5 17:35:34 UTC 2017
New revision: 323190
URL: https://svnweb.freebsd.org/changeset/base/323190

Log:
  MFC r323001:

  In compiler-rt, a few assembler implementations for i386 floating point
  conversion functions use SSE2 instructions, but these are not guarded by
  #ifdef __SSE2__, and there is no implementation using general purpose
  registers.  For these functions, use the generic C variants instead,
  otherwise they will cause SIGILL on older processors.

  Reported by:  bsdpr@phoe.frmug.org
  PR:           221733

  MFC r323014:

  Follow-up to r323001: if the actually selected CPUTYPE is capable of
  SSE2 instructions, we can use them.

  Suggested by: jkim

Changes:
_U  stable/9/
_U  stable/9/lib/
_U  stable/9/lib/libcompiler_rt/
  stable/9/lib/libcompiler_rt/Makefile