605415 - Re-enable MonoICs for ARM: regression from bug 599214

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Chris Leary [:cdleary] (not checking bugmail)]

We need to get trace tuning in tomorrow, so we're disabling MonoIC on ARM until we can find the source of the ARM MonoIC error in the new IC: http://hg.mozilla.org/tracemonkey/rev/81d0ca612cc8

I'm going to be hacking on getting these back online while jbramley keeps on going with the GetProp PIC at an older revno per bug 588021.

Failing trace tests:

    -m basic/bug524826-2.js
    -m basic/bug524826.js
    -m basic/equalInt.js
    -m basic/strings.js
    -m basic/truthies.js
    -m sunspider/check-date-format-tofte.js
    -m sunspider/check-string-tagcloud.js
    -m v8-v5/check-crypto.js
    -m v8-v5/check-deltablue.js

Comment 1

[Jacob Bramley [:jbramley]]

Attachment: Re-enable MICs.

The only problems I see on the current tip are the following:

  • The 'using namespace js::mjit::ic' line is only included for
    JS_POLYIC, but it is also necessary for JS_MONOIC. The symptom of
    this is that BAD_TRACEIC_INDEX is undefined when it is referenced in
    Compiler::jumpAndTrace.

  • I see a warning because EqualityICInfo::cond was a 6-bit bitfield
    (with enum type Assembler::Condition), but ARMAssembler defines the
    enum values as pre-shifted condition codes, like 0x40000000 or
    0x50000000. This was actually just a warning, but would have messed
    with the tests. I suspect that all the conditions would have been
    truncated to 0, which represents the 'EQ' (equal) condition code on
    ARM.

    (This kind of problem is typical with use of bit-fields.)

The attached patch fixes both issues. It passes all jit-tests
(trace-tests) on my ARM board, with '--jitflags=m', '--jitflags=mj' and
with both release and debug builds.

I didn't reproduce the issues that you see, Chris, so I don't know if
this is actually the correct fix for the regression that you see.

Comment 2

[Jacob Bramley [:jbramley]]

Comment on attachment 485282 [details] [diff] [review]
Re-enable MICs.

>-static const uint16 BAD_TRACEIC_INDEX = (uint16_t)-1;
>+static const uint16 BAD_TRACEIC_INDEX = (uint16)0xffff;

That bit shouldn't be strictly necessary but I didn't test without it.

Comment 3

[Chris Leary [:cdleary] (not checking bugmail)]

(In reply to comment #1)
> I didn't reproduce the issues that you see, Chris, so I don't know if
> this is actually the correct fix for the regression that you see.

I confirmed the bitfield was the issue, not the generated code. I'll get this into the tree ASAP.

Comment 4

[Chris Leary [:cdleary] (not checking bugmail)]

http://hg.mozilla.org/tracemonkey/rev/168008d716b4

Comment 5

[Doug Turner (:dougt)]

http://hg.mozilla.org/mozilla-central/rev/1b051dca6365