Clean up operand code by removing unnecessary variants

decoding.odin

@@ -8,7 +8,7 @@ parse_operand :: proc(inst: InstructionInfo, opinfo: OperandInfo, data: []u8, pr
     operand: Operand = None{}
     switch opinfo {
     case .None:
-    case .Register:
+    case .Register, .SegmentRegister:
         reg: u8
         switch inst.reg_info {
         case .None: panic("Register is required but the encoded location is not provided")
@@ -17,26 +17,13 @@ parse_operand :: proc(inst: InstructionInfo, opinfo: OperandInfo, data: []u8, pr
         case .SecondByteMiddle3: reg = (data[1] >> 3) & 0b111
         case .SecondByteLast3:   reg = data[1] & 0b111
         }
-        if word {
+        if opinfo == .SegmentRegister {
+            operand = (RegisterId){id = SEGMENT_REGISTER_START + (int)(reg), access = .Full}
+        } else if word {
             operand = RegisterId { id = (int)(reg), access = .Full }
         } else {
             operand = RegisterId { id = (int)(reg % 4), access = reg < 4 ? .Low : .High }
         }
-    case .SegmentRegister:
-        reg: u8
-        switch inst.reg_info {
-        case .None:
-            panic("Register is required but the encoded location is not provided")
-        case .FirstByteLast3:
-            reg = data[0] & 0b111
-        case .FirstByteMiddle3:
-            reg = (data[0] >> 3) & 0b111
-        case .SecondByteMiddle3:
-            reg = (data[1] >> 3) & 0b111
-        case .SecondByteLast3:
-            reg = data[1] & 0b111
-        }
-        operand = (SegmentRegister)(segment_registers[reg].code)
     case .RegisterMemory:
         mod := data[1] >> 6
         rm := data[1] & 0b111
@@ -44,6 +31,7 @@ parse_operand :: proc(inst: InstructionInfo, opinfo: OperandInfo, data: []u8, pr
         op: Operand
         if mod == 0 {
             if rm == 0b110 {
+                // op = DirectAddress { value = get_i16(data[2:]) }
                 op = (DirectAddress)(get_i16(data[2:]))
                 processed^ += 2
             } else {
@@ -69,14 +57,16 @@ parse_operand :: proc(inst: InstructionInfo, opinfo: OperandInfo, data: []u8, pr
         if inst.has_sign_extension {
             word_signed &&= data[0] & 0b0000_0010 == 0
         }
-        operand = (Operand)(word_signed ? (Immediate16)(get_i16(data[data_idx:])) : (Immediate8)(data[data_idx]))
+        value: u16 = word_signed ? u16(get_i16(data[data_idx:])) : u16(data[data_idx])
+        operand = Immediate { value = value, size = word_signed ? .Signed16 : .Signed8 }
         processed^ += word_signed ? 2 : 1
     case .ImmediateUnsigned:
-        operand = (ImmediateU8)(data[processed^])
+        operand = Immediate { value = u16(data[processed^]), size = .Unsigned8 }
         processed^ += 1
     case .Accumulator:
         operand = RegisterId { id = 0, access = word ? .Full : .Low }
     case .DirectAddress:
+        // operand = DirectAddress { value = get_i16(data[1:]) }
         operand = (DirectAddress)(get_i16(data[1:]))
         processed^ += 2
     case .Jump:
@@ -84,16 +74,16 @@ parse_operand :: proc(inst: InstructionInfo, opinfo: OperandInfo, data: []u8, pr
         // NOTE: In order to mimic the label offset, you have to take the value you got and add two
         operand = (Jump)((i8)(data[1]) + 2)
     case .VariablePort:
-        operand = VariablePort{}
+        operand = RegisterId { id = (int)(variable_port.code), access = .Full }
     case .ShiftRotate:
         v_flag := data[0] & 0b10 != 0
-        operand = (ShiftRotate)(v_flag)
+        operand = v_flag ? RegisterId { id = 1, access = .Low } : Immediate { value = 1 }
     case .Repeat:
         operand = get_repeat_op(data[1])
         processed^ += 1
     case .DirectWithinSegment:
         value := (int)(get_i16(data[1:])) + total_bytes_processed + 3
-        operand = (DirectWithinSegment)(value)
+        operand = Immediate { value = u16(value), size = .Signed16 }
         processed^ += 2
     case .Intersegment:
         operand = Intersegment {
@@ -133,7 +123,7 @@ decode_data :: proc(inst_list: ^[dynamic]Instruction, data: []u8, bytes_to_read:
             continue
         } else if inst.opname == .SEGMENT {
             reg := (curr_byte & 0b11000) >> 3
-            has_segment = segment_registers[reg]
+            has_segment = registers[SEGMENT_REGISTER_START+reg]
             idx += 1
             continue
         } else if inst.opname == .AAM {
@@ -152,7 +142,6 @@ decode_data :: proc(inst_list: ^[dynamic]Instruction, data: []u8, bytes_to_read:
 
         word: bool
         flip: bool
-        indirect_intersegment: bool
         op: Operand
 
         if inst.has_flip {

execution.odin

@@ -11,20 +11,17 @@ execute_instruction :: proc(inst: Instruction) {
         if reg_id,ok := inst.dst.(RegisterId); ok {
             // val := registers[reg_id.id].value.full
             #partial switch val in inst.src {
-            case Immediate8:
+            case Immediate:
-                if reg_id.access == .Low {
+                if val.size == .Signed16 {
-                    registers[reg_id.id].value.low  = (u8)(val)
+                    registers[reg_id.id].value.full = (u16)(val.value)
                 } else {
-                    registers[reg_id.id].value.high  = (u8)(val)
+                    value := u8(val.value & 0xFF)
-                }
-            case ImmediateU8:
                     if reg_id.access == .Low {
-                    registers[reg_id.id].value.low  = (u8)(val)
+                        registers[reg_id.id].value.low  = value
                     } else {
-                    registers[reg_id.id].value.high  = (u8)(val)
+                        registers[reg_id.id].value.high  = value
+                    }
                 }
-            case Immediate16:
-                registers[reg_id.id].value.full = (u16)(val)
             case RegisterId:
                 switch val.access {
                 case .Low, .High:
@@ -37,23 +34,7 @@ execute_instruction :: proc(inst: Instruction) {
                 case .Full:
                     registers[reg_id.id].value.full = registers[val.id].value.full
                 }
-            case SegmentRegister:
+            }
-                registers[reg_id.id].value.full = segment_registers[val].value.full
-            }
-            // fmt.printfln("%s:%04x->%04x", registers[reg_id].fullname, val, registers[reg_id].value.full)
-        } else if reg_id,ok := inst.dst.(SegmentRegister); ok {
-            // val := segment_registers[reg_id].value.full
-            #partial switch val in inst.src {
-            case Immediate8:
-                segment_registers[reg_id].value.low = (u8)(val)
-            case ImmediateU8:
-                segment_registers[reg_id].value.low = (u8)(val)
-            case Immediate16:
-                segment_registers[reg_id].value.full = (u16)(val)
-            case RegisterId:
-                segment_registers[reg_id].value.full = registers[val.id].value.full
-            }
-            // fmt.printfln("%s:%04x->%04x", segment_registers[reg_id].fullname, val, segment_registers[reg_id].value.full)
         }
     }
 }

printing.odin

@@ -141,8 +141,15 @@ get_operand_string :: proc(operand: Operand, has_segment: Maybe(Register)) -> st
         string_val = ""
     case RegisterId:
         string_val = get_register_name(val)
-    case Immediate8, ImmediateU8, Immediate16, DirectWithinSegment:
+    case Immediate:
-        string_val = fmt.aprintf("%d", val)
+        switch val.size {
+        case .Signed8:
+            string_val = fmt.aprintf("%d", i8(val.value))
+        case .Unsigned8:
+            string_val = fmt.aprintf("%d", u8(val.value))
+        case .Signed16:
+            string_val = fmt.aprintf("%d", i16(val.value))
+        }
     case MemoryAddr:
         string_val = get_memory_string(val, has_segment)
     case DirectAddress:
@@ -151,14 +158,8 @@ get_operand_string :: proc(operand: Operand, has_segment: Maybe(Register)) -> st
             seg_string = fmt.aprintf("%s:", segreg.fullname)
         }
         string_val = fmt.aprintf("%s[%d]", seg_string, val)
-    case SegmentRegister:
-        string_val = segment_registers[val].fullname
     case Jump:
         string_val = fmt.aprintf("$%s%d", val >= 0 ? "+" : "", val)
-    case VariablePort:
-        string_val = variable_port.fullname
-    case ShiftRotate:
-         string_val = val ? "cl" : "1"
     case Repeat:
         string_val = (string)(val)
     case Intersegment:
@@ -176,10 +177,10 @@ get_unknown_inst_string :: proc(inst: Instruction) -> string {
 
 get_instruction_string :: proc(inst_info: InstructionInfo, instruction: Instruction) {
     inst := instruction
-    src_is_imm := operand_is(Immediate8, inst.src) || operand_is(Immediate16, inst.src)
+    src_is_imm := operand_is(Immediate, inst.src)
     dst_is_bracketed := operand_is(MemoryAddr, inst.dst) || operand_is(DirectAddress, inst.dst)
     src_is_bracketed := operand_is(MemoryAddr, inst.src) || operand_is(DirectAddress, inst.src)
-    shiftrot := operand_is(ShiftRotate, inst.src)
+    shiftrot := inst_info.opname == .TBD6
     size_string := ""
     if ((src_is_imm && dst_is_bracketed) || (dst_is_bracketed && shiftrot)) || (src_is_bracketed && operand_is(None, inst.dst)) {
         size_string = inst.is_word ? "word " : "byte "

sim8086.odin

@@ -12,7 +12,7 @@ RIGHT_ALIGN_AMOUNT := 35
 // We have to change how we coded this out. Likely we have to remove bytename as an option
 // and then have some kind of flag when you're doing a dst or src operand specifying
 // if it's the high or low part, not sure how to go about that
-registers := [8]Register {
+registers := [?]Register {
     {fullname = "ax", code = 0b000},
     {fullname = "cx", code = 0b001},
     {fullname = "dx", code = 0b010},
@@ -21,9 +21,6 @@ registers := [8]Register {
     {fullname = "bp", code = 0b101},
     {fullname = "si", code = 0b110},
     {fullname = "di", code = 0b111},
-}
-
-segment_registers := [4]Register {
     {fullname = "es", code = 0b000},
     {fullname = "cs", code = 0b001},
     {fullname = "ss", code = 0b010},
@@ -32,6 +29,8 @@ segment_registers := [4]Register {
 
 variable_port := registers[2]
 
+SEGMENT_REGISTER_START :: 8
+// TODO: I don't like this here
 total_bytes_processed := 0
 
 get_i16 :: proc(data: []u8) -> i16 {
@@ -108,9 +107,6 @@ main :: proc() {
         for reg in registers {
             print_reg(reg)
         }
-        for reg in segment_registers {
-            print_reg(reg)
-        }
     }
     if what_to_print == "instructions" || what_to_print == "all" {
         print_instructions_stdout(instructions_list[:])

test_asm.sh

@@ -13,7 +13,7 @@ fi
 
 for ASM_BIN in asm_files/*.bin;
 do
-    ./sim8086 "$ASM_BIN" > output.asm 2> /dev/null
+    ./sim8086 "$ASM_BIN" instructions > output.asm 2> /dev/null
     nasm output.asm -o output.bin 2> /dev/null
     ASM_FILE=${ASM_BIN%.*}.asm
     if [ ! -e  output.bin ]; then

types.odin

@@ -39,39 +39,35 @@ RegisterId :: struct {
     access: RegisterAccess,
     id: int,
 }
-Immediate8 :: distinct i8
+ImmediateSize :: enum {
-Immediate16 :: distinct i16
+    Signed8,
-ImmediateU8 :: distinct u8
+    Unsigned8,
+    Signed16,
+}
+Immediate :: struct {
+    value: u16,
+    size: ImmediateSize,
+}
 MemoryAddr :: struct {
     addr_id: u8,
     displacement: Displacement,
 }
 DirectAddress :: distinct i16
-SegmentRegister :: distinct i16
 Jump :: distinct i8
-VariablePort :: struct {}
-ShiftRotate :: distinct bool
 Repeat :: string
 Intersegment :: struct {
     ip: i16,
     cs: i16,
 }
-DirectWithinSegment :: distinct u16
 
 Operand :: union {
     None,
     RegisterId,
-    Immediate8,
+    Immediate,
-    ImmediateU8,
-    Immediate16,
     MemoryAddr,
     DirectAddress,
-    SegmentRegister,
     Jump,
-    VariablePort,
-    ShiftRotate,
     Repeat,
-    DirectWithinSegment,
     Intersegment,
 }
 
@@ -122,7 +118,9 @@ Instruction :: struct {
     dst: Operand,
     info: InstructionInfo,
     is_word: bool,
-    indirect_intersegment: bool,
+    // indirect_intersegment: bool,
+    // TODO: This is trickier than I thought, it's more than just the one instruction
+    // that uses it
     has_segment: Maybe(Register),
     has_lock: bool,
     bytes_read: int,