Bug 223415 - lang/rust: don't require SSE2 on i386 (at least for binary packages)
Summary: lang/rust: don't require SSE2 on i386 (at least for binary packages)
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: FreeBSD Rust Team
URL:
Keywords: patch
Depends on: 223939
Blocks:
  Show dependency treegraph
 
Reported: 2017-11-04 06:54 UTC by Jan Beich
Modified: 2017-12-11 05:53 UTC (History)
8 users (show)

See Also:
riggs: maintainer-feedback+


Attachments
v0 (3.66 KB, patch)
2017-11-04 06:54 UTC, Jan Beich
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Beich freebsd_committer freebsd_triage 2017-11-04 06:54:24 UTC
Created attachment 187710 [details]
v0

Rust currently assumes SSE2 for all i686 targets. When adopted into existing projects this means killing support for old hardware. For one, www/firefox managed to avoid using -march= but due to encoding_rs pervasive usage now requires SSE2. Users that want performance are better off using -C target-cpu=native and/or switching to x86_64, anyway.

https://github.com/rust-lang/rust/blob/1.21.0/src/librustc_back/target/i686_unknown_freebsd.rs#L16

  $ cat <<EOF >a.rs
  #![feature(cfg_target_feature)]

  fn main() {
      if cfg!(target_feature = "sse2") {
          println!("SSE2 code");
      } else if cfg!(target_feature = "sse") {
          println!("SSE code");
      } else if cfg!(target_feature = "mmx") {
          println!("MMX code");
      } else {
          println!("Generic code");
      }
  }
  EOF

  $ RUSTC_BOOTSTRAP=1 rustc a.rs

Before:

  $ ./a
  SSE2 code

  $ objdump -d a | fgrep xmm | wc -l
       631


After:

  $ ./a
  Generic code

  $ objdump -d a | fgrep xmm | wc -l
	15
Comment 1 Jan Beich freebsd_committer freebsd_triage 2017-11-04 07:03:26 UTC
Comment on attachment 187710 [details]
v0

