Lift register and condition code enums out into their own file.

Lift register and condition code enums out into their own file.

Lift the enums out of IceInstX8632.h and IceTargetLoweringX8632.h.
This will later allow the assembler to share the enum values and
use them as encodings where appropriate. E.g., to avoid having a separate
enum in:
https://codereview.chromium.org/476323004/diff/680001/src/assembler_constants_ia32.h

The "all registers" enum is retained, but separate GPRRegister and
XmmRegister enums are created with tags "Encoded_Reg_foo" to represent
the encoded value of register "foo". Functions are added to convert
from the "all registers" namespace to the encoded ones.

Re-order the BrCond so that they match the encoding according to
the "Instruction Subcode" in B.1 of the Intel Manuals.

BUG=none
R=stichnot@chromium.org

Committed: https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-subzero.git;a=commit;h=bd385e4628d0b23ab54de230ca34bc30a3bb0db7

[jvoung (off chromium), 2014-09-18 18:59:59]

jvoung@chromium.org changed reviewers:
+ stichnot@chromium.org

[jvoung (off chromium), 2014-09-18 19:01:27]

Ah darn... I think I need to fill out the Condition Code table so that the
indices are contiguous...

[jvoung (off chromium), 2014-09-18 19:05:47]

On 2014/09/18 19:01:27, jvoung wrote:
> Ah darn... I think I need to fill out the Condition Code table so that the
> indices are contiguous...

Okay fixed.

[Jim Stichnoth, 2014-09-18 21:16:36]

LGTM.

https://codereview.chromium.org/582113003/diff/60001/src/IceRegistersX8632.h
File src/IceRegistersX8632.h (right):

https://codereview.chromium.org/582113003/diff/60001/src/IceRegistersX8632.h#...
src/IceRegistersX8632.h:29: val,
Just so I understand this correctly, the RegX8632::Reg_foo values are all
unique, and these are the values assigned to Variable::RegNum?  And they are the
values that are used internally until time for code emission?

https://codereview.chromium.org/582113003/diff/60001/src/IceRegistersX8632.h#...
src/IceRegistersX8632.h:64: assert(Reg_eax <= RegNum && RegNum <= Reg_edi);
Would it be practical to use something like Reg_GPR_First and Reg_GPR_Last
instead of Reg_eax and Reg_edi?  (Same comment for the XMM registers.)  You
should probably also subtract Reg_eax / Reg_GPR_First in the same way that
Reg_xmm0 is subtracted in line 70.

[jvoung (off chromium), 2014-09-19 00:52:47]

https://codereview.chromium.org/582113003/diff/60001/src/IceRegistersX8632.h
File src/IceRegistersX8632.h (right):

https://codereview.chromium.org/582113003/diff/60001/src/IceRegistersX8632.h#...
src/IceRegistersX8632.h:29: val,
On 2014/09/18 21:16:35, stichnot wrote:
> Just so I understand this correctly, the RegX8632::Reg_foo values are all
> unique, and these are the values assigned to Variable::RegNum?  And they are
the
> values that are used internally until time for code emission?

Yes, that's right. I'm currently just letting the enum automatically count up.

Then the Variable::RegNum get to RegX8632::Encoded_Reg_foo, etc. for emission.
It seemed risky to have them be non-unique before emission.

https://codereview.chromium.org/582113003/diff/60001/src/IceRegistersX8632.h#...
src/IceRegistersX8632.h:64: assert(Reg_eax <= RegNum && RegNum <= Reg_edi);
On 2014/09/18 21:16:35, stichnot wrote:
> Would it be practical to use something like Reg_GPR_First and Reg_GPR_Last
> instead of Reg_eax and Reg_edi?  (Same comment for the XMM registers.)  You
> should probably also subtract Reg_eax / Reg_GPR_First in the same way that
> Reg_xmm0 is subtracted in line 70.

Done (subtraction, and adding First/Last).

It only seemed "right" to define Reg_GPR_First/Last alongside Reg_eax in the
.defs file, so that they stay in sync. However, I can't do it "inline" with
REGX8632_TABLE (or the sub parts like REGX8632_GPR_TABLE). Otherwise it will end
up adding extra slots for things like:

IceString RegNames[] = {
#define X(name, ...) \
  name
  REGX8632_TABLE
#undef X

and throw off the indexing into the RegNames[] array.

Instead, I put another macro alongside REGX8632_TABLE, called
REGX8632_TABLE_BOUNDS, and AllRegisters would use that as well.

[jvoung (off chromium), 2014-09-19 01:18:15]

Committed patchset #5 (id:80001) manually as bd385e4 (presubmit successful).