From a91c3d9ba92ca2dec96e4b9adb5133f5b34d0226 Mon Sep 17 00:00:00 2001 From: Joseph Ferano Date: Wed, 19 Mar 2025 18:57:34 +0700 Subject: [PATCH] Clean up operand code by removing unnecessary variants --- decoding.odin | 37 +++++++++++++------------------------ execution.odin | 37 +++++++++---------------------------- printing.odin | 21 +++++++++++---------- sim8086.odin | 10 +++------- test_asm.sh | 2 +- types.odin | 28 +++++++++++++--------------- 6 files changed, 50 insertions(+), 85 deletions(-) diff --git a/decoding.odin b/decoding.odin index c319723..a548735 100644 --- a/decoding.odin +++ b/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 { diff --git a/execution.odin b/execution.odin index 574c4fd..89527aa 100644 --- a/execution.odin +++ b/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: - if reg_id.access == .Low { - registers[reg_id.id].value.low = (u8)(val) + case Immediate: + if val.size == .Signed16 { + registers[reg_id.id].value.full = (u16)(val.value) } else { - registers[reg_id.id].value.high = (u8)(val) + value := u8(val.value & 0xFF) + if reg_id.access == .Low { + registers[reg_id.id].value.low = value + } else { + registers[reg_id.id].value.high = value + } } - case ImmediateU8: - if reg_id.access == .Low { - registers[reg_id.id].value.low = (u8)(val) - } else { - registers[reg_id.id].value.high = (u8)(val) - } - 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) } } } diff --git a/printing.odin b/printing.odin index 60f2f6f..b6425ca 100644 --- a/printing.odin +++ b/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: - string_val = fmt.aprintf("%d", val) + case Immediate: + 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 " diff --git a/sim8086.odin b/sim8086.odin index e5f018d..4e20b2c 100644 --- a/sim8086.odin +++ b/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[:]) diff --git a/test_asm.sh b/test_asm.sh index 25ba9df..c2563fe 100755 --- a/test_asm.sh +++ b/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 diff --git a/types.odin b/types.odin index c47ff2d..b47b627 100644 --- a/types.odin +++ b/types.odin @@ -39,39 +39,35 @@ RegisterId :: struct { access: RegisterAccess, id: int, } -Immediate8 :: distinct i8 -Immediate16 :: distinct i16 -ImmediateU8 :: distinct u8 +ImmediateSize :: enum { + Signed8, + 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, - ImmediateU8, - Immediate16, + Immediate, 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,