Change table data_processing_immediate to use the new notation. Also

Change table data_processing_immediate to use the new notation. Also
generalizes the notion of (pattern) constraints to allow more general
expressions when not in the deprecated form.

BUG= http://code.google.com/p/nativeclient/issues/detail?id=2673
Committed: https://src.chromium.org/viewvc/native_client?view=rev&revision=9821

[JF, 2012-09-19 01:14:23]

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
File src/trusted/validator_arm/armv7.table (right):

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:396: setflags := S=1; imm32 :=
ARMExpandImm(imm12);
ARMExpandImm_C

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:398: safety := (Rd=1111 & S=1) =>
UNDEFINED &  # ARM
Rd=1111 & S=1 is actually the SUB PC, LR instruction, which is forbidden. Why
not just mark it as FORBIDDEN_OPERANDS?

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:401: *2 Binary2RegisterImmediateOp =>
Defs12To15
Why make this different from *1? The only difference is (Rn=1111 & S=0), but
that's not undefined: it's ADR. There would be a parse conflict if this weren't
the case. It seems like *2 uses could be folded into *1 uses.

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:415: *4 Unary1RegisterImmediateOp =>
Defs12To15
*4 is only different from *3 in the add variable. bit 23 is often the U bit, and
*3/*4 are the same with:
  add := U=1;

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:422: { cond(31:28), Rn(19:15), imm12(11:0)
}
Rn(19:16)

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:423: imm32 := AMRExpandImm(imm12);
ARMExpandImm_C

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:424: defs := {NZCV};
uses := {Rn};

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:429: { cond(31:28), Rn(19:15), imm12(11:0)
}
Rn(19:16)

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:430: imm32 := AMRExpandImm(imm12);
ARMExpandImm_C

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:432: sets_Z_if_bits_clear := true;
uses := {Rn};

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:433: *7 Unary1RegisterImmediateOp =>
Defs12To15
*7 is wrong and encodes encoding A2, not A1. It should be:

{ cond(31:28), S(20), Rd(15:12), imm12(11:10) }
setflags := S=1; imm32 := ARMExpandImm_C(imm12);
defs := {Rd, NZCV if setflags else None};
safety := (Rd=1111 & S=1) => FORBIDDEN_OPERANDS &
           Rd=1111 => FORBIDDEN_OPERANDS;
uses := {};

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:442: setflags := S=1; imm32 :=
ARMExpandImm(imm12);
ARMExpandImm_C

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:444: safety := (Rd=1111 & S=1) =>
UNDEFINED &  # ARM
FORBIDDEN_OPERANDS, as above.

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:448: *9 Unary1RegisterImmediateOp =>
Defs12To15
This is the same as *7 that I've defined above (except for corrections above).

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:457: & not (Rd=1111 & S=1);
Why this restriction? Won't the safety make this case UNDEFINED?

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:460: & not (Rd=1111 & S=1);
Ditto on UNDEFINED restriction.

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:462: | 0010x      ~1111    = *2 pattern :=
cccc0010010snnnnddddiiiiiiiiiiii
As noted above, change *2 to *1.

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:463: & not (Rd=1111 & S=1);
Ditto on UNDEFINED restriction.

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:465: # Hence, parse restriction does not
apply (See SUB (SP minus immediate)).
Why this note? It confuses me.

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:470: & not (Rd=1111 & S=1);
Ditto on UNDEFINED restriction.

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:472: | 0100x      ~1111    = *2 pattern :=
cccc0010100snnnnddddiiiiiiiiiiii
As noted above, change *2 to *1.

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:473: & not (Rd=1111 & S=1);
Ditto on UNDEFINED restriction.

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:475: # Hence, parse restriction does not
apply (See ADD (SP plus immediate)).
Ditto on my confusion.

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:477: | 0100x      1111     = *4 pattern :=
cccc001010001111ddddiiiiiiiiiiii;
As noted above, change to *3.

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:480: & not (Rd=1111 & S=1);
Ditto on UNDEFINED restriction.

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:483: & not (Rd=1111 & S=1);
Ditto on UNDEFINED restriction.

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:486: & not (Rd=1111 & S=1);
Ditto on UNDEFINED restriction.

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:499: & not (Rd=1111 & S=1);
Ditto on UNDEFINED restriction.

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:504: & not (Rd=1111 & S=1);
Ditto on UNDEFINED restriction.

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:506: | 1111x      -        = *9 pattern :=
cccc0011111s0000ddddiiiiiiiiiiii
Change to *7.

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:507: & not (Rd=1111 & S=1);
Ditto on UNDEFINED restriction.

[Karl, 2012-09-20 19:47:42]

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
File src/trusted/validator_arm/armv7.table (right):

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:396: setflags := S=1; imm32 :=
ARMExpandImm(imm12);
On 2012/09/19 01:14:23, JF wrote:
> ARMExpandImm_C

Done.

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:398: safety := (Rd=1111 & S=1) =>
UNDEFINED &  # ARM
On 2012/09/19 01:14:23, JF wrote:
> Rd=1111 & S=1 is actually the SUB PC, LR instruction, which is forbidden. Why
> not just mark it as FORBIDDEN_OPERANDS?

The real problem is that we don't have a value that states we have a problem
with our decoder.

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:398: safety := (Rd=1111 & S=1) =>
UNDEFINED &  # ARM
On 2012/09/19 01:14:23, JF wrote:
> Rd=1111 & S=1 is actually the SUB PC, LR instruction, which is forbidden. Why
> not just mark it as FORBIDDEN_OPERANDS?
Changed to DECODER_ERROR.

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:401: *2 Binary2RegisterImmediateOp =>
Defs12To15
On 2012/09/19 01:14:23, JF wrote:
> Why make this different from *1? The only difference is (Rn=1111 & S=0), but
> that's not undefined: it's ADR. There would be a parse conflict if this
weren't
> the case. It seems like *2 uses could be folded into *1 uses.
Like above, made a DECODER_ERROR entry.

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:415: *4 Unary1RegisterImmediateOp =>
Defs12To15
On 2012/09/19 01:14:23, JF wrote:
> *4 is only different from *3 in the add variable. bit 23 is often the U bit,
and
> *3/*4 are the same with:
>   add := U=1;
Merged.

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:422: { cond(31:28), Rn(19:15), imm12(11:0)
}
On 2012/09/19 01:14:23, JF wrote:
> Rn(19:16)

Done.

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:423: imm32 := AMRExpandImm(imm12);
On 2012/09/19 01:14:23, JF wrote:
> ARMExpandImm_C

Done.

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:424: defs := {NZCV};
On 2012/09/19 01:14:23, JF wrote:
> uses := {Rn};

Done.

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:429: { cond(31:28), Rn(19:15), imm12(11:0)
}
On 2012/09/19 01:14:23, JF wrote:
> Rn(19:16)

Done.

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:430: imm32 := AMRExpandImm(imm12);
On 2012/09/19 01:14:23, JF wrote:
> ARMExpandImm_C

Done.

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:432: sets_Z_if_bits_clear := true;
On 2012/09/19 01:14:23, JF wrote:
> uses := {Rn};

Done.

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:433: *7 Unary1RegisterImmediateOp =>
Defs12To15
On 2012/09/19 01:14:23, JF wrote:
> *7 is wrong and encodes encoding A2, not A1. It should be:
> 
> { cond(31:28), S(20), Rd(15:12), imm12(11:10) }
> setflags := S=1; imm32 := ARMExpandImm_C(imm12);
> defs := {Rd, NZCV if setflags else None};
> safety := (Rd=1111 & S=1) => FORBIDDEN_OPERANDS &
>            Rd=1111 => FORBIDDEN_OPERANDS;
> uses := {};

Fixed.

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:442: setflags := S=1; imm32 :=
ARMExpandImm(imm12);
On 2012/09/19 01:14:23, JF wrote:
> ARMExpandImm_C

Done.

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:444: safety := (Rd=1111 & S=1) =>
UNDEFINED &  # ARM
On 2012/09/19 01:14:23, JF wrote:
> FORBIDDEN_OPERANDS, as above.

Changed to DECODER_ERROR.

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:448: *9 Unary1RegisterImmediateOp =>
Defs12To15
On 2012/09/19 01:14:23, JF wrote:
> This is the same as *7 that I've defined above (except for corrections above).

Merged.

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:457: & not (Rd=1111 & S=1);
On 2012/09/19 01:14:23, JF wrote:
> Why this restriction? Won't the safety make this case UNDEFINED?

This is a testing infrastructure issue. It is needed to communicate to the
testing infrastructure that we should find a different decoder here.

However, looking closer, the conflicting instructions are system instructions
that aren't in our decoder tables, and that we disallow with the ARM restriction
Rd=~1111. Hence, I've removed them

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:460: & not (Rd=1111 & S=1);
On 2012/09/19 01:14:23, JF wrote:
> Ditto on UNDEFINED restriction.

removed

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:462: | 0010x      ~1111    = *2 pattern :=
cccc0010010snnnnddddiiiiiiiiiiii
On 2012/09/19 01:14:23, JF wrote:
> As noted above, change *2 to *1.

Done.

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:463: & not (Rd=1111 & S=1);
On 2012/09/19 01:14:23, JF wrote:
> Ditto on UNDEFINED restriction.

removed

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:465: # Hence, parse restriction does not
apply (See SUB (SP minus immediate)).
On 2012/09/19 01:14:23, JF wrote:
> Why this note? It confuses me.

removed

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:470: & not (Rd=1111 & S=1);
On 2012/09/19 01:14:23, JF wrote:
> Ditto on UNDEFINED restriction.

removed

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:472: | 0100x      ~1111    = *2 pattern :=
cccc0010100snnnnddddiiiiiiiiiiii
On 2012/09/19 01:14:23, JF wrote:
> As noted above, change *2 to *1.

Done.

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:473: & not (Rd=1111 & S=1);
On 2012/09/19 01:14:23, JF wrote:
> Ditto on UNDEFINED restriction.

removed

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:475: # Hence, parse restriction does not
apply (See ADD (SP plus immediate)).
On 2012/09/19 01:14:23, JF wrote:
> Ditto on my confusion.

removed

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:477: | 0100x      1111     = *4 pattern :=
cccc001010001111ddddiiiiiiiiiiii;
On 2012/09/19 01:14:23, JF wrote:
> As noted above, change to *3.

Done.

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:480: & not (Rd=1111 & S=1);
On 2012/09/19 01:14:23, JF wrote:
> Ditto on UNDEFINED restriction.

removed

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:483: & not (Rd=1111 & S=1);
On 2012/09/19 01:14:23, JF wrote:
> Ditto on UNDEFINED restriction.

removed

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:486: & not (Rd=1111 & S=1);
On 2012/09/19 01:14:23, JF wrote:
> Ditto on UNDEFINED restriction.

removed

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:499: & not (Rd=1111 & S=1);
On 2012/09/19 01:14:23, JF wrote:
> Ditto on UNDEFINED restriction.

removed

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:504: & not (Rd=1111 & S=1);
On 2012/09/19 01:14:23, JF wrote:
> Ditto on UNDEFINED restriction.

removed

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:506: | 1111x      -        = *9 pattern :=
cccc0011111s0000ddddiiiiiiiiiiii
On 2012/09/19 01:14:23, JF wrote:
> Change to *7.

Done.

https://codereview.chromium.org/10949006/diff/1/src/trusted/validator_arm/arm...
src/trusted/validator_arm/armv7.table:507: & not (Rd=1111 & S=1);
On 2012/09/19 01:14:23, JF wrote:
> Ditto on UNDEFINED restriction.

removed

[JF, 2012-09-24 14:36:48]

lgtm