Bug 48123 – Do not initialize to undefined variables with an initializer

• Status: RESOLVED
• Resolution: INVALID
• Component: JavaScriptCore

[Xan Lopez]

Playing a bit with the bytecode dumping option of jsc I found that we always generate mov operations to initialize variables to undefined, even if immediately after that they'll be set another value.

For example, for the code:

var a = 1; var b; var c = 2;

we get:

8 m_instructions; 84 bytes at 0x82f1c00; 1 parameter(s); 1 callee register(s)

[   0] enter
[   1] mov		 r-13, undefined(@k0)
[   4] mov		 r-14, undefined(@k0)
[   7] mov		 r-15, undefined(@k0)
[  10] mov		 r0, undefined(@k0)
[  13] mov		 r-13, 1(@k1)
[  16] mov		 r-15, 2(@k2)
[  19] end		 r0

Where the second and fourth instructions seem to be redundant.

I patched BytecodeGenerator to not do this, so now we get:


6 m_instructions; 60 bytes at 0x82f1c00; 1 parameter(s); 1 callee register(s)

[   0] enter
[   1] mov		 r-14, undefined(@k0)
[   4] mov		 r0, undefined(@k0)
[   7] mov		 r-13, 1(@k1)
[  10] mov		 r-15, 2(@k2)
[  13] end		 r0


for the same code.

All tests seem to still pass, although I don't see any gain (or loss) of performance in SunSpider:


TEST                   COMPARISON            FROM                 TO             DETAILS

=============================================================================

** TOTAL **:           -                 732.3ms +/- 0.1%   731.8ms +/- 0.2% 

=============================================================================

  3d:                  ??                 95.1ms +/- 0.4%    95.4ms +/- 0.5%     not conclusive: might be *1.003x as slow*
    cube:              ??                 31.3ms +/- 1.3%    31.4ms +/- 1.2%     not conclusive: might be *1.003x as slow*
    morph:             ??                 28.4ms +/- 0.5%    28.5ms +/- 0.7%     not conclusive: might be *1.004x as slow*
    raytrace:          ??                 35.5ms +/- 0.4%    35.5ms +/- 0.5%     not conclusive: might be *1.001x as slow*

  access:              -                 103.2ms +/- 0.3%   103.1ms +/- 0.3% 
    binary-trees:      ??                  9.9ms +/- 0.8%    10.0ms +/- 0.8%     not conclusive: might be *1.004x as slow*
    fannkuch:          -                  50.3ms +/- 0.3%    50.3ms +/- 0.3% 
    nbody:             -                  28.5ms +/- 0.5%    28.4ms +/- 0.5% 
    nsieve:            -                  14.5ms +/- 1.1%    14.5ms +/- 1.0% 

  bitops:              -                  67.9ms +/- 0.4%    67.7ms +/- 0.3% 
    3bit-bits-in-byte: -                   9.0ms +/- 0.4%     9.0ms +/- 0.4% 
    bits-in-byte:      ??                 17.9ms +/- 0.6%    17.9ms +/- 0.4%     not conclusive: might be *1.001x as slow*
    bitwise-and:       -                  15.6ms +/- 1.1%    15.5ms +/- 1.0% 
    nsieve-bits:       -                  25.4ms +/- 0.6%    25.3ms +/- 0.5% 

  controlflow:         1.027x as fast      7.6ms +/- 1.9%     7.4ms +/- 1.9%     significant
    recursive:         1.027x as fast      7.6ms +/- 1.9%     7.4ms +/- 1.9%     significant

  crypto:              -                  44.8ms +/- 0.5%    44.7ms +/- 0.6% 
    aes:               -                  26.1ms +/- 0.4%    26.1ms +/- 0.7% 
    md5:               -                  10.2ms +/- 1.2%    10.1ms +/- 1.0% 
    sha1:              -                   8.5ms +/- 1.7%     8.5ms +/- 1.8% 

  date:                -                  81.2ms +/- 0.4%    81.2ms +/- 0.4% 
    format-tofte:      -                  42.9ms +/- 0.5%    43.0ms +/- 0.6% 
    format-xparb:      -                  38.3ms +/- 0.6%    38.2ms +/- 0.6% 

  math:                ??                 76.2ms +/- 0.3%    76.3ms +/- 0.3%     not conclusive: might be *1.001x as slow*
    cordic:            ??                 23.5ms +/- 0.6%    23.6ms +/- 0.6%     not conclusive: might be *1.003x as slow*
    partial-sums:      ??                 39.3ms +/- 0.3%    39.4ms +/- 0.4%     not conclusive: might be *1.001x as slow*
    spectral-norm:     -                  13.4ms +/- 1.1%    13.4ms +/- 1.1% 

  regexp:              -                  31.6ms +/- 0.5%    31.6ms +/- 0.5% 
    dna:               -                  31.6ms +/- 0.5%    31.6ms +/- 0.5% 

  string:              -                 224.8ms +/- 0.2%   224.4ms +/- 0.2% 
    base64:            ??                 26.5ms +/- 0.5%    26.6ms +/- 0.6%     not conclusive: might be *1.005x as slow*
    fasta:             1.009x as fast     31.7ms +/- 0.6%    31.4ms +/- 0.6%     significant
    tagcloud:          -                  55.5ms +/- 0.3%    55.3ms +/- 0.3% 
    unpack-code:       -                  81.0ms +/- 0.3%    80.8ms +/- 0.3% 
    validate-input:    ??                 30.0ms +/- 0.5%    30.2ms +/- 0.5%     not conclusive: might be *1.005x as slow*

Unsure if this is useful or even against some design decision, but since I did it I'll attach the patch.

[Xan Lopez]

Created attachment 71550 [details]
initundefined.diff

[Oliver Hunt]

Comment on attachment 71550 [details]
initundefined.diff

This isn't actually safe, as you need control flow analysis to know whether the variable will be accessed prior to initialisation:
<script>
alert(x);
var x = 5;
</script>

x is used prior to be defined but it is not initialised until later.  You also can't hoist the initialiser as it may depend on prior computation, eg.

y=10; 
var x=y;

etc.

[Xan Lopez]

Comment on attachment 71550 [details]
initundefined.diff

Ah, of course. Marking as obsolete. I wonder if there should be some kind of test that checks that variables have the undefined value prior when used prior to initalization?

[Xan Lopez]

Closing as INVALID.