1459038 - Deduplicate CacheIR emitters that differ only in stub fields

Status: RESOLVED FIXED

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

Description

[Matthew Gaudet (he/him) [:mgaudet]]

Once Bug 1348792 lands, we will be able to merge the Baseline and Ion CacheIR compiler code for opcodes whose implementations differ only in terms of how they handle stub fields.

Comment 1

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Deduplicate MegamorphicLoadSlotResult using EmitLoadStubField

Comment 2

[Matthew Gaudet (he/him) [:mgaudet]]

I took a pass through and have highlighted the below methods as either perfect for deduplication, or almost perfect. 

Perfect cases for deduplication

* emitGuardGroupHasUnanalyzedNewScript
* emitGuardXrayExpandoShapeAndDefaultProto 
* emitGuardFunctionPrototype l
* emitMegamorphicLoadSlotResult (Bug 1460895) 
* emitMegamorphicStoreSlotResult
* emitLoadObject

Look unifiable, though may have require one extra scratch reg:

* emitGuardShape 
* emitGuardGroup 
* emitLoadUnboxedPropertyResult 
* emitLoadEnvironmentFixedSlotResult
* emitLoadEnvironmentDynamicSlotResult

Assuming Bug 1460895 is a good example, I'll follow up opening bugs for the perfect cases above. 

Jan: About the deduplications where we'd need an extra scratch register (and immediate load). What is your feeling: Worth it in terms of deduplication value, or potentially a performance concern?

Comment 3

[Jan de Mooij [:jandem]]

(In reply to Matthew Gaudet (he/him) [:mgaudet] (UTC-5) from comment #2)
> Jan: About the deduplications where we'd need an extra scratch register (and
> immediate load). What is your feeling: Worth it in terms of deduplication
> value, or potentially a performance concern?

For the ones you listed I'd say it's probably not worth it because they're all very simple instructions. Also, emitGuardShape in particular is very hot because most property access stubs contain at least one shape guard, so adding an extra load + scratch register isn't great :)

Unifying the others sounds great though!

Comment 4

[Matthew Gaudet (he/him) [:mgaudet]]

I think it's possible to deduplicate things like emitGuardShape, with some more magic. But I don't have a clear answer right now, so I won't open a blocking bug for that.

Comment 5

[Matthew Gaudet (he/him) [:mgaudet]]

All the dependent bugs are closed, so closing this.