Open Bug 1388720 Opened 7 years ago Updated 1 year ago

MIR: Improve virtual constant accesses.

Categories

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

enhancement

Tracking

()

People

(Reporter: nbp, Unassigned)

References

(Blocks 1 open bug)

Details

Currently, we spend ~1.5% of the compilation time calling MIR functions such as op, numOperands, getAliasSet, possiblyCall, canRecoverOnBailout, numSucessors, …  These functions are virtual because it depends on the MIR or some of its fields content and most of the time they are returning simple constants (when not always).

Ideally we should be able to store these constants in the vtable, and just do 1 memory reads to get the constants, and avoid a read, a call and 5 instructions (-fno-omit-frame-pointer for profiling reasons).

Unfortunately, C++ does not provide any way to store anything else than function pointers in the vtable.  The alternative would be to add our own virtual table to store these constants.

Today, our Mir instructions actually have a lot of functions which are returning either constants, or values which are only set at the creation.  One idea would be to create a structure which contains:

 - MNode::Kind
 - MNode::numOperands (*)(**)
 - MDefinitopn::Opcode
 - MDefinition::getAliasSet (*)
 - MDefinition::isControlInstruction
 - MDefinition::canRecoverOnBailout (*)
 - …

Unfortunately, this approach cost an extra pointer on our MIR instructions.

(*) Some of these fields are not constants, but if we allow to have a special value to represent the fact that the value is computed dynamically instead of being available from the constant.  Then we can separate the problem into:

  best case:
    1 reads, 1 branch (+ special value load?)
  worst case:
    2 reads, 1 branch (+ special value load?), 1 call, + x instructions (x >= 5)

(**) This might be a problem for example with MResumePoint::numOperands or MPhi.  Up to the point where it might as well be interesting to dynamically create some of these "virtual constant table" to just include whatever numbers they have, and share them between operations with the same number of operands.

Note, all the MPhi of a basic block have the same number of operands.
Also, all ResumePoint, except for stack depth variations have likely a number of operands found in a previous resume point.
See Also: → 1392530
Priority: -- → P3
Severity: normal → S3

It looks like we plucked the lowest-hanging fruit in bug 1392530.

I'm bumping the priority down on this, because we're not currently seeing Ion compilation time as a performance hotspot. In the long run, it would still be nice to devirtualize MIR. Another approach that we've had success with for things like CacheIR is to replace a virtual call with a big switch statement keyed on the op.

Blocks: sm-opt-jits
No longer blocks: 1245279
Priority: P3 → P5
You need to log in before you can comment on or make changes to this bug.