Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change default opt-level for Config to speed. #988

Merged
merged 2 commits into from
Feb 26, 2020

Conversation

peterhuene
Copy link
Member

This PR changes the default opt-level for a new Config to speed.

Fixes #981.

@peterhuene
Copy link
Member Author

Looking into the various test failures.

@peterhuene
Copy link
Member Author

peterhuene commented Feb 26, 2020

Looks like some of the SIMD spec testsuite tests are failing when optimizing for speed:

Cranelift::simd::simd_address
Cranelift::simd::simd_align
Cranelift::simd::simd_bitwise
Cranelift::simd::simd_f32x4_cmp
Cranelift::simd::simd_f64x2_cmp
Cranelift::simd::simd_i16x8_cmp
Cranelift::simd::simd_i32x4_cmp
Cranelift::simd::simd_i8x16_cmp
Cranelift::simd::simd_store

@alexcrichton
Copy link
Member

Want to update the wast testsuites with a FIXME where we set the optimization level to none, but only for the simd tests?

@peterhuene
Copy link
Member Author

Can do. Pinging @abrown relating to the SIMD failures when we optimize for speed.

Looks like there are additional failures for trap-related tests on Windows that I'll investigate, possibly related to unwind.

@peterhuene
Copy link
Member Author

For posterity, here's one such SIMD failure:

Error: failed directive on tests/spec_testsuite/proposals/simd/simd_f32x4_cmp.wast:7799:1

Caused by:
    0: WebAssembly failed to compile
    1: Compilation error: function u0:3(i64 vmctx [%rdi], i64 [%rsi]) system_v {
           gv0 = vmctx
           gv1 = load.i64 notrap aligned readonly gv0+4
           heap0 = static gv1, min 0, bound 0x0001_0000_0000, offset_guard 0x8000_0000, index_type i32

                                       block0(v0: i64, v1: i64):
       @0134 [RexOp1pu_id#b8]              v4 = iconst.i32 0
       @0136 [RexOp1umr#89]                v15 = uextend.i64 v4
       @0136 [RexOp1ldDisp8#808b]          v16 = load.i64 notrap aligned readonly v0+4
       @0136 [DynRexOp1rr#8001]            v5 = iadd v16, v15
       @0136 [DynRexOp2fld#410]            v6 = load_complex.i8x16 v16+v15
       ;~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
       ; error: inst2 (v6 = load_complex.i8x16 v16+v15): Instruction failed to re-encode DynRexOp2fld#410

       @013a [Op1jmpb#eb]                  jump block4(v6)

                                       block4(v3: i8x16):
       @013d [RexOp1pu_id#b8]              v8 = iconst.i32 1
       @013f [RexOp1umr#89]                v17 = uextend.i64 v8
       @013f [RexOp1ldDisp8#808b]          v18 = load.i64 notrap aligned readonly v0+4
       @013f [DynRexOp1rr#8001]            v9 = iadd v18, v17
       @013f [DynRexOp2fld#410]            v10 = load_complex.i8x16 v18+v17
       ;~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
       ; error: inst6 (v10 = load_complex.i8x16 v18+v17): Instruction failed to re-encode DynRexOp2fld#410

       @0143 [Op1jmpb#eb]                  jump block5(v10)

                                       block5(v7: i8x16):
       @0144 [null_fpr#00]                 v11 = raw_bitcast.f32x4 v3
       @0144 [null_fpr#00]                 v12 = raw_bitcast.f32x4 v7
       @0144 [RexOp2pfcmp#4c2]             v13 = fcmp le v11, v12
       @0146 [null_fpr#00]                 v14 = raw_bitcast.i8x16 v13
       @0146 [Op1jmpb#eb]                  jump block3(v14)

                                       block3(v2: i8x16):
       @0148 [Op1jmpb#eb]                  jump block2

                                       block2:
       @0149 [Op1jmpb#eb]                  jump block1

                                       block1:
       @0149 [Op1ret#c3]                   return
       }

       ; 2 verifier errors detected (see above). Compilation aborted.

@peterhuene
Copy link
Member Author

peterhuene commented Feb 26, 2020

The SIMD tests are the only ones that failing but I am seeing a bunch of additional panic output during the test executions:

thread '<unnamed>' panicked at 'assertion failed: ok', C:\Users\phuene\.cargo\registry\src\github.com-1ecc6299db9ec823\cranelift-codegen-0.59.0\src\postopt.rs:353:5

@abrown
Copy link
Contributor

abrown commented Feb 26, 2020

@peterhuene, thanks for the heads up. I suspect this is related to bytecodealliance/cranelift#1388 (using REX prefixes to access more registers) but I'm not exactly sure I understand what the effects of speed are. I'll look at this more tomorrow.

@abrown
Copy link
Contributor

abrown commented Feb 26, 2020

The simd_f32x4_cmp.wast example above is actually failing because load_complex does not (yet) have an encoding for the vector types; that should be relatively easy to add. I looked at simd_address.wast and it is failing for the same reason; postop.rs is replacing load with load_complex and the assertion at postop.rs:353 is failing because that instruction has no encoding yet. I agree with @alexcrichton: turn off the optimization for now until I can imlement load_complex for SIMD.

@peterhuene
Copy link
Member Author

@abrown thanks for looking into it! I'll disable optimization for the tests for now.

This commit changes the default opt-level for a new `Config` to `speed`.

Fixes bytecodealliance#981.
@peterhuene
Copy link
Member Author

I've opened bytecodealliance/cranelift#1409 to track the SIMD issue.

@peterhuene peterhuene merged commit 0c23c2e into bytecodealliance:master Feb 26, 2020
@peterhuene peterhuene deleted the default-opt-level branch February 26, 2020 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cranelift optimizations aren't enabled by default for the API.
3 participants