On Wed, Feb 24, 2010 at 12:28 PM, <[email protected]> wrote: > > http://codereview.chromium.org/651029/diff/1006/55 > File src/arm/instr-thumb2.cc (right): > > http://codereview.chromium.org/651029/diff/1006/55#newcode40 > src/arm/instr-thumb2.cc:40: if (Bits0(15, 13) == b111 && Bits0(12, 11) > != b00) { > We don't feel these bxxx constants add enough in terms of clarity to > compensate for their strangeness. Please use hex throughout. If > possible of course the constants should have real names. Any opcode in > table A6-1 and the tables on the following pages should have a name. In > this case it would be clearer to write:
if (Bits(15, 11) > kMax16BitThumbOpcode) > The comparison literally follows A6.3, but I agree that your version is more elegant. Concerning the bxxx constants, I think they sufficiently simplify checking that the code matches the spec to justify their existence, at least during development. And adding all the strange thumb opcodes / encodings will be painful and error-prone enough without the additional hex conversion step. The same type of constants is used e.g in the Theora spec: http://www.theora.org/doc/Theora.pdf .I am happy to prepare a script that replaces them with hex numbers, so we can get rid of them easily when the code is more stable. > where kMax16BitThumbOpcode is 0x1c, from table A6-1. > > > http://codereview.chromium.org/651029 > -- Stefan Haustein Google UK Limited Registered Office: Belgrave House, 76 Buckingham Palace Road, London SW1W 9TQ; Registered in England Number: 3977902 -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
