Closed Bug 914899 Opened 6 years ago Closed 6 years ago

20% kraken-crypto-ccm regression

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: shu, Assigned: shu)

References

Details

Attachments

(1 file, 2 obsolete files)

Splitting from bug 914478.
Attached patch bug905986.patch (obsolete) — Splinter Review
Regression was caused by the refactoring of value-to-int32 logic not keeping an OOL double truncation path.
Assignee: general → shu
Attachment #802673 - Flags: review?(jdemooij)
Comment on attachment 802673 [details] [diff] [review]
bug905986.patch

Attached the wrong patch...
Attachment #802673 - Attachment is obsolete: true
Attachment #802673 - Flags: review?(jdemooij)
Attached patch bug914899.patch (obsolete) — Splinter Review
Regression was caused by the refactoring of value-to-int32 logic not keeping an OOL double truncation path.
Attachment #802678 - Flags: review?(jdemooij)
Attached patch v2Splinter Review
Fix typo.
Attachment #802678 - Attachment is obsolete: true
Attachment #802678 - Flags: review?(jdemooij)
Attachment #802701 - Flags: review?(jdemooij)
Comment on attachment 802701 [details] [diff] [review]
v2

Review of attachment 802701 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for tracking this down!

I'm a bit worried about code size; we should move the OOL truncate code to a shared stub (bug 818750) and call it instead but that doesn't need to happen in this bug.

::: js/src/jit/CodeGenerator.cpp
@@ +6794,5 @@
>      Label fails;
>      if (input->mightBeType(MIRType_String)) {
> +        OutOfLineCode *oolDouble = oolTruncateDouble(tempFloat, output);
> +        if (!oolDouble)
> +            return false;

Why do we only need the truncate path in the might-be-string case?
Attachment #802701 - Flags: review?(jdemooij) → review+
(In reply to Jan de Mooij [:jandem] from comment #5)
> Comment on attachment 802701 [details] [diff] [review]
> v2
> 
> Review of attachment 802701 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thanks for tracking this down!
> 
> I'm a bit worried about code size; we should move the OOL truncate code to a
> shared stub (bug 818750) and call it instead but that doesn't need to happen
> in this bug.
> 
> ::: js/src/jit/CodeGenerator.cpp
> @@ +6794,5 @@
> >      Label fails;
> >      if (input->mightBeType(MIRType_String)) {
> > +        OutOfLineCode *oolDouble = oolTruncateDouble(tempFloat, output);
> > +        if (!oolDouble)
> > +            return false;
> 
> Why do we only need the truncate path in the might-be-string case?

Oops, that's a mistake.
https://hg.mozilla.org/mozilla-central/rev/c05f8dce65ca
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Tracked some crashes on an ARM debug build to the OOL OutOfLineTruncateSlow
code, for which the return branch is bad.  Might the return label not be
bound in visitClampVToUint8?
Depends on: 916755
You need to log in before you can comment on or make changes to this bug.