IonMonkey: Remove negative zero check on MToInt32

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: h4writer, Assigned: h4writer)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

6 years ago
Same as bug 736135 but now for MToInt32. When converting MToInt32 from double to int we do a negative zero check. This is not needed when we know that all uses consume a negative zero as it was a normal 0.

(mjrosenb also suggested  array index (MLoadElement and MStoreElement) and toString. So I'll add them when verified.)
(Assignee)

Comment 1

6 years ago
Created attachment 609967 [details] [diff] [review]
WIP
(Assignee)

Comment 2

6 years ago
Created attachment 609971 [details] [diff] [review]
WIP2

Includes MStoreElement and friends
Assignee: general → hv1989
Attachment #609967 - Attachment is obsolete: true
(Assignee)

Comment 3

6 years ago
Created attachment 610082 [details] [diff] [review]
Remove negative zero checks on ToInt32

Changelog:
- Adds canBeNegativeZero boolean to MToInt32 (similar to MDiv and MMul)
- Use "NeedNegativeZeroCheck" to remove unneeded negative zero checks on MToInt32
- Improved "NeedNegativeZeroCheck"; Following MIR's added:
+          case MDefinition::Op_StoreElementHole:
+          case MDefinition::Op_LoadElementHole:
+          case MDefinition::Op_StoreElement:
+          case MDefinition::Op_LoadElement:
+          case MDefinition::Op_LoadTypedArrayElement:
+          case MDefinition::Op_LoadTypedArrayElementHole:
+          case MDefinition::Op_CharCodeAt:
+          case MDefinition::Op_BoundsCheck:
+          case MDefinition::Op_ToString:
+          case MDefinition::Op_FromCharCode:
+          case MDefinition::Op_TableSwitch:
- x86: Fixed issue where "-0" would go to the default case instead of the "0" case (testcase added)
- arm: Fixed issue where "0" and "-0" would go to default case instead of "0" case (testcase added)

@mjrosenb: could you check arm parts?
Attachment #609971 - Attachment is obsolete: true
Attachment #610082 - Flags: review?(dvander)
(Assignee)

Updated

6 years ago
Attachment #610082 - Flags: review?(mrosenberg)
Attachment #610082 - Attachment is patch: true

Updated

6 years ago
Summary: Remove negative zero check on MToInt32 → IonMonkey: Remove negative zero check on MToInt32
Comment on attachment 610082 [details] [diff] [review]
Remove negative zero checks on ToInt32

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

The ARM bits look good to me.

::: js/src/ion/arm/CodeGenerator-arm.cpp
@@ +923,1 @@
>      // guard for /= 0.

This should go inside the block.  You may also want to change it to "!= 0"
Attachment #610082 - Flags: review?(mrosenberg) → review+
Attachment #610082 - Flags: review?(dvander) → review+
(Assignee)

Comment 5

6 years ago
https://hg.mozilla.org/projects/ionmonkey/rev/2c7e9bd43480
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.