Last Comment Bug 739854 - IonMonkey: Remove negative zero check on MToInt32
: IonMonkey: Remove negative zero check on MToInt32
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: x86_64 Linux
: -- normal (vote)
: ---
Assigned To: Hannes Verschore [:h4writer]
: Jason Orendorff [:jorendorff]
Depends on:
  Show dependency treegraph
Reported: 2012-03-27 17:36 PDT by Hannes Verschore [:h4writer]
Modified: 2012-03-29 14:22 PDT (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

WIP (11.98 KB, patch)
2012-03-27 17:37 PDT, Hannes Verschore [:h4writer]
no flags Details | Diff | Splinter Review
WIP2 (12.97 KB, patch)
2012-03-27 18:03 PDT, Hannes Verschore [:h4writer]
no flags Details | Diff | Splinter Review
Remove negative zero checks on ToInt32 (16.11 KB, patch)
2012-03-28 05:23 PDT, Hannes Verschore [:h4writer]
dvander: review+
marty.rosenberg: review+
Details | Diff | Splinter Review

Description Hannes Verschore [:h4writer] 2012-03-27 17:36:27 PDT
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.)
Comment 1 Hannes Verschore [:h4writer] 2012-03-27 17:37:41 PDT
Created attachment 609967 [details] [diff] [review]
Comment 2 Hannes Verschore [:h4writer] 2012-03-27 18:03:37 PDT
Created attachment 609971 [details] [diff] [review]

Includes MStoreElement and friends
Comment 3 Hannes Verschore [:h4writer] 2012-03-28 05:23:35 PDT
Created attachment 610082 [details] [diff] [review]
Remove negative zero checks on ToInt32

- 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?
Comment 4 Marty Rosenberg [:mjrosenb] 2012-03-28 11:51:05 PDT
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"
Comment 5 Hannes Verschore [:h4writer] 2012-03-29 14:22:59 PDT

Note You need to log in before you can comment on or make changes to this bug.