1112563 - Use useRegisterAtStart for LoadSlot/LoadFixedSlot/GetDOMMember

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Jan de Mooij [:jandem]]

Attachment: Patch

The main worry is when we load a Value on 32-bit platforms (we emit two loads and we don't want the first one to clobber our object/slots register), but fortunately masm.loadValue already knows how to deal with this (it just swaps the loads) so this should be safe.

Comment 1

[Dan Gohman [:sunfish]]

Comment on attachment 8537809 [details] [diff] [review]
Patch

Review of attachment 8537809 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. Can LStoreFixedSlotV and friends use AtStart too?

Comment 2

[Jan de Mooij [:jandem]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cfc612739ad7

(In reply to Dan Gohman [:sunfish] from comment #1)
> Can LStoreFixedSlotV and friends use AtStart too?

I think so, but what'd be the benefit if they have no defs/temps? We've been pretty conservative with *AtStart to do the safe thing by default..

Comment 3

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/cfc612739ad7

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Follow-up fix

bz noticed that codegen is slightly worse on x64 when we're loading an object and reusing the input register (extra move to scratch register). This patch fixes that by using useRegister in this case. x64 has more registers so the optimization is less useful there anyway.

Comment 5

[Dan Gohman [:sunfish]]

Comment on attachment 8538493 [details] [diff] [review]
Follow-up fix

Review of attachment 8538493 [details] [diff] [review]:
-----------------------------------------------------------------

Makes sense.

Comment 6

[Jan de Mooij [:jandem]]

Part 2:

https://hg.mozilla.org/integration/mozilla-inbound/rev/e4f5b3825bbb

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e4f5b3825bbb