Remove some dead code from the JS engine

RESOLVED FIXED in mozilla27

Status

()

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Ehsan, Assigned: sfink)

Tracking

Trunk
mozilla27
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
Created attachment 807190 [details] [diff] [review]
Patch (v1)

Discovered by clang.
Attachment #807190 - Flags: review?(luke)

Comment 1

5 years ago
Comment on attachment 807190 [details] [diff] [review]
Patch (v1)

Thanks!
Attachment #807190 - Flags: review?(luke) → review+
(Assignee)

Comment 3

5 years ago
Comment on attachment 807190 [details] [diff] [review]
Patch (v1)

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

::: js/src/jit/MIR.h
@@ +8454,5 @@
>  
>  class MAsmJSLoadGlobalVar : public MNullaryInstruction
>  {
>      MAsmJSLoadGlobalVar(MIRType type, unsigned globalDataOffset, bool isConstant)
> +      : globalDataOffset_(globalDataOffset)

Seems like the isConstant parameter is dead now too.
(Assignee)

Comment 4

5 years ago
Created attachment 807327 [details] [diff] [review]
Eliminate dead isConstant param

Or would this be useful information in the future?
Attachment #807327 - Flags: review?(luke)
(Assignee)

Updated

5 years ago
Assignee: ehsan → sphink

Comment 5

5 years ago
Let's check with Douglas: are there some pending patches that will be using isConstant?
Flags: needinfo?(dtc-moz)

Comment 6

5 years ago
https://hg.mozilla.org/mozilla-central/rev/04bf94160337
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
The lack of use of isConstant might be a regression from
bug 877338, or it might no longer be necessary after the
patch in 877338.   It might still be possible to relax
the constraints on movement of a global load when it is
a constant.  I'll need to study the code further.
Depends on: 920452
Flags: needinfo?(dtc-moz)

Updated

5 years ago
Attachment #807327 - Flags: review?(luke)
You need to log in before you can comment on or make changes to this bug.