ARM Simulator: increase the bit length of the instruction counter to 64 bits

RESOLVED FIXED in mozilla30

Status

()

Core
JavaScript Engine: JIT
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: dougc, Assigned: dougc)

Tracking

unspecified
mozilla30
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

4 years ago
The instruction counter (not the machines program counter) is currently a 32 bit integer which overflows quickly, so use a 64 bit integer. The counter is used for debugging and it is possible to break into the debugger are a particular count.

Also add a getter for the instruction count as it is useful to be able to access from other areas for hack debugging.
(Assignee)

Comment 1

4 years ago
Created attachment 8369906 [details] [diff] [review]
Increase the bit length of the instruction counter to 64 bits

Most of the larger tests quickly overflow the 32 bit instruction counter, and there have already been glitches that need exploring well past the limit of the 32 bit counter.

The patch does not currently accept a 64 bit integer 'stop count' from the command line, just from the environment variable.  Will this cut it?
Attachment #8369906 - Flags: review?(jdemooij)
Comment on attachment 8369906 [details] [diff] [review]
Increase the bit length of the instruction counter to 64 bits

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

Looks good. I'm sure this is fine but just to be safe, can you make sure an --enable-optmize build doesn't get noticeably slower when running jit-tests or some large script?

::: js/src/jit/arm/Simulator-arm.h
@@ +350,5 @@
>      };
>      StopCountAndDesc watched_stops_[kNumOfWatchedStops];
> +
> +  public:
> +    int64_t getIcount(void) {

Style nit: no "get" prefix for infallible methods like this (it's possible imported v8 code violates this) and (void) is not necessary in C++, so

int64_t icount() {
Attachment #8369906 - Flags: review?(jdemooij) → review+
(Assignee)

Comment 3

4 years ago
Created attachment 8371068 [details] [diff] [review]
Increase the bit length of the instruction counter to 64 bits

Address review feedback.  Renamed method.  The generated code for 64 bit addition and comparison looks efficient and there is no noticeable impact on performance.

Carry forward r+.
Attachment #8369906 - Attachment is obsolete: true
Attachment #8371068 - Flags: review+
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/8bfdadae80bb
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/8bfdadae80bb
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.