Use SIMD in `gfx` when possible #10900

mmatyas · 2016-04-28T16:58:53Z

Currently SIMD support is enabled by checking the target architecture (x86_64 and AArch64). A better approach would be to check if a SIMD feature is actually available on the platform, by using cfg_target_feature. This patch enables SIMD code if possible, which will cause speedup on Android and ARM embedded devices.

This change is

highfive · 2016-04-28T16:58:57Z

Warning

These commits modify gfx code, but no tests are modified. Please consider adding a test!

mmatyas · 2016-04-28T17:07:10Z

Fixed the tidy warning.

metajack · 2016-04-28T17:11:49Z

@bors-servo r+

Reviewed 3 of 3 files at r1.
Review status: 2 of 3 files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

bors-servo · 2016-04-28T17:11:52Z

📌 Commit f81c1b1 has been approved by metajack

bors-servo · 2016-04-28T17:11:54Z

⌛ Testing commit f81c1b1 with merge 3229782...


        Auto merge of #10900 - mmatyas:simdglyph, r=metajack

Use SIMD in `gfx` when possible Currently SIMD support is enabled by checking the target architecture (x86_64 and AArch64). A better approach would be to check if a SIMD feature is actually available on the platform, by using `cfg_target_feature`. This patch enables SIMD code if possible, which will cause speedup on Android and ARM embedded devices.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10900)

bors-servo · 2016-04-28T17:15:16Z

💔 Test failed - arm32

metajack · 2016-04-28T17:18:09Z

common.rs:19:5: 19:22 error: unresolved importarm::neon::common. Could not findneoninarm[E0432]

mmatyas · 2016-04-28T17:24:46Z

The problem is here in simd: https://github.com/huonw/simd/blob/master/src/common.rs#L18, I'll send a PR there too.

mmatyas · 2016-04-28T17:32:32Z

Hm, but that would also require to only compile simd if needed. I wonder if there is a way to decide that in Cargo.toml.

bors-servo · 2016-04-29T05:38:28Z

