Skip to content

Conversation

@annxingyuan
Copy link
Contributor

@annxingyuan annxingyuan commented May 28, 2020

Adds a flag to evaluate SIMD support.

To see the logs from the Cloud Build CI, please join either our discussion or announcement mailing list.


This change is Reviewable

@annxingyuan annxingyuan marked this pull request as ready for review June 8, 2020 14:24
@annxingyuan annxingyuan self-assigned this Jun 8, 2020
Copy link
Collaborator

@lina128 lina128 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Contributor

@tafsiri tafsiri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with one requested change (adding a generic async flag test in core).

Reviewed 4 of 4 files at r1.
Reviewable status: :shipit: complete! 2 of 1 approvals obtained (waiting on @annxingyuan, @dsmilkov, @nsthorat, @pyu10055, and @tafsiri)


tfjs-backend-wasm/src/index_test.ts, line 142 at r1 (raw file):

  });

  it('evaluating async flag works', async () => {

These look good, but when we talked I was thinking you could add a test in core to test a generic async flag. That way whether this code goes away or not, the code paths introduced in core get tested.

Copy link
Contributor Author

@annxingyuan annxingyuan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! 2 of 1 approvals obtained (waiting on @dsmilkov, @nsthorat, @pyu10055, and @tafsiri)


tfjs-backend-wasm/src/index_test.ts, line 142 at r1 (raw file):

Previously, tafsiri (Yannick Assogba) wrote…

These look good, but when we talked I was thinking you could add a test in core to test a generic async flag. That way whether this code goes away or not, the code paths introduced in core get tested.

Done

@annxingyuan annxingyuan merged commit 22b9c53 into master Jun 9, 2020
@annxingyuan annxingyuan deleted the wasm_simd_flag branch June 9, 2020 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants