1354114 - Optimize away more ArrayIteratorNext instructions

Status: NEW

(Core :: JavaScript Engine: JIT, enhancement, P3)

Description

[Tom S [:evilpie]]

for-of on arrays and array destructuring still has a bunch of unnecessary overhead, we need to try and eliminate that in Ion.

See Jan's bug 1353170 comment 10 for ideas.

Comment 1

[Tom S [:evilpie]]

Attachment: func03.pdf

So it seems to me like we don't eliminate the {value, done} object in next.

[Escape] Check object
newobject327 = newobject constant326
resumepoint mode=After (caller in block5) constant291 constant292 newarrayiterator41 typebarrier324 constant293 constant293 constant293 constant293 constant293 newarrayiterator41 newobject327
  at self-hosted:538
  in test.js line 89 > Function:7
[Escape]   is escaped by
phi72 = phi constant64 newobject327
  after test.js line 89 > Function:7

constant64 is undefined and defined before the loop entry

Comment 2

[Jan de Mooij [:jandem]]

I think that's because of the bytecode we emit for for-of. We leave the {value, done} object on the stack and then at the top of the loop we access .value.

arai, would it be possible to restructure this so when we reach the backedge we have VALUE on the stack instead of RESULT? Ideally the RESULT object would not flow into any phis so we can more easily optimize it away in the JIT.

Comment 3

[Tooru Fujisawa [:arai]]

