[MIPS] Use break instruction as NACL_HALT for MIPS.

[MIPS] Use break instruction as NACL_HALT for MIPS.

Replacing previous NACL_HALT (jr $zero) with a break instruction.

BUG= http://code.google.com/p/nativeclient/issues/detail?id=2275
TEST= pnacl/build.sh misc-tools

[petarj, 2012-09-26 15:50:41]

Replacing NACL_HALT with a break instruction has been previously discussed at:

https://chromiumcodereview.appspot.com/10919162/diff/2005/src/trusted/service...

Here is the patch.

Petar

[Mark Seaborn, 2012-09-26 16:59:24]

http://codereview.chromium.org/10978045/diff/1/src/trusted/service_runtime/ar...
File src/trusted/service_runtime/arch/mips/nacl_text_pad_test.S (right):

http://codereview.chromium.org/10978045/diff/1/src/trusted/service_runtime/ar...
src/trusted/service_runtime/arch/mips/nacl_text_pad_test.S:12: #define NACL_HALT
 0x0000000D  /* opcode for "break", MIPS nacl halt */
You could reuse NACL_HALT_OPCODE rather than duplicating a definition here. 
(Also, it's slightly confusing that NACL_HALT is a #define for an assembly
instruction elsewhere, not a number.)

http://codereview.chromium.org/10978045/diff/1/src/trusted/validator_mips/gen...
File src/trusted/validator_mips/gen/decode.cc (right):

http://codereview.chromium.org/10978045/diff/1/src/trusted/validator_mips/gen...
src/trusted/validator_mips/gen/decode.cc:18: const Branch _Branch_instance;
Why do Branch and FPLoadStore get swapped?  Does the validator's generator not
produce deterministic output?  If not, you might want to fix that.

With these non-trivial changes to generated code, you might want to find an
additional reviewer who has reviewed the MIPS validator before.

[petarj, 2012-09-28 17:55:02]

New patch set uploaded.

http://codereview.chromium.org/10978045/diff/1/src/trusted/service_runtime/ar...
File src/trusted/service_runtime/arch/mips/nacl_text_pad_test.S (right):

http://codereview.chromium.org/10978045/diff/1/src/trusted/service_runtime/ar...
src/trusted/service_runtime/arch/mips/nacl_text_pad_test.S:12: #define NACL_HALT
 0x0000000D  /* opcode for "break", MIPS nacl halt */
On 2012/09/26 16:59:24, Mark Seaborn wrote:
> You could reuse NACL_HALT_OPCODE rather than duplicating a definition here. 
> (Also, it's slightly confusing that NACL_HALT is a #define for an assembly
> instruction elsewhere, not a number.)

Done. You may want to do the same in ARM code.

http://codereview.chromium.org/10978045/diff/1/src/trusted/validator_mips/gen...
File src/trusted/validator_mips/gen/decode.cc (right):

http://codereview.chromium.org/10978045/diff/1/src/trusted/validator_mips/gen...
src/trusted/validator_mips/gen/decode.cc:18: const Branch _Branch_instance;
On 2012/09/26 16:59:24, Mark Seaborn wrote:
> Why do Branch and FPLoadStore get swapped?  Does the validator's generator not
> produce deterministic output?  If not, you might want to fix that.
> 
> With these non-trivial changes to generated code, you might want to find an
> additional reviewer who has reviewed the MIPS validator before.

Validator's generator will produce deterministic output for the same input. In
this case, mips32.table has changed, which resulted in changed mips-opt.table,
and that led us to changes in decode.cc.

The change is not large, yet if you believe someone else should look at
them, please add a person who would be a suitable reviewer (too many people
reviewed the original validator change).