Without testing under qemu or on real hardware this is a shot in the dark. FreeBSD doesn't support filtering SIMD at kernel level. Not to mention, benefit vs. maintenance ratio is unclear.
Comment 2 Thomas Zander freebsd_committer freebsd_triage 2017-11-04 07:44:22 UTC
(In reply to Jan Beich from comment #1)

We do have a couple of users who are still running non-SSE hardware. It has bitten me in the past with the multimedia stuff. Having said that, I would not expect the number to be very high. The number of firefox users on these machines is probably fairly low.

If we really want to support this on our own (these days more and more upstream projects abandon i386/i686 support altogether, let alone non-SSE variants), someone possessing said hardware should maintain this part.

My personal preference is not supporting it unless upstream does it.
Comment 3 Jan Beich freebsd_committer freebsd_triage 2017-11-08 07:03:03 UTC
i686-unknown-freebsd may have been copied from i686-unknown-dragonfly without checking Clang default CPU on FreeBSD.

https://github.com/rust-lang/rust/commit/296c74de96e2
https://github.com/llvm-mirror/clang/blob/6fc97e7c1cf4/lib/Driver/ToolChains/Arch/X86.cpp#L98
Comment 4 Jan Beich freebsd_committer freebsd_triage 2017-11-08 07:24:57 UTC
Can toolchain@ clarify plans regarding i386 support in future? Is FreeBSD going to be stuck targeting i486 until the architecture is dead? How much effort port maintainers are supposed to exert for i486 if upstream projects couldn't care less? It's not like FreeBSD to care about ancient hardware (unlike NetBSD).
Comment 5 Dimitry Andric freebsd_committer freebsd_triage 2017-11-08 18:32:42 UTC
(In reply to Jan Beich from comment #4)
> Can toolchain@ clarify plans regarding i386 support in future? Is FreeBSD
> going to be stuck targeting i486 until the architecture is dead?

I think the architecture itself will live on for quite some time, whether we like it or not.  In my opinion we should start requiring at least i686 or higher (or maybe even pentium4).

IIRC there have been plans to start an arch separate from i386, specifically for updating to 64-bit time_t, that could maybe also be used for such an update. :)

On the other hand, in Linux land there are already distros dropping i386 completely, e.g: https://www.archlinux.org/news/the-end-of-i686-support/


> How much
> effort port maintainers are supposed to exert for i486 if upstream projects
> couldn't care less? It's not like FreeBSD to care about ancient hardware
> (unlike NetBSD).

At some point, I guess we must simply stop supporting some ports for such targets.  I can't be too long until firefox and chromium are simply too large to run in a 32-bit address space...
Comment 6 Ed Maste freebsd_committer freebsd_triage 2017-11-14 13:49:57 UTC
I do not expect heroic efforts from porters to target pre-i686 x32 when it is not supported by upstream.
Comment 7 Jean-Sébastien Pédron freebsd_committer freebsd_triage 2017-11-27 18:23:09 UTC
Since you already did the work, feel free to commit it (or I can, just tell me).

If in the future, if the patch breaks, we can revisit the decision to maintain it or not.
Comment 8 commit-hook freebsd_committer freebsd_triage 2017-11-27 22:03:04 UTC
A commit references this bug:

Author: jbeich
Date: Mon Nov 27 22:02:30 UTC 2017
New revision: 454995
URL: https://svnweb.freebsd.org/changeset/ports/454995

Log:
  lang/rust: avoid LLVM targeting SSE2 on i386 by default

  This may help ports like textproc/ripgrep to run on old hardware.
  Rust itself still requires SSE2 until bootstrap is regenerated.

  PR:		223415
  Approved by:	rust (dumbbell)

Changes:
  head/Mk/bsd.gecko.mk
  head/lang/rust/Makefile
  head/lang/rust/files/patch-src_librustc__back_target_i686__unknown__freebsd.rs
  head/textproc/ripgrep/Makefile
  head/www/firefox/Makefile
Comment 9 commit-hook freebsd_committer freebsd_triage 2017-11-28 12:58:40 UTC
A commit references this bug:

Author: jbeich
Date: Tue Nov 28 12:57:57 UTC 2017
New revision: 455039
URL: https://svnweb.freebsd.org/changeset/ports/455039

Log:
  gecko: respect CPUTYPE for Rust code

  After r454995 build may fail if SSE2 is only enabled for C/C++ code as
  simd crate expects SSE2 based on MACHINE_CPU.

  PR:		223415 223300
  Reported by:	vvd@unislabs.com

Changes:
  head/Mk/bsd.gecko.mk
Comment 10 Jan Beich freebsd_committer freebsd_triage 2017-12-11 03:20:55 UTC
Don or Bertrand, can you check ripgrep binary package from /latest set works fine on a machine without SSE2? Building locally isn't recommended: textproc/ripgrep as -j1 has MaxRSS ~539 MiB.
Comment 11 Jan Beich freebsd_committer freebsd_triage 2017-12-11 03:58:22 UTC
If you're short on time here're the QA steps:

$ mkdir -p /usr/local/etc/pkg/repos/
$ sed -En '/url/s/quarterly/latest/; /{|}/p' /etc/pkg/FreeBSD.conf \
      >/usr/local/etc/pkg/repos/FreeBSD.conf

$ pkg install ripgrep

$ rg --version
ripgrep 0.7.1
-AVX -SIMD

$ command time -l rg shenanigan /usr/src
/usr/src/share/dict/web2
180127:shenanigan

/usr/src/sys/dev/ciss/ciss.c
3046:    * The reason for all these shenanigans is to create a maxio value that

/usr/src/contrib/llvm/lib/Target/X86/X86FastISel.cpp
1089:    // Handle extension to 64-bits via sub-register shenanigans.

/usr/src/crypto/heimdal/appl/telnet/telnetd/sys_term.c
485:         * Here are some shenanigans to make sure that there
        0.62 real         2.80 user         1.91 sys
     13200  maximum resident set size
      2572  average shared memory size
        44  average unshared data size
       128  average unshared stack size
      2251  page reclaims
         0  page faults
         0  swaps
      8292  block input operations
         0  block output operations
         0  messages sent
         0  messages received
         0  signals received
      2631  voluntary context switches
      2156  involuntary context switches

$ pkg delete ripgrep
Comment 12 Bertrand Petit 2017-12-11 04:19:03 UTC
(In reply to Jan Beich from comment #11)
Jan, I took another route:

$ fetch http://pkg.freebsd.org/FreeBSD:11:i386/latest/All/ripgrep-0.7.1_3.txz
[...]
$ sha256 ripgrep-0.7.1_3.txz 
SHA256 (ripgrep-0.7.1_3.txz) = 501e31aef29bf5cd7c73bdea4eca376126ee2bf1b06da044071e0d8c3179af51
$ tar xf ripgrep-0.7.1_3.txz
$ objdump -d usr/local/bin/rg | grep xmm
  1d922c:       f2 0f 2a 4c 24 08       cvtsi2sd 0x8(%esp),%xmm1
  1d9232:       f3 0f 10 44 24 04       movss  0x4(%esp),%xmm0
  1d923e:       f2 0f 59 88 13 62 05    mulsd  0x56213(%eax),%xmm1
  1d9246:       f2 0f 10 90 03 62 05    movsd  0x56203(%eax),%xmm2
  1d924e:       f2 0f 5c ca             subsd  %xmm2,%xmm1
  1d9252:       66 0f 56 c2             orpd   %xmm2,%xmm0
  1d9256:       f2 0f 58 c1             addsd  %xmm1,%xmm0
  1d925a:       f2 0f 11 44 24 04       movsd  %xmm0,0x4(%esp)
  1d9268:       f3 0f 10 4c 24 08       movss  0x8(%esp),%xmm1
  1d926e:       f3 0f 10 44 24 04       movss  0x4(%esp),%xmm0
  1d927a:       66 0f 56 88 07 62 05    orpd   0x56207(%eax),%xmm1
  1d9282:       f2 0f 5c 88 f7 61 05    subsd  0x561f7(%eax),%xmm1
  1d928a:       66 0f 56 80 e7 61 05    orpd   0x561e7(%eax),%xmm0
  1d9292:       f2 0f 58 c1             addsd  %xmm1,%xmm0
  1d9296:       f2 0f 11 44 24 04       movsd  %xmm0,0x4(%esp)

Unfortunately cvtsi2sd is an SSE2 instruction, this package wont run on non SSE2 hosts unless that operation is located in dead code.
Comment 13 Jan Beich freebsd_committer freebsd_triage 2017-12-11 05:04:37 UTC
(In reply to Bertrand Petit from comment #12)
> $ objdump -d usr/local/bin/rg | grep xmm
[...]

WITH_DEBUG=1 port build contains symbol names. It seems xmm registers are used by __floatdidf and __floatundidf from compiler-rt. Rust compiler always includes optimized routines on i386 but not always uses them. If my understanding is correct then the code is dead.

# Rust 1.22.1
https://github.com/rust-lang-nursery/compiler-builtins/blob/0b9844764ea1/build.rs#L3977
https://github.com/rust-lang-nursery/compiler-builtins/blob/0b9844764ea1/src/float/conv.rs#L91
https://github.com/rust-lang-nursery/compiler-builtins/blob/0b9844764ea1/src/float/conv.rs#L120
https://github.com/rust-lang/compiler-rt/blob/c8a8767c56ad/lib/builtins/i386/floatdidf.S
https://github.com/rust-lang/compiler-rt/blob/c8a8767c56ad/lib/builtins/i386/floatundidf.S
Comment 14 Bertrand Petit 2017-12-11 05:53:51 UTC
(In reply to Jan Beich from comment #13)

You are likely correct. However you have just uncovered a different bug, a replication on one already fixed in src:
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=221733
You may want to adress it to rid the rust bootstrap of SIMD instructions.