Closed Bug 739854 Opened 9 years ago Closed 9 years ago

IonMonkey: Remove negative zero check on MToInt32

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: h4writer, Assigned: h4writer)

Details

Attachments

(1 file, 2 obsolete files)

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.)
Attached patch WIP (obsolete) — Splinter Review
Attached patch WIP2 (obsolete) — Splinter Review
Includes MStoreElement and friends
Assignee: general → hv1989
Attachment #609967 - Attachment is obsolete: true
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)
Attachment #610082 - Flags: review?(mrosenberg)
Attachment #610082 - Attachment is patch: true
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+
https://hg.mozilla.org/projects/ionmonkey/rev/2c7e9bd43480
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.