648526 - Emit JSOP_OBJECT when array initializer contains negative numbers

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jan de Mooij [:jandem]]

Noticed this when reducing some testcase:
--
js> [1]; dis()  
main:
00000:  object [1]
00003:  popv
--
And this:
--
js> [-1]; dis()    
main:
00000:  newarray 1
00004:  zero
00005:  int8 -1
00007:  initelem
00008:  endinit
--
The problem seems to be that JSParseNode::getConstantValue does not support negative numbers.

I don't know if this will win performance somewhere. SS base64 uses an array with negative numbers though and some benchmarks like to use arrays as vectors like this: [-1, 0, 1].

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Patch

Brian asking your r? because you wrote JSParseNode::isConstant. The problem was that -10 is constant folded to one token after parsing the initializer.

Comment 2

[Brian Hackett [Laid off!]]

This will I think break in JSParseNode::getConstantValue if it is not also updated.  Can you add a testcase or update tests/basic/testInitSingleton?

Comment 3

[Tom S. (please needinfo tschuster)]

Attachment: do FoldConstants for every node

Brain is this a sensible approach to do this, or is there some reason that we can't doing folding there?

Comment 4

[Brian Hackett [Laid off!]]

Comment on attachment 584467 [details] [diff] [review]
do FoldConstants for every node

I think FoldConstants is fine to use here, and should be more robust.

Comment 5

[Tom S. (please needinfo tschuster)]

Attachment: Call FoldConstants of every node.

Finally came around finishing this. I know the tests are kinda dense, but I hope we test constant folding in enough other places.

Comment 6

[Tom S. (please needinfo tschuster)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/865c938824b1

Comment 7

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/865c938824b1