483640 - compiling with ABC interpreter causes numerous compiler errors

Status: VERIFIED FIXED

(Tamarin Graveyard :: Virtual Machine, defect)

Description

[Edwin Smith]

configure.py --enable-abc-interp adds -DAVMPLUS_ABC_INTERPRETER, which disables the wordcode interpreter.

initial problem list:
- showState() refers to WordCode
- warning that pc is declared register when passed by ref to readU30()
- signature of CallStackNode has changed

Comment 1

[Rick Reitmaier]

Attachment: ver1 - fix

patches fixes (1) and (3).  After looking at the generated assembly, (2) appears to be bogus and has been in the build for a while.

Comment 2

[Edwin Smith]

Comment on attachment 367638 [details] [diff] [review]
ver1 - fix

the new call to formatOpcode should just pass opcode, no masking or casting required.  but the code in the patch is harmless too, so i dont need/want to re-review it :-)

doesn't fix the "register" warning, but no matter, i have a patch, will post when able.

Comment 3

[Rick Reitmaier]

The masking is needed; at least for wordcode.  I hit a bug wherein the upper 16bits and lower 16bits were identical (Lars mentioned it was a debugging aid that leaked out).

I'm curious to see the fix for the *register* warning.

Comment 4

[Rick Reitmaier]

pushed 4c76cb476759: Fix bug 483640 - abc interpreter not building after CallStackNode changes - rev 1603

Comment 5

[Lars T Hansen]

(In reply to comment #3)
> The masking is needed; at least for wordcode.  I hit a bug wherein the upper
> 16bits and lower 16bits were identical (Lars mentioned it was a debugging aid
> that leaked out).
> 
> I'm curious to see the fix for the *register* warning.

The masking is only needed for word code.

Please do not remove the *register* annotation, if we get a warning that means we need to rewrite the interpreter code (that's why it's there).  The address of the pc should never be taken.  Probably in this case we need a new version of readU30.

Comment 6

[Edwin Smith]

Attachment: restore the masking for wordcode formatOpcode()

Comment 7

[Rick Reitmaier]

Comment on attachment 367813 [details] [diff] [review]
restore the masking for wordcode formatOpcode()

#ifdef AVMPLUS_WORD_CODE
      opcode = (WordOpcode ((int32_t)opcode&0xffff);
#endif

could also do the trick.  And we shouldn't need explicit casting in the call.

'+' since this fix is fine.

Comment 8

[Edwin Smith]

pushed 
http://hg.mozilla.org/tamarin-redux/rev/0030984f6788

Comment 9

[Brent Baker]

wordcode and abc interpreters are both compiled and tested in the build system