☔ The latest upstream changes (presumably #10830) made this pull request unmergeable. Please resolve the merge conflicts.

mmatyas · 2016-04-29T07:24:39Z

Rebased and squashed.

nox · 2016-04-29T08:58:21Z

@mmatyas Could you file an issue about this on the simd crate?

@bors-servo r+

bors-servo · 2016-04-29T08:58:23Z

📌 Commit 538dc0d has been approved by nox

notriddle · 2016-09-12T19:13:34Z

Are there any Android experts who'd be able to help with this?

bors-servo · 2016-11-17T23:46:53Z

☔ The latest upstream changes (presumably #14246) made this pull request unmergeable. Please resolve the merge conflicts.

mmatyas · 2016-11-18T12:10:17Z

Rebased, hopefully it can land after Android gets fixed.

nox · 2016-11-28T12:51:06Z

@bors-servo try

bors-servo · 2016-11-28T12:51:09Z

⌛ Trying commit 196eb6e with merge 13e9ffe...


        Auto merge of #10900 - mmatyas:simdglyph, r=<try>

Use SIMD in `gfx` when possible Currently SIMD support is enabled by checking the target architecture (x86_64 and AArch64). A better approach would be to check if a SIMD feature is actually available on the platform, by using `cfg_target_feature`. This patch enables SIMD code if possible, which will cause speedup on Android and ARM embedded devices.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10900)

bors-servo · 2016-11-28T13:37:17Z

💔 Test failed - linux-rel-wpt

highfive · 2016-11-28T13:37:19Z

  ▶ TIMEOUT [expected PASS] /_mozilla/css/border_radius_elliptical_a.html
  │ 
  │ VMware, Inc.
  │ Gallium 0.4 on softpipe
  └ 3.3 (Core Profile) Mesa 12.0.1

nox · 2016-11-28T13:54:21Z

This is #14229.

How can we test that there is no issue on Android anymore?

mmatyas · 2016-11-28T23:05:15Z

At the moment, Android just shows a gray screen (see #13154), by "Android gets fixed", I've meant that issue :)
If everything works correctly, we can test it with etc/ci/check_dynamic_symbols.py, it should not show any unknown symbols. But since there have been some changes in the Android build, that script may need an update too, after the current display problems get fixed.

larsbergstrom · 2016-12-13T21:36:07Z

@bors-servo try

Running a try build so that I can check and see if the symbols have changed. Will r+ if it looks good.

larsbergstrom · 2016-12-13T21:36:17Z

@bors-servo retry


        Auto merge of #10900 - mmatyas:simdglyph, r=<try>

Use SIMD in `gfx` when possible Currently SIMD support is enabled by checking the target architecture (x86_64 and AArch64). A better approach would be to check if a SIMD feature is actually available on the platform, by using `cfg_target_feature`. This patch enables SIMD code if possible, which will cause speedup on Android and ARM embedded devices.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10900)

bors-servo · 2016-12-13T21:36:21Z

⌛ Trying commit 196eb6e with merge 3581b3a...

bors-servo · 2016-12-13T23:27:32Z

☀️ Test successful - arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-dev

larsbergstrom · 2016-12-14T01:08:45Z

This seems to have failed: http://build.servo.org/builders/android/builds/4447/steps/compile/logs/stdio

error[E0308]: mismatched types
   --> /home/servo/.cargo/git/checkouts/simd-a428b8cc9c1d1986/0d85d25/src/arm/neon.rs:506:45
    |
506 |                         let x = super::$min(lo, hi);
    |                                             ^^ expected struct `arm::neon::u32x2`, found struct `u32x2`
...
524 |     bools! {
    |     - in this macro invocation
    |
    = note: expected type `arm::neon::u32x2`
    = note:    found type `u32x2`

error[E0308]: mismatched types
   --> /home/servo/.cargo/git/checkouts/simd-a428b8cc9c1d1986/0d85d25/src/arm/neon.rs:506:49
    |
506 |                         let x = super::$min(lo, hi);
    |                                                 ^^ expected struct `arm::neon::u32x2`, found struct `u32x2`
...
524 |     bools! {
    |     - in this macro invocation
    |
    = note: expected type `arm::neon::u32x2`
    = note:    found type `u32x2`

error[E0308]: mismatched types
   --> /home/servo/.cargo/git/checkouts/simd-a428b8cc9c1d1986/0d85d25/src/arm/neon.rs:515:45
    |
515 |                         let x = super::$max(lo, hi);
    |                                             ^^ expected struct `arm::neon::u32x2`, found struct `u32x2`
...
524 |     bools! {
    |     - in this macro invocation
    |
    = note: expected type `arm::neon::u32x2`
    = note:    found type `u32x2`

error[E0308]: mismatched types
   --> /home/servo/.cargo/git/checkouts/simd-a428b8cc9c1d1986/0d85d25/src/arm/neon.rs:515:49
    |
515 |                         let x = super::$max(lo, hi);
    |                                                 ^^ expected struct `arm::neon::u32x2`, found struct `u32x2`
...
524 |     bools! {
    |     - in this macro invocation
    |
    = note: expected type `arm::neon::u32x2`
    = note:    found type `u32x2`

error[E0308]: mismatched types
   --> /home/servo/.cargo/git/checkouts/simd-a428b8cc9c1d1986/0d85d25/src/arm/neon.rs:506:45
    |
506 |                         let x = super::$min(lo, hi);
    |                                             ^^ expected struct `arm::neon::u32x2`, found struct `u32x2`
...
524 |     bools! {
    |     - in this macro invocation
    |
    = note: expected type `arm::neon::u32x2`
    = note:    found type `u32x2`

error[E0308]: mismatched types
   --> /home/servo/.cargo/git/checkouts/simd-a428b8cc9c1d1986/0d85d25/src/arm/neon.rs:506:49
    |
506 |                         let x = super::$min(lo, hi);
    |                                                 ^^ expected struct `arm::neon::u32x2`, found struct `u32x2`
...
524 |     bools! {
    |     - in this macro invocation
    |
    = note: expected type `arm::neon::u32x2`
    = note:    found type `u32x2`

error[E0308]: mismatched types
   --> /home/servo/.cargo/git/checkouts/simd-a428b8cc9c1d1986/0d85d25/src/arm/neon.rs:515:45
    |
515 |                         let x = super::$max(lo, hi);
    |                                             ^^ expected struct `arm::neon::u32x2`, found struct `u32x2`
...
524 |     bools! {
    |     - in this macro invocation
    |
    = note: expected type `arm::neon::u32x2`
    = note:    found type `u32x2`

error[E0308]: mismatched types
   --> /home/servo/.cargo/git/checkouts/simd-a428b8cc9c1d1986/0d85d25/src/arm/neon.rs:515:49
    |
515 |                         let x = super::$max(lo, hi);
    |                                                 ^^ expected struct `arm::neon::u32x2`, found struct `u32x2`
...
524 |     bools! {
    |     - in this macro invocation
    |
    = note: expected type `arm::neon::u32x2`
    = note:    found type `u32x2`

error: aborting due to 8 previous errors


        Use SIMD in `gfx` when possible

mmatyas · 2016-12-14T10:31:53Z

Hmm, I wonder if there was some kind of API change... cc @huonw. I've also rebased the patch to the current master.

KiChjang · 2016-12-14T10:50:24Z

@mmatyas Huon is prohibited from participating with any Rust projects (and discussions as well? Not sure). He might not respond.

larsbergstrom · 2016-12-21T20:43:04Z

@mmatyas Should the new one you pushed work? I can do another try build. Or r+, now that we actually are gating on Android again!

mmatyas · 2016-12-22T11:23:36Z

Not yet, but I've just sent a PR to simd, as soon as that lands (and we start using it), this patch will compile fine too.

nox · 2017-02-04T11:47:33Z

Superseded by #15381 with simd 0.2.0, thanks for keeping up with it!

highfive assigned metajack Apr 28, 2016

highfive added the S-awaiting-review label Apr 28, 2016

highfive unassigned metajack Apr 28, 2016

This was referenced Apr 28, 2016

Assign reviewers to PRs when they approve servo/highfive#92

Merged

Revert "Assign reviewers to PRs when they approve" servo/highfive#102

Merged

jdm assigned metajack Apr 28, 2016

highfive removed the S-awaiting-review label Apr 28, 2016

highfive added the S-awaiting-merge label Apr 28, 2016

highfive added S-tests-failed and removed S-awaiting-merge labels Apr 28, 2016

highfive added S-awaiting-review and removed S-tests-failed labels Apr 28, 2016

highfive added the S-needs-rebase label Apr 29, 2016

mmatyas force-pushed the mmatyas:simdglyph branch from b41fdfd to 538dc0d Apr 29, 2016

highfive removed S-awaiting-review S-needs-rebase labels Apr 29, 2016

highfive added the S-needs-rebase label Nov 17, 2016

mmatyas force-pushed the mmatyas:simdglyph branch from f10b3ed to 196eb6e Nov 18, 2016

nox removed the S-needs-rebase label Nov 28, 2016

highfive added the S-tests-failed label Nov 28, 2016

nox removed the S-tests-failed label Nov 28, 2016

Use SIMD in `gfx` when possible

Loading status checks…

1fc36ca

mmatyas force-pushed the mmatyas:simdglyph branch from 196eb6e to 1fc36ca Dec 14, 2016

mmatyas mentioned this pull request Dec 22, 2016

Fix ARM NEON compile errors huonw/simd#37

Open

nox closed this Feb 4, 2017

servo / servo

Use SIMD in `gfx` when possible #10900

Use SIMD in `gfx` when possible #10900

mmatyas commented Apr 28, 2016 •

edited by metajack

highfive commented Apr 28, 2016

mmatyas commented Apr 28, 2016

metajack commented Apr 28, 2016

bors-servo commented Apr 28, 2016

bors-servo commented Apr 28, 2016

bors-servo commented Apr 28, 2016

metajack commented Apr 28, 2016

mmatyas commented Apr 28, 2016

mmatyas commented Apr 28, 2016

bors-servo commented Apr 29, 2016

mmatyas commented Apr 29, 2016

nox commented Apr 29, 2016

bors-servo commented Apr 29, 2016

notriddle commented Sep 12, 2016

bors-servo commented Nov 17, 2016

mmatyas commented Nov 18, 2016

nox commented Nov 28, 2016

bors-servo commented Nov 28, 2016

bors-servo commented Nov 28, 2016

highfive commented Nov 28, 2016

nox commented Nov 28, 2016

mmatyas commented Nov 28, 2016

larsbergstrom commented Dec 13, 2016

larsbergstrom commented Dec 13, 2016

bors-servo commented Dec 13, 2016

bors-servo commented Dec 13, 2016

larsbergstrom commented Dec 14, 2016

mmatyas commented Dec 14, 2016

KiChjang commented Dec 14, 2016

larsbergstrom commented Dec 21, 2016

mmatyas commented Dec 22, 2016

nox commented Feb 4, 2017

servo / servo

Join GitHub today

Use SIMD in `gfx` when possible #10900

Use SIMD in `gfx` when possible #10900

Conversation

mmatyas commented Apr 28, 2016 • edited by metajack

highfive commented Apr 28, 2016

mmatyas commented Apr 28, 2016

metajack commented Apr 28, 2016

bors-servo commented Apr 28, 2016

bors-servo commented Apr 28, 2016

bors-servo commented Apr 28, 2016

metajack commented Apr 28, 2016

mmatyas commented Apr 28, 2016

mmatyas commented Apr 28, 2016

bors-servo commented Apr 29, 2016

mmatyas commented Apr 29, 2016

nox commented Apr 29, 2016

bors-servo commented Apr 29, 2016

notriddle commented Sep 12, 2016

bors-servo commented Nov 17, 2016

mmatyas commented Nov 18, 2016

nox commented Nov 28, 2016

bors-servo commented Nov 28, 2016

bors-servo commented Nov 28, 2016

highfive commented Nov 28, 2016

nox commented Nov 28, 2016

mmatyas commented Nov 28, 2016

larsbergstrom commented Dec 13, 2016

larsbergstrom commented Dec 13, 2016

bors-servo commented Dec 13, 2016

bors-servo commented Dec 13, 2016

larsbergstrom commented Dec 14, 2016

mmatyas commented Dec 14, 2016

KiChjang commented Dec 14, 2016

larsbergstrom commented Dec 21, 2016

mmatyas commented Dec 22, 2016

nox commented Feb 4, 2017

mmatyas commented Apr 28, 2016 •

edited by metajack