Closed Bug 880471 Opened 7 years ago Closed 7 years ago

IonMonkey: Remove temp. variable for x86/arm in TypeBarrier

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla24

People

(Reporter: h4writer, Assigned: h4writer)

Details

Attachments

(1 file)

Typebarrier requests a temporary variable for TypeBarrier. This isn't needed on x86/arm so this increases register pressure on those architectures. Should be really easy to use a BogusTemp instead
Assignee: general → hv1989
Attached patch PatchSplinter Review
I introduced tempToSplitValue() that can be used during lowering for a temp that will only be used for splitting a Value into a payload or a type. This will return a BogusTemp on x86/arm since there we don't need a temp to split a value and a normal temp on x64.

This will probably only introduce an improvement on x86, since register pressure is worst there. Octane isn't blocked on this, but I saw a 2% improvement on ss-cordic
Attachment #761037 - Flags: review?(jdemooij)
Comment on attachment 761037 [details] [diff] [review]
Patch

Review of attachment 761037 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/ion/shared/CodeGenerator-shared-inl.h
@@ +45,5 @@
>      return ToRegister(*def->output());
>  }
>  
>  static inline Register
> +ToSplitRegister(const LDefinition *def)

Something like ToUnboxTempRegister is a bit clearer I think.

::: js/src/ion/x86/Lowering-x86.h
@@ +25,5 @@
>      bool useBox(LInstruction *lir, size_t n, MDefinition *mir,
>                  LUse::Policy policy = LUse::REGISTER, bool useAtStart = false);
>      bool useBoxFixed(LInstruction *lir, size_t n, MDefinition *mir, Register reg1, Register reg2);
>  
> +    inline LDefinition tempToSplitValue() {

Nit: s/Split/Unbox here as well.
Attachment #761037 - Flags: review?(jdemooij) → review+
(In reply to Jan de Mooij [:jandem] from comment #2)
> >  static inline Register
> > +ToSplitRegister(const LDefinition *def)
> 
> Something like ToUnboxTempRegister is a bit clearer I think.

But also incorrect, since it can also get used to split the type and unboxing refer to getting the payload only... That's why I didn't take anything with "unbox".
https://hg.mozilla.org/mozilla-central/rev/e9e7d8066cb9
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.