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

Asm.js Simd.js : Fix for simd shr sequence cloberring the source reg value #941

Merged
merged 2 commits into from
May 7, 2016

Conversation

Krovatkin
Copy link
Collaborator

No description provided.

@Krovatkin
Copy link
Collaborator Author

fixes #940

@Krovatkin
Copy link
Collaborator Author

@nmostafa FYI a PR submitted for #940 (msft) / # 95 (intel-chakra)

@Krovatkin Krovatkin changed the title Fix for shr sequence cloberring the source reg value Asm.js Simd.js : Fix for simd shr sequence cloberring the source reg value May 6, 2016
@Krovatkin Krovatkin force-pushed the b95_onto_r12_test branch from 0394339 to 2a3c12f Compare May 6, 2016 18:57
productizing the fix

making const the second opnd

fixing unit test breaks

reducing the change

rename vars

fix a blank

better comments

unit test

adding copyright

Relax shift amount type to be Int instead of Unsigned
@Krovatkin Krovatkin force-pushed the b95_onto_r12_test branch from 2a3c12f to 795bcdc Compare May 6, 2016 20:25
@Krovatkin Krovatkin force-pushed the b95_onto_r12_test branch from 181ce23 to 38f26c9 Compare May 6, 2016 20:37
@nmostafa
Copy link
Contributor

nmostafa commented May 6, 2016

@MikeHolman , can you review this ?

@@ -1661,8 +1661,8 @@ namespace Js
simdFunctions[AsmJsSIMDBuiltin_int8x16_not] = SIMDFunc(PropertyIds::not, Anew(&mAllocator, AsmJsSIMDFunction, nullptr, &mAllocator, 1, AsmJsSIMDBuiltin_int8x16_not, OpCodeAsmJs::Simd128_Not_I16, AsmJsRetType::Int8x16, AsmJsType::Int8x16));
simdFunctions[AsmJsSIMDBuiltin_int8x16_min] = SIMDFunc(PropertyIds::min, Anew(&mAllocator, AsmJsSIMDFunction, nullptr, &mAllocator, 2, AsmJsSIMDBuiltin_int8x16_min, OpCodeAsmJs::Simd128_Min_I16, AsmJsRetType::Int8x16, AsmJsType::Int8x16, AsmJsType::Int8x16));
simdFunctions[AsmJsSIMDBuiltin_int8x16_max] = SIMDFunc(PropertyIds::max, Anew(&mAllocator, AsmJsSIMDFunction, nullptr, &mAllocator, 2, AsmJsSIMDBuiltin_int8x16_max, OpCodeAsmJs::Simd128_Max_I16, AsmJsRetType::Int8x16, AsmJsType::Int8x16, AsmJsType::Int8x16));
simdFunctions[AsmJsSIMDBuiltin_int8x16_shiftLeftByScalar] = SIMDFunc(PropertyIds::shiftLeftByScalar, Anew(&mAllocator, AsmJsSIMDFunction, nullptr, &mAllocator, 2, AsmJsSIMDBuiltin_int8x16_shiftLeftByScalar, OpCodeAsmJs::Simd128_ShLtByScalar_I16, AsmJsRetType::Int8x16, AsmJsType::Int8x16, AsmJsType::Unsigned));
simdFunctions[AsmJsSIMDBuiltin_int8x16_shiftRightByScalar] = SIMDFunc(PropertyIds::shiftRightByScalar, Anew(&mAllocator, AsmJsSIMDFunction, nullptr, &mAllocator, 2, AsmJsSIMDBuiltin_int8x16_shiftRightByScalar, OpCodeAsmJs::Simd128_ShRtByScalar_I16, AsmJsRetType::Int8x16, AsmJsType::Int8x16, AsmJsType::Unsigned));
simdFunctions[AsmJsSIMDBuiltin_int8x16_shiftLeftByScalar] = SIMDFunc(PropertyIds::shiftLeftByScalar, Anew(&mAllocator, AsmJsSIMDFunction, nullptr, &mAllocator, 2, AsmJsSIMDBuiltin_int8x16_shiftLeftByScalar, OpCodeAsmJs::Simd128_ShLtByScalar_I16, AsmJsRetType::Int8x16, AsmJsType::Int8x16, AsmJsType::Int));
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this conflict with #934?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see how. We are relaxing the shift amount type from Unsigned to Int. #934 allows coercion of SIMD return types.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah you're right, was misreading that as the ret type!

@MikeHolman
Copy link
Contributor

lgtm

@chakrabot chakrabot merged commit 38f26c9 into chakra-core:release/1.2 May 7, 2016
chakrabot pushed a commit that referenced this pull request May 7, 2016
…e source reg value

Merge pull request #941 from Krovatkin:b95_onto_r12_test
chakrabot pushed a commit that referenced this pull request May 10, 2016
…cloberring the source reg value

Merge pull request #941 from Krovatkin:b95_onto_r12_test
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.

5 participants