Open
Bug 1073058
Opened 10 years ago
Updated 3 months ago
start using |use| rather than |useRegister|, and |useAtStart| rather than |useRegisterAtStart|
Categories
(Core :: JavaScript Engine: JIT, enhancement, P5)
Tracking
()
NEW
People
(Reporter: mjrosenb, Unassigned)
References
(Blocks 1 open bug)
Details
http://hg.mozilla.org/mozilla-central/annotate/e9e56750ca5b/js/src/jit/Lowering.cpp#l1775 is in architecture-independent code, and I suspect was hard-coded to call |useRegister|, because Arm instructions in general, cannot operate on memory. However, our LIR frequently maps to multiple instructions, as LIntToFloat32 does. In this case, if there is an int32 on the stack, which will be converted to a Float32/Double, the useRegister will make us generate code that looks like this:
> ldr r0, [sp, #56]
> vmov s6, r0
> vcvt.f32.i32 s6, s6
We may actually use the scratch register, or something like that, but the important things are: the gpr (r0) is *only* used to shuffle data over to the FPU, and because we're using an extra GPR, this has the ability to forcibly evict a register that is being used.
If we made this into just use(opd), or possibly useAtStart(opd), then the LInstruction can pretend that it handles memory operands, and instead generate
> vldr s6, [sp, #56]
> vcvt.f32.i32 s6, s6
If I get to this, it won't be until next week at the earliest. Hopefully, x86 and x64 will "just work" if these instructions are given an Operand, rather than a Register.
Comment 1•10 years ago
|
||
(In reply to Marty Rosenberg [:mjrosenb] from comment #0)
> http://hg.mozilla.org/mozilla-central/annotate/e9e56750ca5b/js/src/jit/
> Lowering.cpp#l1775 is in architecture-independent code, and I suspect was
> hard-coded to call |useRegister|, because Arm instructions in general,
> cannot operate on memory.
The x86 and x64 path also has useRegister{AtStart} for this path.
> However, our LIR frequently maps to multiple
> instructions, as LIntToFloat32 does. In this case, if there is an int32 on
> the stack, which will be converted to a Float32/Double, the useRegister will
> make us generate code that looks like this:
>
> > ldr r0, [sp, #56]
> > vmov s6, r0
> > vcvt.f32.i32 s6, s6
>
> We may actually use the scratch register, or something like that, but the
> important things are: the gpr (r0) is *only* used to shuffle data over to
> the FPU, and because we're using an extra GPR, this has the ability to
> forcibly evict a register that is being used.
> If we made this into just use(opd), or possibly useAtStart(opd), then the
> LInstruction can pretend that it handles memory operands, and instead
> generate
>
> > vldr s6, [sp, #56]
> > vcvt.f32.i32 s6, s6
>
> If I get to this, it won't be until next week at the earliest. Hopefully,
> x86 and x64 will "just work" if these instructions are given an Operand,
> rather than a Register.
Interesting, but does 'use' versus 'useRegister' also give a hint to the register allocator that there is little cost in having the operand on the stack? If so then the change might bias the register allocator to generator worse code for the ARM?
Can the optimization be addressed in the move emitter? Does it know that it has to move a stack value to a vfp register and optimize it as above?
Reporter | ||
Comment 2•10 years ago
|
||
> Interesting, but does 'use' versus 'useRegister' also give a hint to the
> register allocator that there is little cost in having the operand on the
> stack? If so then the change might bias the register allocator to generator
> worse code for the ARM?
>
unclear. I don't /think/ that is something that is done, but I may be wrong.
> Can the optimization be addressed in the move emitter? Does it know that it
> has to move a stack value to a vfp register and optimize it as above?
That sounds hard. We'd need to get information about how each (or at least some) registers are used into the move emitter in order to do this, and it sounds like it would be quite fragile.
Updated•8 years ago
|
Priority: -- → P5
Updated•2 years ago
|
Severity: normal → S3
Updated•3 months ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•