(In reply to Jan de Mooij [:jandem] from comment #2)
> I think that's because of the bytecode we emit for for-of. We leave the
> {value, done} object on the stack and then at the top of the loop we access
> .value.
> 
> arai, would it be possible to restructure this so when we reach the backedge
> we have VALUE on the stack instead of RESULT? Ideally the RESULT object
> would not flow into any phis so we can more easily optimize it away in the
> JIT.

if we can have extra branch at the end of the loop, it's possible to remove RESULT from the stack before backedge.
I'll prepare some draft patch.

Comment 4

[Tooru Fujisawa [:arai]]

something like this:

  symbol 1                        # y y @@iterator
  callelem                        # y ITERFUNC
  swap                            # ITERFUNC y
  calliter 0                      # ITER
  checkisobj 3                    # ITER
  undefined                       # ITER undefined
  goto ENTRY                      # ITER undefined

# loop {

HEAD:
  loophead                        # ITER VALUE
  uninitialized                   # ITER VALUE UNINITIALIZED
  initlexical 0                   # ITER VALUE UNINITIALIZED
  pop                             # ITER VALUE

  try CATCH                       # ITER VALUE
# try {

  ...                             # ITER UNKNOWN

  goto TRY-END                    # ITER UNKNOWN
# } catch {
CATCH:
  jumptarget                      # ITER VALUE
  exception                       # ITER VALUE EXC
  dupat 2                         # ITER VALUE EXC ITER
  undefined                       # ITER VALUE EXC ITER undefined
  strictne                        # ITER VALUE EXC (ITER !== undefined)

  ifeq ITER-END                   # ITER VALUE EXC
# if {
  jumptarget                      # ITER VALUE EXC
  dupat 2                         # ITER VALUE EXC ITER
  dup                             # ITER VALUE EXC ITER ITER
  callprop "return"               # ITER VALUE EXC ITER RETURN
  dup                             # ITER VALUE EXC ITER RETURN RETURN
  undefined                       # ITER VALUE EXC ITER RETURN RETURN undefined
  ne                              # ITER VALUE EXC ITER RETURN (RETURN != undefined)

  ifeq NO-RETURN                  # ITER VALUE EXC ITER RETURN
# if {
  jumptarget                      # ITER VALUE EXC ITER RETURN
  checkiscallable 0               # ITER VALUE EXC ITER RETURN
  swap                            # ITER VALUE EXC RETURN ITER
  undefined                       # ITER VALUE EXC RETURN ITER undefined

  try CATCH2                      # ITER VALUE EXC RETURN ITER undefined
# try {
  dupat 2                         # ITER VALUE EXC RETURN ITER undefined RETURN
  dupat 2                         # ITER VALUE EXC RETURN ITER undefined RETURN ITER
  call 0                          # ITER VALUE EXC RETURN ITER undefined RETURN()
  swap                            # ITER VALUE EXC RETURN ITER RETURN() undefined
  pop                             # ITER VALUE EXC RETURN ITER RETURN()
  goto TRY-END2                   # ITER VALUE EXC RETURN ITER RETURN()
# } catch {
CATCH2:
  jumptarget                      # ITER VALUE EXC RETURN ITER undefined
  exception                       # ITER VALUE EXC RETURN ITER undefined EXC
  pop                             # ITER VALUE EXC RETURN ITER undefined
  nop                             # ITER VALUE EXC RETURN ITER undefined
# }

TRY-END2:
  jumptarget                      # ITER VALUE EXC RETURN ITER merged<RETURN()>
  unpick 2                        # ITER VALUE EXC merged<RETURN()> RETURN ITER
  pop                             # ITER VALUE EXC merged<RETURN()> RETURN
  pop                             # ITER VALUE EXC merged<RETURN()>
  goto HAS-RETURN-END             # ITER VALUE EXC merged<RETURN()>
# } else {

NO-RETURN:
  jumptarget                      # ITER VALUE EXC ITER RETURN
  pop                             # ITER VALUE EXC ITER

# }
HAS-RETURN-END:
  jumptarget                      # ITER VALUE EXC merged<RETURN()>
  pop                             # ITER VALUE EXC
# }

ITER-END:
  jumptarget                      # ITER VALUE EXC
  throw                           # ITER VALUE
  goto TRY-END                    # !!! UNREACHABLE !!!
  nop                             # !!! UNREACHABLE !!!
# }

TRY-END:
  jumptarget                      # ITER UNKNOWN

ENTRY:
  loopentry 129                   # ITER undefined
  pop                             # ITER
  dup                             # ITER ITER
  dup                             # ITER ITER ITER
  callprop "next"                 # ITER ITER NEXFUNC
  swap                            # ITER NEXFUNC ITER
  call 0                          # ITER RESULT
  checkisobj 0                    # ITER RESULT
  dup                             # ITER RESULT RESULT
  getprop "done"                  # ITER RESULT DONE

  ifeq NOT-DONE                   # ITER RESULT
# if {
  pop                             # ITER
  undefined                       # ITER undefined
  true                            # ITER undefined TRUE
# } else {

NOT-DONE:
  getprop "value"                 # ITER VALUE
  false                           # ITER VALUE FALSE
# }

  ifeq HEAD                       # ITER merged<VALUE>
# }

DONE:
  jumptarget                      # ITER merged<VALUE>
  pop                             #
  pop                             #

Comment 5

[Tooru Fujisawa [:arai]]

forgot "goto" after "true" in then-branch :P

Comment 6

[Tooru Fujisawa [:arai]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2723c085182b2a5f1256ea54c4983fe663e0d414
(maybe I should file a new bug for this specific change?)

Comment 7

[Tooru Fujisawa [:arai]]

I'll fix in bug 1354554

Comment 8

[Tom S [:evilpie]]

Thanks to bug 1354554 we are now almost as fast as v8 on this benchmark. There is still some branches inside the loop and we now introduced unnecessary boxing for the iterator value.

Comment 9

[Tom S [:evilpie]]

So there two branches here that we should definitely optimize way.

For the first I already have a patch: We ended up with a compare typebarrier constant, where typebarrier was wrapping another int32 constant. I think we can just constant fold MTyperBarriers with a constant.

The second branch that is coming from ToLength: We get a branch for the the if (v <= 0). This is a bit silly, we have int32 so we don't have to worry about -0, and the v is an arraylength node so we already know it can't be < 0, just = 0.