Open Bug 1205524 Opened 4 years ago Updated 3 years ago

IonMonkey: MIPS: Use ScratchRegisterScope


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




Tracking Status
firefox43 --- affected


(Reporter: hev, Assigned: hev)



Replace all ScratchRegister, SecondScratchReg by scratch register scope.
Depends on: 1140954
I have a trouble trying use AutoRegisterScope. The func3 call func2, then func2 call func1 at below. The trouble is get ScratchRegister failed in func1, looks it conflict to scratch register in allocated in func2. In fact, I think the two ScratchRegsiters are not conflicted. In other words, I think the tracking of AutoRegisterScope is too strict. How to use AutoRegisterScope on MIPS? Thanks!

// func1
void branchTest32(Condition cond, Register lhs, Register rhs, Label* label) {
    MOZ_ASSERT(cond == Zero || cond == NonZero || cond == Signed || cond == NotSigned);
    if (lhs == rhs) {
        ma_b(lhs, rhs, label, cond);
    } else {
        ScratchRegisterScope scratch(asMasm());
        as_and(scratch, lhs, rhs);
        ma_b(scratch, scratch, label, cond);
// func2
void branchTest32(Condition cond, Register lhs, Imm32 imm, Label* label) {
    ScratchRegisterScope scratch(asMasm());
    ma_li(scratch, imm);
    branchTest32(cond, lhs, scratch, label);
// func3
void branchTest32(Condition cond, const Address& address, Imm32 imm, Label* label) {
    SecondScratchRegisterScope scratch(asMasm());
    load32(address, scratch);
    branchTest32(cond, scratch, imm, label);
Flags: needinfo?(sstangl)
AutoRegisterScope is indeed too strict.

We also have this problem on ARM, being tracked in Bug 1206652. There is a patch to provide a clobberable scratch register class, so you could pass scratch into func1 and func1 could acquire it separately internally, but nbp doesn't like it.

Since that bug isn't making progress, I would just change func1 to use ScratchRegister directly (and not ScratchRegisterScope), while leaving a "TODO" comment referencing Bug 1206652.
Flags: needinfo?(sstangl)
Priority: -- → P5
You need to log in before you can comment on or make changes to this bug.