Closed Bug 821268 Opened 7 years ago Closed 7 years ago

BaselineCompiler: Add double comparison stub

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla20

People

(Reporter: jandem, Assigned: jandem)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Need this for SS math-cordic.
Patch for m-c to move NaNCond and related functions.
Attachment #691819 - Flags: review?(kvijayan)
Also fixes ARM FloatReg0: ReturnFloatReg was changed from d1 to d0 recently.
Attachment #691821 - Flags: review?(kvijayan)
Attachment #691819 - Flags: review?(kvijayan) → review+
Comment on attachment 691821 [details] [diff] [review]
Part 2: Double compare stub

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

::: js/src/ion/BaselineIC.h
@@ +720,5 @@
>      }
>  
> +    void ensureDouble(MacroAssembler &masm, const ValueOperand &source, FloatRegister dest,
> +                      Label *failure);
> +

This really feels like it should go on MacroAssembler.

The only baseline-specific part of ensureDouble is the usage of "ExtractTemp0", but since only one is needed, x64 can jut use its scratch reg for that, and x86 and arm don't need one.

@@ +1087,5 @@
>  {
>      friend class ICStubSpace;
>  
>      ICCompare_Int32(IonCode *stubCode)
> +      : ICStub(ICStub::Compare_Int32, stubCode) {}

Nice catch.

::: js/src/ion/arm/BaselineIC-arm.cpp
@@ +57,5 @@
> +    Assembler::Condition cond = Assembler::ConditionFromDoubleCondition(doubleCond);
> +
> +    masm.compareDouble(FloatReg0, FloatReg1);
> +    masm.ma_mov(Imm32(0), dest);
> +    masm.ma_mov(Imm32(1), dest, NoSetCond, cond);

The first mov(Imm32(0), dest) should be NoSetCond as well.
Attachment #691821 - Flags: review?(kvijayan) → review+
https://hg.mozilla.org/mozilla-central/rev/6002b73474a7
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
https://hg.mozilla.org/projects/ionmonkey/rev/c255129f4994

(In reply to Kannan Vijayan [:djvj] from comment #3)
> 
> The first mov(Imm32(0), dest) should be NoSetCond as well.

NoSetCond is the default.
You need to log in before you can comment on or make changes to this bug.