Closed Bug 1070971 Opened 10 years ago Closed 10 years ago

IonMonkey ARM: declare operands to common ALU and FPU operations to be used at the start.

Categories

(Core :: JavaScript Engine: JIT, defect)

ARM
All
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla35

People

(Reporter: dougc, Assigned: dougc)

Details

Attachments

(1 file, 2 obsolete files)

Declaring operands to be used at the start of the operation allows the register allocator to reuse an argument register for the output which can reduce register pressure. Some of the ALU operations check the arguments after writing to the destination, but do this only in the bail out paths so we can still declare them AtStart for Odin compilation.
Assignee: nobody → dtc-moz
Comment on attachment 8493064 [details] [diff] [review]
Declare operands to common ALU and FPU operations to be used at the start.

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

::: js/src/jit/arm/Lowering-arm.cpp
@@ +190,5 @@
>  LIRGeneratorARM::lowerForALU(LInstructionHelper<1, 2, 0> *ins, MDefinition *mir, MDefinition *lhs, MDefinition *rhs)
>  {
> +    // Some operations depend on checking inputs after writing the result, e.g.
> +    // MulI, but only for bail out paths so for AsmJS we can useAtStart.
> +    if (gen->compilingAsmJS()) {

Checking for AsmJS mode is something we try to avoid when possible. For example, range analysis can also eliminate bailouts in non-AsmJS mode in some cases. Would testing for ins->snapshot() != nullptr work instead? That's still perhaps not perfect, and still worthy of a comment, but it seems like it'd be closer to the minimal requirement.
Thank you for the feedback. Checking for snapshot looks much better, thanks.
Attachment #8493064 - Attachment is obsolete: true
Attachment #8493483 - Flags: review?(sunfish)
Comment on attachment 8493483 [details] [diff] [review]
Declare operands to common ALU and FPU operations to be used at the start.

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

Architectures with three-address instructions are much simpler than x86 in this territory :-).
Attachment #8493483 - Flags: review?(sunfish) → review+
Rebase. Carry forward r+.
Attachment #8493483 - Attachment is obsolete: true
Attachment #8498853 - Flags: review+
Some try run failures, but they look like infrastructure issues. Requesting this patch land.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/b1b5b6c55e82
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.