1173869 - MBinaryArithInstruction::infer cleanups

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Hannes Verschore [:h4writer]]

There is some dead code inside MBinaryArithInstruction::infer. So time to remove that.

Comment 1

[Hannes Verschore [:h4writer]]

Attachment: bug1173869-infercleanups

Comment 2

[Hannes Verschore [:h4writer]]

Comment on attachment 8621085 [details] [diff] [review]
bug1173869-infercleanups

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

::: js/src/jit/MIR.cpp
@@ +2628,5 @@
>      MOZ_ASSERT(lhs < MIRType_String || lhs == MIRType_Value);
>      MOZ_ASSERT(rhs < MIRType_String || rhs == MIRType_Value);
>  
>      MIRType rval = this->type();
> +    specialization_ = type;

s/type/rval

Comment 3

[Jan de Mooij [:jandem]]

Comment on attachment 8621085 [details] [diff] [review]
bug1173869-infercleanups

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

Looks good.

::: js/src/jit/MIR.cpp
@@ +2606,5 @@
>          setResultType(MIRType_Int32);
> +
> +        // If the operation will always overflow on its constant operands, use a
> +        // double specialization so that it can be constant folded later.
> +        if ((isMul() || isDiv())) {

Nit: remove the extra parentheses.

@@ +2613,5 @@
> +            if (typeChange)
> +                setResultType(MIRType_Double);
> +        }
> +    } else if (IsFloatingPointType(lhs) || IsFloatingPointType(rhs)) {
> +        // Double operations are prioritary over float32 operations (i.e. if any operand needs

Nit: pre-existing typo, priority

@@ +2623,4 @@
>  
>      // If the operation has ever overflowed, use a double specialization.
>      if (inspector->hasSeenDoubleResult(pc))
>          setResultType(MIRType_Double);

Can you move this inside the Int32-if?

@@ +2628,5 @@
>      MOZ_ASSERT(lhs < MIRType_String || lhs == MIRType_Value);
>      MOZ_ASSERT(rhs < MIRType_String || rhs == MIRType_Value);
>  
>      MIRType rval = this->type();
> +    specialization_ = type;

This can be

specialization_ = this->type();
MOZ_ASSERT(IsNumberType(specialization_));

@@ +2633,4 @@
>  
>      if (isAdd() || isMul())
>          setCommutative();
>      setResultType(rval);

I think this line can be removed.

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/44174f5d6bc7

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/44174f5d6bc7