Open
Bug 1103311
Opened 10 years ago
Updated 2 years ago
Pointless register move output when calling a DOM getter that was found on the receiver
Categories
(Core :: JavaScript Engine: JIT, defect, P5)
Tracking
()
NEW
People
(Reporter: bzbarsky, Unassigned)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
1.31 KB,
patch
|
Details | Diff | Splinter Review | |
3.01 KB,
patch
|
Details | Diff | Splinter Review |
Consider the situation in which we're in IonBuilder::getPropTryCommonGetter, "obj" is a singleton, and foundProto comes back as the same object as obj. In other words, the window.foo case on the web. ;) Here's what JIT inspector says happens: [Pointer:GC_THING] movabsq $0x134ae0060, %rax [GuardShape] movabsq $0x12796c358, %r11 cmpq %r11, 0(%rax) jne ((1562)) [Pointer:GC_THING] movabsq $0x134ae0060, %rax That's because we have an MConstant that's pointed to by both the GuardShape and the actual getter MInstruction. Lowering converts this to an LPointer, but a new LPointer each time the MInstruction is seen. So we end up with a pointless movabsq there. The question is what we can do about this... I'm really not sure. It _looks_ annoying, but not sure how much it matters in practice.
Comment 1•10 years ago
|
||
Here's a quick patch with one possible approach: emitting code for pointer constants on x64 at their definition, rather than at each use. The downside is that this increases register pressure in some cases.
Comment 2•10 years ago
|
||
The above patch also causes regressions due to a bunch of constants getting materialized in registers that aren't actually needed. This patch fixes one such case. The StringSplit code builds an MConstant to hold the template object, but the MConstant itself isn't actually used. Attached is a patch which eliminates the MConstant.
Reporter | ||
Comment 3•10 years ago
|
||
Yeah, globally changing how constants work seems like a pretty big hammer for this problem, possibly out of proportion to the size of the problem... Not sure whether there are smaller hammers here, though.
Reporter | ||
Updated•9 years ago
|
Blocks: dom-requests
Updated•8 years ago
|
Depends on: sm-js-perf
Priority: -- → P5
Updated•7 years ago
|
No longer depends on: sm-js-perf
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•