Move validator_x86_XX.rl out of unreviewed.

src/trusted/validator_ragel/decoder_x86_64.rl

 /*
  * Copyright (c) 2011 The Native Client Authors. All rights reserved.
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include <assert.h>
 #include <stddef.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ skipping 49 matching lines @@
     "native_client/src/trusted/validator_ragel/unreviewed/parse_instruction.rl";
 
   include decode_x86_64 "decoder_x86_64_instruction.rl";
 
   main := (one_instruction
      @{
         switch (instruction.rm.disp_type) {
           case DISPNONE: instruction.rm.offset = 0; break;
           case DISP8: instruction.rm.offset = (int8_t) *disp; break;
           case DISP16: instruction.rm.offset =
             (uint16_t) (disp[0] + 256U * disp[1]);

[Brad Chen, 2012/09/28 20:41:01]

Is it deliberate or accidental that you use uint16_t here but int16_t for the
32-bit version? Regardless, avoiding the code duplication would make these
differences much easier to see and understand. At this point these
maintainability issues are much more important than performance.

Diffing the two files shows the files are about 80% identical with a variety of
subtle differences. I would prefer if you would:
- eliminate duplicated code
- eliminate accidental differences
- document deliberate differences
Make sense? It's wonderful the 32- and 64-bit decoders are so similar; a great
opportunity to simplify things and get rid of code!

I'm not going to read the rest of this file until you respond to this initial
feedback. At the risk of stating he obvious please consider the style comments
on the 32-bit file to apply throughout.

[khim, 2012/09/28 23:22:06]

It's just a typo - and an unimportant one at that since currently we don't
decode instructions which use 16-bit offsets.
I agree. Initially these files had more differences, but now most of them are
eliminated. I'll see if I can just unify them.
Decoders are only used in tests anyway. For them speed is of no issue.
Sure.

             break;
           case DISP32: instruction.rm.offset = (int32_t)
             (disp[0] + 256U * (disp[1] + 256U * (disp[2] + 256U * (disp[3]))));
             break;
           case DISP64: instruction.rm.offset = (int64_t)
             (*disp + 256ULL * (disp[1] + 256ULL * (disp[2] + 256ULL * (disp[3] +
             256ULL * (disp[4] + 256ULL * (disp[5] + 256ULL * (disp[6] + 256ULL *
                                                                  disp[7])))))));
             break;
         }
@@ skipping 126 matching lines @@
   SET_REPZ_PREFIX(FALSE);
   SET_BRANCH_NOT_TAKEN(FALSE);
   SET_BRANCH_TAKEN(FALSE);
 
   %% write init;
   %% write exec;
 
 error_detected:
   return result;
 }