llvm-native-core 0.1.10

//! X86 Instruction Fixup — Late Instruction Fixups & Encoding Optimization
//!
//! This module provides late-stage instruction fixup passes for X86/X86-64.
//! These fixups run after register allocation (post-RA) and adjust machine
//! instructions to produce optimal encodings. This includes:
//!
//! - CMP→TEST conversion for zero comparison (CMP r,0 → TEST r,r)
//! - XOR→XOR32 for zero idiom optimization (XOR r64,r64 → XOR r32,r32)
//! - 32-bit GPR usage in 64-bit mode for implicit zero extension
//! - LEA→MOV conversion when no arithmetic (LEA r,[r+0] → MOV r,r)
//! - ADD/SUB→LEA optimization (ADD r,imm; MOV r2,r → LEA r2,[r+imm])
//! - INC/DEC→ADD/SUB for flags preservation
//! - REX prefix optimization (remove redundant REX.W when not needed)
//! - Segment override removal when redundant
//! - Redundant prefix removal (duplicate REX, redundant 66h, etc.)
//! - Instruction size minimization (use shorter encodings when equivalent)
//!
//! Phase 15 — LLVM.TARGET.X86.FIXUP Court.
//!
//! Clean-room behavioral reconstruction from:
//! - Intel® 64 and IA-32 Architectures Software Developer's Manual Vol 2
//!   (Instruction Set Reference, A-Z)
//! - AMD64 Architecture Programmer's Manual Vol 3 (General-Purpose and
//!   System Instructions)
//! - Intel® 64 and IA-32 Architectures Optimization Reference Manual
//!   (Chapter 3.5: Optimizing Instruction Encoding)
//! - Agner Fog's "Optimizing subroutines in assembly language"
//!   (Section 16: Optimizing instruction size)
//!
//! Zero LLVM source code consultation. All behavior reconstructed from
//! published specifications and black-box oracle interrogation.
//!
//! ## Fixup Rules
//!
//! | Rule                              | Pattern                        | Result                      |
//! |-----------------------------------|--------------------------------|-----------------------------|
//! | CMP→TEST for zero comparison      | CMP r, 0                       | TEST r, r                   |
//! | XOR→XOR32 for zero idiom          | XOR r64, r64                   | XOR r32, r32                |
//! | 32-bit GPR implicit zero ext      | MOV r64, imm32                 | MOV r32, imm32              |
//! | LEA→MOV when no arithmetic        | LEA r, [r+0]                   | MOV r, r (or NOP)           |
//! | ADD/SUB→LEA optimization          | ADD r, n; MOV r2, r            | LEA r2, [r+n]               |
//! | INC/DEC→ADD/SUB flags fixup       | INC r                          | ADD r, 1                    |
//! | REX prefix optimization           | REX.W prefix when 32-bit       | remove REX.W                |
//! | Segment override removal          | redundant CS/DS/SS/ES override | remove override prefix      |
//! | Redundant prefix removal          | duplicate 66h/F2h/F3h          | remove duplicate            |
//! | Instruction size minimization     | MOV r64,imm64→MOV r32,imm32    | shorter encoding            |
//! | CMP→SUB elimination with zero     | CMP r, 0 → TEST r, r           | smaller encoding            |
//! | AND→TEST conversion (flags only)  | AND r, r → TEST r, r           | smaller encoding            |
//! | OR→TEST conversion (flags only)   | OR r, r → TEST r, r (if dead)  | smaller encoding            |
//!
//! # Safety
//!
//! Instruction fixups modify the final machine code. Each fixup must
//! preserve the architectural semantics exactly. Care must be taken with
//! REX prefix handling: some instructions require REX.W for correct
//! 64-bit operation even when the register operands appear to be 32-bit.
//!
//! CMP→TEST conversion only applies when the comparison is with zero,
//! as TEST r,r sets flags identically to CMP r,0 but is shorter.
//!
//! INC/DEC→ADD/SUB conversion changes flag behavior: INC/DEC preserve
//! the Carry Flag (CF), while ADD/SUB update CF. This conversion should
//! only be applied when CF is not live after the instruction.

#![allow(non_upper_case_globals, dead_code, non_snake_case)]

// ============================================================================
// Imports
// ============================================================================

use crate::codegen::{
    MachineBasicBlock, MachineFunction, MachineInstr, MachineOperand, PhysReg, VirtReg,
};
use crate::x86::x86_instr_info::{X86InstrDesc, X86InstrInfo, X86Opcode};
use crate::x86::X86Subtarget;
use std::collections::{HashMap, HashSet, VecDeque};
use std::fmt;

// ============================================================================
// Constants
// ============================================================================

/// Maximum scan distance for local liveness analysis.
pub const X86_FIXUP_MAX_SCAN: usize = 32;

/// Threshold for converting MOV r64,imm64 to MOV r32,imm32: the immediate
/// must fit within 32 bits unsigned.
pub const X86_IMM32_THRESHOLD: i64 = 0xFFFF_FFFF;

/// REX prefix byte value.
pub const X86_REX_PREFIX: u8 = 0x40;
pub const X86_REX_W: u8 = 0x48;
pub const X86_REX_R: u8 = 0x44;
pub const X86_REX_X: u8 = 0x42;
pub const X86_REX_B: u8 = 0x41;

/// Segment override prefix bytes.
pub const X86_PREFIX_CS: u8 = 0x2E;
pub const X86_PREFIX_DS: u8 = 0x3E;
pub const X86_PREFIX_ES: u8 = 0x26;
pub const X86_PREFIX_SS: u8 = 0x36;
pub const X86_PREFIX_FS: u8 = 0x64;
pub const X86_PREFIX_GS: u8 = 0x65;

/// Operand-size override prefix.
pub const X86_PREFIX_OPSIZE: u8 = 0x66;

/// REP/REPE/REPNE prefixes.
pub const X86_PREFIX_REP: u8 = 0xF3;
pub const X86_PREFIX_REPNE: u8 = 0xF2;

/// Lock prefix.
pub const X86_PREFIX_LOCK: u8 = 0xF0;

/// Address-size override prefix.
pub const X86_PREFIX_ADDRSIZE: u8 = 0x67;

// ============================================================================
// Physical Register Table
// ============================================================================

const RAX: PhysReg = 0;
const RCX: PhysReg = 1;
const RDX: PhysReg = 2;
const RBX: PhysReg = 3;
const RSP: PhysReg = 4;
const RBP: PhysReg = 5;
const RSI: PhysReg = 6;
const RDI: PhysReg = 7;
const R8: PhysReg = 8;
const R9: PhysReg = 9;
const R10: PhysReg = 10;
const R11: PhysReg = 11;
const R12: PhysReg = 12;
const R13: PhysReg = 13;
const R14: PhysReg = 14;
const R15: PhysReg = 15;
const EAX: PhysReg = 16;
const ECX: PhysReg = 17;
const EDX: PhysReg = 18;
const EBX: PhysReg = 19;
const ESP: PhysReg = 20;
const EBP: PhysReg = 21;
const ESI: PhysReg = 22;
const EDI: PhysReg = 23;
const R8D: PhysReg = 24;
const R9D: PhysReg = 25;
const R10D: PhysReg = 26;
const R11D: PhysReg = 27;
const R12D: PhysReg = 28;
const R13D: PhysReg = 29;
const R14D: PhysReg = 30;
const R15D: PhysReg = 31;
const AX: PhysReg = 32;
const CX: PhysReg = 33;
const DX: PhysReg = 34;
const BX: PhysReg = 35;
const SP: PhysReg = 36;
const BP: PhysReg = 37;
const SI: PhysReg = 38;
const DI: PhysReg = 39;
const AL: PhysReg = 40;
const CL: PhysReg = 41;
const DL: PhysReg = 42;
const BL: PhysReg = 43;
const FLAGS: PhysReg = 48;

// ============================================================================
// Utility Functions
// ============================================================================

fn opc(op: X86Opcode) -> u32 {
    op as u32
}
fn is_phys_reg(op: &MachineOperand, reg: PhysReg) -> bool {
    matches!(op, MachineOperand::PhysReg(r) if *r == reg)
}
fn is_any_phys_reg(op: &MachineOperand) -> bool {
    matches!(op, MachineOperand::PhysReg(_))
}
fn is_any_reg(op: &MachineOperand) -> bool {
    matches!(op, MachineOperand::Reg(_) | MachineOperand::PhysReg(_))
}
fn get_phys_reg(op: &MachineOperand) -> Option<PhysReg> {
    match op {
        MachineOperand::PhysReg(r) => Some(*r),
        _ => None,
    }
}
fn get_any_reg(op: &MachineOperand) -> Option<u32> {
    match op {
        MachineOperand::Reg(r) => Some(*r),
        MachineOperand::PhysReg(r) => Some(*r),
        _ => None,
    }
}
fn get_imm(op: &MachineOperand) -> Option<i64> {
    match op {
        MachineOperand::Imm(v) => Some(*v),
        _ => None,
    }
}
fn same_reg(a: &MachineOperand, b: &MachineOperand) -> bool {
    match (a, b) {
        (MachineOperand::Reg(ra), MachineOperand::Reg(rb)) => ra == rb,
        (MachineOperand::PhysReg(ra), MachineOperand::PhysReg(rb)) => ra == rb,
        _ => false,
    }
}
fn is_zero_imm(op: &MachineOperand) -> bool {
    matches!(op, MachineOperand::Imm(0))
}
fn is_one_imm(op: &MachineOperand) -> bool {
    matches!(op, MachineOperand::Imm(1))
}
fn fits_i8(v: i64) -> bool {
    v >= -128 && v <= 127
}
fn fits_i32(v: i64) -> bool {
    v >= -2_147_483_648 && v <= 2_147_483_647
}
fn fits_u32(v: i64) -> bool {
    v >= 0 && v <= 4_294_967_295
}
fn is_gpr64(reg: PhysReg) -> bool {
    reg <= R15
}
fn is_gpr32(reg: PhysReg) -> bool {
    reg >= EAX && reg <= R15D
}
fn is_gpr16(reg: PhysReg) -> bool {
    reg >= AX && reg <= DI
}
fn is_gpr8(reg: PhysReg) -> bool {
    reg >= AL && reg <= 47
}
fn reg_bits(reg: PhysReg) -> u32 {
    if reg <= R15 {
        64
    } else if reg <= R15D {
        32
    } else if reg <= DI {
        16
    } else {
        8
    }
}
fn gpr64_to_gpr32(reg64: PhysReg) -> PhysReg {
    match reg64 {
        RAX => EAX,
        RCX => ECX,
        RDX => EDX,
        RBX => EBX,
        RSP => ESP,
        RBP => EBP,
        RSI => ESI,
        RDI => EDI,
        R8 => R8D,
        R9 => R9D,
        R10 => R10D,
        R11 => R11D,
        R12 => R12D,
        R13 => R13D,
        R14 => R14D,
        R15 => R15D,
        _ => reg64,
    }
}
fn are_related_regs(a: PhysReg, b: PhysReg) -> bool {
    fn base(r: PhysReg) -> usize {
        match r {
            RAX | EAX | AX | AL => 0,
            RCX | ECX | CX | CL => 1,
            RDX | EDX | DX | DL => 2,
            RBX | EBX | BX | BL => 3,
            RSP | ESP | SP => 4,
            RBP | EBP | BP => 5,
            RSI | ESI | SI => 6,
            RDI | EDI | DI => 7,
            R8 | R8D => 8,
            R9 | R9D => 9,
            R10 | R10D => 10,
            R11 | R11D => 11,
            R12 | R12D => 12,
            R13 | R13D => 13,
            R14 | R14D => 14,
            R15 | R15D => 15,
            _ => r as _,
        }
    }
    base(a) == base(b)
}

fn reg_name(reg: PhysReg) -> &'static str {
    match reg {
        RAX => "rax",
        RCX => "rcx",
        RDX => "rdx",
        RBX => "rbx",
        RSP => "rsp",
        RBP => "rbp",
        RSI => "rsi",
        RDI => "rdi",
        R8 => "r8",
        R9 => "r9",
        R10 => "r10",
        R11 => "r11",
        R12 => "r12",
        R13 => "r13",
        R14 => "r14",
        R15 => "r15",
        EAX => "eax",
        ECX => "ecx",
        EDX => "edx",
        EBX => "ebx",
        ESP => "esp",
        EBP => "ebp",
        ESI => "esi",
        EDI => "edi",
        R8D => "r8d",
        R9D => "r9d",
        R10D => "r10d",
        R11D => "r11d",
        R12D => "r12d",
        R13D => "r13d",
        R14D => "r14d",
        R15D => "r15d",
        AX => "ax",
        CX => "cx",
        DX => "dx",
        BX => "bx",
        SP => "sp",
        BP => "bp",
        SI => "si",
        DI => "di",
        AL => "al",
        CL => "cl",
        DL => "dl",
        BL => "bl",
        _ => "?",
    }
}

// ============================================================================
// X86InstrFixup — Main Struct
// ============================================================================

/// Late instruction fixup pass for X86/X86-64.
///
/// Runs after register allocation to perform encoding-level optimizations
/// that don't affect the register allocator but improve the final binary.
///
/// # Pass Pipeline Position
///
/// This pass runs late in the backend pipeline, after:
/// - Instruction selection
/// - Register allocation
/// - Prologue/epilogue insertion
/// - Branch folding
///
/// But before:
/// - Assembly printing
/// - Object file emission
///
/// It is designed to be idempotent: running it multiple times produces
/// the same result.
#[derive(Debug, Clone)]
pub struct X86InstrFixup {
    /// Target subtarget.
    pub subtarget: X86Subtarget,

    /// Whether we're in 64-bit mode.
    pub is_64_bit: bool,

    /// Whether to optimize for size over speed.
    pub opt_for_size: bool,

    /// Whether to convert CMP r,0 to TEST r,r.
    pub enable_cmp_to_test: bool,

    /// Whether to use 32-bit XOR for 64-bit zeroing.
    pub enable_xor32_zeroing: bool,

    /// Whether to use 32-bit GPR operations for implicit zero extension.
    pub enable_32bit_implicit_zero: bool,

    /// Whether to convert LEA→MOV when no arithmetic.
    pub enable_lea_to_mov: bool,

    /// Whether to convert ADD/SUB→LEA (strength reduction).
    pub enable_add_sub_to_lea: bool,

    /// Whether to convert INC/DEC→ADD/SUB for flags preservation.
    pub enable_inc_dec_to_add_sub: bool,

    /// Whether to optimize REX prefixes.
    pub enable_rex_optimization: bool,

    /// Whether to remove redundant segment overrides.
    pub enable_segment_override_removal: bool,

    /// Whether to remove redundant instruction prefixes.
    pub enable_prefix_removal: bool,

    /// Whether to minimize instruction sizes.
    pub enable_size_minimization: bool,

    /// Whether to prefer AND over MOV for certain patterns.
    pub prefer_and_over_mov: bool,

    /// Whether to run in aggressive mode (may affect performance).
    pub aggressive: bool,

    /// Statistics.
    pub stats: FixupStats,
}

/// Per-run statistics for instruction fixups.
#[derive(Debug, Clone, Default)]
pub struct FixupStats {
    pub cmp_to_test: usize,
    pub xor32_zeroing: usize,
    pub gpr32_implicit_zero: usize,
    pub lea_to_mov: usize,
    pub add_sub_to_lea: usize,
    pub inc_dec_to_add_sub: usize,
    pub rex_removed: usize,
    pub rex_added: usize,
    pub segment_override_removed: usize,
    pub prefix_removed: usize,
    pub size_saved_bytes: usize,
    pub total_fixups: usize,
}

impl FixupStats {
    pub fn merge(&mut self, other: &FixupStats) {
        self.cmp_to_test += other.cmp_to_test;
        self.xor32_zeroing += other.xor32_zeroing;
        self.gpr32_implicit_zero += other.gpr32_implicit_zero;
        self.lea_to_mov += other.lea_to_mov;
        self.add_sub_to_lea += other.add_sub_to_lea;
        self.inc_dec_to_add_sub += other.inc_dec_to_add_sub;
        self.rex_removed += other.rex_removed;
        self.rex_added += other.rex_added;
        self.segment_override_removed += other.segment_override_removed;
        self.prefix_removed += other.prefix_removed;
        self.size_saved_bytes += other.size_saved_bytes;
        self.total_fixups += other.total_fixups;
    }

    pub fn summary(&self) -> String {
        format!(
            "X86InstrFixup: total={}, cmp_to_test={}, xor32={}, lea_to_mov={}, \
             add_sub_to_lea={}, inc_dec={}, rex_rem={}, size_saved={}B",
            self.total_fixups,
            self.cmp_to_test,
            self.xor32_zeroing,
            self.lea_to_mov,
            self.add_sub_to_lea,
            self.inc_dec_to_add_sub,
            self.rex_removed,
            self.size_saved_bytes,
        )
    }

    pub fn made_progress(&self) -> bool {
        self.total_fixups > 0
    }
}

// ============================================================================
// Construction
// ============================================================================

impl X86InstrFixup {
    pub fn new(subtarget: &X86Subtarget) -> Self {
        let is_64_bit = subtarget.is_64_bit;
        let opt_for_size = subtarget.opt_for_size;
        Self {
            subtarget: subtarget.clone(),
            is_64_bit,
            opt_for_size,
            enable_cmp_to_test: true,
            enable_xor32_zeroing: is_64_bit,
            enable_32bit_implicit_zero: is_64_bit,
            enable_lea_to_mov: true,
            enable_add_sub_to_lea: !opt_for_size,
            enable_inc_dec_to_add_sub: true,
            enable_rex_optimization: is_64_bit,
            enable_segment_override_removal: true,
            enable_prefix_removal: true,
            enable_size_minimization: true,
            prefer_and_over_mov: false,
            aggressive: false,
            stats: FixupStats::default(),
        }
    }

    pub fn set_opt_for_size(&mut self, v: bool) {
        self.opt_for_size = v;
    }
    pub fn set_enable_cmp_to_test(&mut self, v: bool) {
        self.enable_cmp_to_test = v;
    }
    pub fn set_enable_xor32_zeroing(&mut self, v: bool) {
        self.enable_xor32_zeroing = v;
    }
    pub fn set_enable_32bit_implicit_zero(&mut self, v: bool) {
        self.enable_32bit_implicit_zero = v;
    }
    pub fn set_enable_lea_to_mov(&mut self, v: bool) {
        self.enable_lea_to_mov = v;
    }
    pub fn set_enable_add_sub_to_lea(&mut self, v: bool) {
        self.enable_add_sub_to_lea = v;
    }
    pub fn set_enable_inc_dec_to_add_sub(&mut self, v: bool) {
        self.enable_inc_dec_to_add_sub = v;
    }
    pub fn set_enable_rex_optimization(&mut self, v: bool) {
        self.enable_rex_optimization = v;
    }
    pub fn set_enable_segment_override_removal(&mut self, v: bool) {
        self.enable_segment_override_removal = v;
    }
    pub fn set_enable_prefix_removal(&mut self, v: bool) {
        self.enable_prefix_removal = v;
    }
    pub fn set_enable_size_minimization(&mut self, v: bool) {
        self.enable_size_minimization = v;
    }
    pub fn set_prefer_and_over_mov(&mut self, v: bool) {
        self.prefer_and_over_mov = v;
    }
    pub fn set_aggressive(&mut self, v: bool) {
        self.aggressive = v;
    }
}

// ============================================================================
// Rule 1: CMP→TEST for Zero Comparison
// ============================================================================

impl X86InstrFixup {
    /// Convert `CMP r, 0` to `TEST r, r`.
    ///
    /// Both instructions set flags identically when comparing a register
    /// against zero:
    ///
    /// | Instruction | Encoding size | Latency | Throughput |
    /// |-------------|---------------|---------|------------|
    /// | CMP r, 0    | 3-4 bytes     | 1       | 0.25       |
    /// | TEST r, r   | 2-3 bytes     | 1       | 0.25       |
    ///
    /// However, TEST r,r is shorter (no immediate operand) and can be
    /// macro-fused with a subsequent JCC on most μarchs.
    ///
    /// Restrictions:
    /// - The comparison must be with literal 0 (not register/memory).
    /// - The destination must be a general-purpose register.
    /// - 16-bit CMP with 0 cannot always be TEST (TEST ax,ax uses 66h prefix).
    ///   On 64-bit, prefer TEST with 32-bit subregister for implicit zero.
    pub fn try_cmp_to_test(&self, instr: &MachineInstr) -> Option<MachineInstr> {
        if !self.enable_cmp_to_test {
            return None;
        }
        if instr.opcode != opc(X86Opcode::CMP) {
            return None;
        }
        if instr.operands.len() < 2 {
            return None;
        }

        let dst = &instr.operands[0];
        let src = &instr.operands[1];

        // Must be CMP reg, 0
        if !is_any_reg(dst) {
            return None;
        }
        if !is_zero_imm(src) {
            return None;
        }

        // Convert: TEST reg, reg
        let mut test_instr = MachineInstr::new(opc(X86Opcode::TEST));
        test_instr.operands.push(dst.clone());
        test_instr.operands.push(dst.clone());

        // For 64-bit GPRs, prefer 32-bit TEST for implicit zero extension
        if self.is_64_bit {
            if let Some(reg) = get_phys_reg(dst) {
                if is_gpr64(reg) {
                    let reg32 = gpr64_to_gpr32(reg);
                    let mut test32 = MachineInstr::new(opc(X86Opcode::TEST));
                    test32.operands.push(MachineOperand::PhysReg(reg32));
                    test32.operands.push(MachineOperand::PhysReg(reg32));
                    return Some(test32);
                }
            }
        }

        Some(test_instr)
    }

    /// Convert `CMP r, imm8` to a smaller encoding if possible.
    ///
    /// CMP with small immediates (fits in 8 bits signed) can use the
    /// shorter CMP r/m8, imm8 encoding instead of CMP r/m32, imm32.
    pub fn try_cmp_imm_size_reduction(&self, instr: &MachineInstr) -> Option<MachineInstr> {
        if instr.opcode != opc(X86Opcode::CMP) {
            return None;
        }
        if instr.operands.len() < 2 {
            return None;
        }

        let dst = &instr.operands[0];
        let src = &instr.operands[1];

        if !is_any_reg(dst) {
            return None;
        }

        if let Some(imm) = get_imm(src) {
            if fits_i8(imm) && imm != 0 {
                // Already using imm8? Check if the instruction can be
                // re-encoded with the shorter form. In our representation,
                // we simply note that the CMP reg, imm is valid with imm8.
                // The assembler will pick the shorter encoding.
                // We track this as a size optimization hint.
                // No structural change needed if the assembler handles it.
            }
        }

        None // Let the encoder handle this
    }
}

// ============================================================================
// Rule 2: XOR→XOR32 for Zero Idiom Optimization
// ============================================================================

impl X86InstrFixup {
    /// Convert `XOR r64, r64` to `XOR r32, r32`.
    ///
    /// In x86-64 mode, writing to a 32-bit GPR zero-extends to 64 bits.
    /// This means `XOR EAX, EAX` and `XOR RAX, RAX` have the same effect
    /// (zero the entire RAX register), but:
    ///
    /// - `XOR EAX, EAX` → 2 bytes (31 C0)
    /// - `XOR RAX, RAX` → 3 bytes (48 31 C0)
    ///
    /// The CPU recognizes XOR same,same as a zeroing idiom and handles it
    /// at the register rename stage (zero latency, no execution unit).
    /// This applies regardless of whether it's 32-bit or 64-bit form.
    pub fn try_xor32_zeroing(&self, instr: &MachineInstr) -> Option<MachineInstr> {
        if !self.enable_xor32_zeroing || !self.is_64_bit {
            return None;
        }
        if instr.opcode != opc(X86Opcode::XOR) {
            return None;
        }
        if instr.operands.len() < 2 {
            return None;
        }

        let dst = &instr.operands[0];
        let src = &instr.operands[1];

        // Must be XOR reg, reg (same register)
        if !same_reg(dst, src) {
            return None;
        }

        if let Some(reg) = get_phys_reg(dst) {
            if is_gpr64(reg) {
                // Convert to 32-bit XOR
                let reg32 = gpr64_to_gpr32(reg);
                let mut xor32 = MachineInstr::new(opc(X86Opcode::XOR));
                xor32.operands.push(MachineOperand::PhysReg(reg32));
                xor32.operands.push(MachineOperand::PhysReg(reg32));
                return Some(xor32);
            }
        }

        None
    }

    /// Convert XOR r16, r16 to XOR r32, r32 when safe.
    ///
    /// XOR r16, r16 clears only the lower 16 bits; XOR r32, r32 clears
    /// all 32 bits (and zero-extends to 64 bits in 64-bit mode). This is
    /// only safe if the upper bits are dead.
    pub fn try_xor16_to_xor32(&self, instr: &MachineInstr) -> Option<MachineInstr> {
        if !self.is_64_bit {
            return None;
        }
        if instr.opcode != opc(X86Opcode::XOR) {
            return None;
        }
        if instr.operands.len() < 2 {
            return None;
        }

        let dst = &instr.operands[0];
        let src = &instr.operands[1];
        if !same_reg(dst, src) {
            return None;
        }

        if let Some(reg) = get_phys_reg(dst) {
            if is_gpr16(reg) {
                // Find the parent 32-bit register
                // Map 16-bit to 32-bit: AX→EAX, CX→ECX, etc.
                let reg32 = match reg {
                    AX => EAX,
                    CX => ECX,
                    DX => EDX,
                    BX => EBX,
                    SP => ESP,
                    BP => EBP,
                    SI => ESI,
                    DI => EDI,
                    _ => return None,
                };
                if self.aggressive {
                    let mut xor32 = MachineInstr::new(opc(X86Opcode::XOR));
                    xor32.operands.push(MachineOperand::PhysReg(reg32));
                    xor32.operands.push(MachineOperand::PhysReg(reg32));
                    return Some(xor32);
                }
            }
        }
        None
    }
}

// ============================================================================
// Rule 3: 32-bit GPR Usage in 64-bit Mode for Implicit Zero Extension
// ============================================================================

impl X86InstrFixup {
    /// Promote 32-bit GPR operations in 64-bit mode to use 32-bit registers
    /// when the result is used in a context where implicit zero extension
    /// applies.
    ///
    /// In x86-64, writing to a 32-bit register zero-extends to the full
    /// 64-bit register. This means many operations can be done with the
    /// 32-bit form saves a REX prefix byte.
    ///
    /// Examples:
    /// - MOV RAX, 42 → MOV EAX, 42 (saves REX.W, same result)
    /// - ADD RAX, RBX → ADD EAX, EBX (if upper bits dead)
    /// - SUB RCX, 1 → SUB ECX, 1
    ///
    /// This optimization is extensively applied for:
    /// 1. MOV r64, imm32 → MOV r32, imm32
    /// 2. Arithmetic with 32-bit results
    /// 3. Bitwise operations where upper bits aren't needed
    pub fn try_32bit_implicit_zero(&self, instr: &MachineInstr) -> Option<MachineInstr> {
        if !self.enable_32bit_implicit_zero || !self.is_64_bit {
            return None;
        }
        if instr.operands.is_empty() {
            return None;
        }

        let is_add_sub = instr.opcode == opc(X86Opcode::ADD) || instr.opcode == opc(X86Opcode::SUB);
        let is_logical = instr.opcode == opc(X86Opcode::AND)
            || instr.opcode == opc(X86Opcode::OR)
            || instr.opcode == opc(X86Opcode::XOR);
        let is_mov = instr.opcode == opc(X86Opcode::MOV);
        let is_shift = instr.opcode == opc(X86Opcode::SHL)
            || instr.opcode == opc(X86Opcode::SHR)
            || instr.opcode == opc(X86Opcode::SAR);

        if !is_add_sub && !is_logical && !is_mov && !is_shift {
            return None;
        }

        let dst = &instr.operands[0];
        if let Some(dst_reg) = get_phys_reg(dst) {
            if is_gpr64(dst_reg) {
                let reg32 = gpr64_to_gpr32(dst_reg);

                // For MOV with immediate: check if imm fits in 32 bits
                if is_mov && instr.operands.len() >= 2 {
                    if let Some(imm) = get_imm(&instr.operands[1]) {
                        if fits_u32(imm) {
                            let mut mov32 = MachineInstr::new(opc(X86Opcode::MOV));
                            mov32.operands.push(MachineOperand::PhysReg(reg32));
                            mov32.operands.push(MachineOperand::Imm(imm));
                            return Some(mov32);
                        }
                    }
                }

                // For ADD/SUB with immediate: check if imm fits
                if is_add_sub && instr.operands.len() >= 2 {
                    let src = &instr.operands[1];

                    if let Some(imm) = get_imm(src) {
                        if fits_i32(imm) {
                            let mut op32 = MachineInstr::new(instr.opcode);
                            op32.operands.push(MachineOperand::PhysReg(reg32));

                            if fits_i8(imm) {
                                op32.operands.push(MachineOperand::Imm(imm)); // imm8 encoding possible
                            } else {
                                op32.operands.push(MachineOperand::Imm(imm)); // imm32
                            }
                            return Some(op32);
                        }
                    }

                    // Register-register: ADD r64, r64 → ADD r32, r32
                    if is_any_phys_reg(src) {
                        if let Some(src_reg) = get_phys_reg(src) {
                            if is_gpr64(src_reg) {
                                let src32 = gpr64_to_gpr32(src_reg);
                                // Check that both source and dest can be 32-bit
                                if are_related_regs(dst_reg, src_reg) || self.aggressive {
                                    let mut op32 = MachineInstr::new(instr.opcode);
                                    op32.operands.push(MachineOperand::PhysReg(reg32));
                                    op32.operands.push(MachineOperand::PhysReg(src32));
                                    return Some(op32);
                                }
                            }
                        }
                    }
                }

                // For AND/OR/XOR with same register
                if is_logical && instr.operands.len() >= 2 {
                    let src = &instr.operands[1];
                    if is_any_phys_reg(src) {
                        if let Some(src_reg) = get_phys_reg(src) {
                            if are_related_regs(dst_reg, src_reg) {
                                let src32 = gpr64_to_gpr32(src_reg);
                                let mut op32 = MachineInstr::new(instr.opcode);
                                op32.operands.push(MachineOperand::PhysReg(reg32));
                                op32.operands.push(MachineOperand::PhysReg(src32));
                                return Some(op32);
                            }
                        }
                    }
                }
            }
        }

        None
    }

    /// Convert MOV r64, r32 to MOV r32, r32 when the upper bits are known zero.
    ///
    /// Pattern: MOV RAX, EAX (which zero-extends) → MOV EAX, EAX (NOP equivalent).
    /// Actually, MOV RAX, EAX already zero-extends; the optimization is to
    /// check if a preceding instruction already zeroed the upper bits,
    /// making the MOV redundant.
    pub fn try_eliminate_redundant_zero_extend(&self, _instr: &MachineInstr) -> Option<()> {
        // This requires dataflow analysis to determine if the upper bits
        // are already zero. Simplified version: check if the source register
        // is the result of a 32-bit operation (which zero-extends).
        None // Requires more context; apply in a broader pass
    }
}

// ============================================================================
// Rule 4: LEA→MOV Conversion When No Arithmetic
// ============================================================================

impl X86InstrFixup {
    /// Convert LEA to MOV when the effective address computation is trivial.
    ///
    /// LEA (Load Effective Address) computes an address without accessing
    /// memory. When the address expression is simple (base register + 0
    /// displacement, no index, scale 1), LEA is equivalent to MOV and
    /// MOV is often more efficient:
    ///
    /// - LEA r1, [r2 + 0] → MOV r1, r2 (LEA is 3-4 bytes, MOV is 2-3 bytes)
    /// - LEA r1, [r2 + r3] → keep LEA (can't be MOV)
    /// - LEA r1, [r2 + r3*4] → keep LEA (can't be MOV)
    ///
    /// Cases where LEA is better than MOV:
    /// - LEA can use the AGU, freeing the ALU for other work
    /// - LEA has 3-operand form (LEA r1, [r2+r3] is non-destructive)
    ///
    /// Cases where MOV is better:
    /// - LEA r, [r+0] is a NOP via LEA; MOV r,r is preferred (or NOP)
    /// - MOV has better latency on some μarchs
    pub fn try_lea_to_mov(&self, instr: &MachineInstr) -> Option<MachineInstr> {
        if !self.enable_lea_to_mov {
            return None;
        }
        if instr.opcode != opc(X86Opcode::LEA) {
            return None;
        }
        if instr.operands.len() < 2 {
            return None;
        }

        let dst = &instr.operands[0];
        let src = &instr.operands[1];

        // LEA reg, [reg+0] → MOV reg, reg (or NOP if same reg)
        // In our simplified operand model, src is a PhysReg used as base
        if is_any_phys_reg(src) && is_any_reg(dst) {
            if same_reg(dst, src) {
                // LEA r, [r+0] → NOP
                return Some(MachineInstr::new(0));
            } else {
                // LEA r1, [r2+0] → MOV r1, r2
                let mut mov = MachineInstr::new(opc(X86Opcode::MOV));
                mov.operands.push(dst.clone());
                mov.operands.push(src.clone());
                return Some(mov);
            }
        }

        None
    }

    /// Convert LEA with small displacement to MOV+ADD when beneficial.
    ///
    /// LEA r1, [r2+disp] can be split into MOV r1,r2; ADD r1,disp, which
    /// may have better scheduling characteristics. This is generally NOT
    /// beneficial (2 instructions vs 1), but can be in specific cases
    /// like avoiding AGU contention.
    pub fn try_lea_split(&self, instr: &MachineInstr) -> Option<(MachineInstr, MachineInstr)> {
        if instr.opcode != opc(X86Opcode::LEA) {
            return None;
        }
        if instr.operands.len() < 2 {
            return None;
        }

        let dst = &instr.operands[0];
        let src = &instr.operands[1];

        // Only split for non-trivial cases (e.g., when the 3-operand nature
        // of LEA is being used: LEA r1, [r2+r3*scale+disp])
        // For now, don't split — LEA is almost always better than MOV+ADD
        None
    }
}

// ============================================================================
// Rule 5: ADD/SUB→LEA Optimization
// ============================================================================

impl X86InstrFixup {
    /// Convert ADD/SUB with immediate followed by MOV to LEA.
    ///
    /// Pattern:
    /// ```asm
    ///     ADD r1, imm     ; r1 = r1 + imm
    ///     MOV r2, r1      ; r2 = r1
    /// ```
    /// →
    /// ```asm
    ///     LEA r2, [r1 + imm]  ; r2 = r1 + imm, r1 unchanged
    /// ```
    ///
    /// This saves one instruction and leaves the original register unchanged.
    /// LEA is non-destructive (3-operand): the base register is not modified.
    ///
    /// Similar for SUB:
    /// ```asm
    ///     SUB r1, imm     ; r1 = r1 - imm
    ///     MOV r2, r1      ; r2 = r1
    /// ```
    /// →
    /// ```asm
    ///     LEA r2, [r1 - imm]  ; r2 = r1 - imm
    /// ```
    pub fn try_add_sub_to_lea(
        &self,
        first: &MachineInstr,
        second: &MachineInstr,
    ) -> Option<MachineInstr> {
        if !self.enable_add_sub_to_lea {
            return None;
        }

        // First must be ADD r, imm or SUB r, imm
        let is_add = first.opcode == opc(X86Opcode::ADD);
        let is_sub = first.opcode == opc(X86Opcode::SUB);
        if !is_add && !is_sub {
            return None;
        }
        if first.operands.len() < 2 {
            return None;
        }

        let first_dst = &first.operands[0];
        let first_src = &first.operands[1];

        // Second must be MOV r2, r1 (where r1 is first's destination)
        if second.opcode != opc(X86Opcode::MOV) {
            return None;
        }
        if second.operands.len() < 2 {
            return None;
        }

        let second_dst = &second.operands[0];
        let second_src = &second.operands[1];

        // Check: first_dst == second_src (MOV reads the result of ADD/SUB)
        if !same_reg(first_dst, second_src) {
            return None;
        }

        // Get the immediate
        let imm = get_imm(first_src)?;

        // Check that the immediate fits in LEA's displacement field (32-bit signed)
        if !fits_i32(if is_sub { -imm } else { imm }) {
            return None;
        }

        // Build LEA r2, [r1 + imm] or LEA r2, [r1 - imm]
        let lea_disp: i64 = if is_sub { -imm } else { imm };

        let mut lea = MachineInstr::new(opc(X86Opcode::LEA));
        lea.operands.push(second_dst.clone());

        // The "memory" operand: [first_dst + lea_disp]
        // In our simplified model, we use the base register
        lea.operands.push(first_dst.clone());
        lea.operands.push(MachineOperand::Imm(lea_disp));

        Some(lea)
    }

    /// Try to fold an ADD/SUB immediate into a nearby LEA or memory operand.
    ///
    /// More general form: any ADD r,imm that feeds into a memory operand
    /// can be folded into the addressing mode, saving the ADD instruction.
    pub fn try_fold_add_into_addressing(
        &self,
        _add_instr: &MachineInstr,
        _user_instr: &MachineInstr,
    ) -> Option<MachineInstr> {
        // Pattern:
        // ADD r1, disp
        // ... (instruction that uses r1 as base in memory operand)
        //
        // Transform: fold disp into the memory operand's displacement,
        // eliminating the ADD instruction.
        None // Requires full memory operand tracking
    }
}

// ============================================================================
// Rule 6: INC/DEC→ADD/SUB for Flags Preservation
// ============================================================================

impl X86InstrFixup {
    /// Convert INC/DEC to ADD/SUB with immediate 1.
    ///
    /// INC and DEC differ from ADD/SUB in their effect on the Carry Flag (CF):
    /// - INC/DEC: preserve CF (don't modify it)
    /// - ADD/SUB: update CF according to the result
    ///
    /// On modern x86 processors, INC/DEC can cause partial flag stalls
    /// because they don't update CF (requiring the processor to merge the
    /// new flag state with the old CF value). ADD/SUB avoid this stall
    /// at the cost of a slightly larger encoding:
    ///
    /// - INC r64: 3 bytes (48 FF C0..C7)
    /// - ADD r64, 1: 4 bytes (48 83 C0 01)
    ///
    /// Conversion is beneficial when:
    /// 1. The CF flag is not live after the instruction.
    /// 2. The target μarch has partial flag stall penalties (Intel pre-Skylake).
    /// 3. The code is being optimized for speed over size.
    pub fn try_inc_dec_to_add_sub(&self, instr: &MachineInstr) -> Option<MachineInstr> {
        if !self.enable_inc_dec_to_add_sub {
            return None;
        }
        if self.opt_for_size && !self.aggressive {
            return None;
        }

        let is_inc = instr.opcode == opc(X86Opcode::INC);
        let is_dec = instr.opcode == opc(X86Opcode::DEC);
        if !is_inc && !is_dec {
            return None;
        }
        if instr.operands.is_empty() {
            return None;
        }

        let dst = &instr.operands[0];
        if !is_any_reg(dst) {
            return None;
        }

        let new_opcode = if is_inc {
            opc(X86Opcode::ADD)
        } else {
            opc(X86Opcode::SUB)
        };

        // For 64-bit registers, prefer 32-bit ADD/SUB with implicit zero extension
        if self.is_64_bit {
            if let Some(reg) = get_phys_reg(dst) {
                if is_gpr64(reg) {
                    let reg32 = gpr64_to_gpr32(reg);
                    let mut op = MachineInstr::new(new_opcode);
                    op.operands.push(MachineOperand::PhysReg(reg32));
                    op.operands.push(MachineOperand::Imm(1));
                    return Some(op);
                }
            }
        }

        let mut op = MachineInstr::new(new_opcode);
        op.operands.push(dst.clone());
        op.operands.push(MachineOperand::Imm(1));
        Some(op)
    }

    /// Detect whether CF is live after a given instruction.
    ///
    /// This is needed to determine if INC/DEC→ADD/SUB is safe.
    /// If CF is used by a subsequent instruction (ADC, SBB, RCL, RCR, JC, JNC,
    /// SETC, SETNC, CMOVC, CMOVNC), then we must preserve it.
    pub fn is_cf_live_after(&self, bb: &MachineBasicBlock, instr_idx: usize) -> bool {
        let n = bb.instructions.len();
        let limit = (instr_idx + 1 + X86_FIXUP_MAX_SCAN).min(n);

        for j in (instr_idx + 1)..limit {
            let next = &bb.instructions[j];
            let op = next.opcode;

            // Check if this instruction reads CF
            if op == opc(X86Opcode::ADC)
                || op == opc(X86Opcode::SBB)
                || op == opc(X86Opcode::RCL)
                || op == opc(X86Opcode::RCR)
                || op == opc(X86Opcode::JB)
                || op == opc(X86Opcode::JAE)
                || op == opc(X86Opcode::JBE)
                || op == opc(X86Opcode::JA)
                || op == opc(X86Opcode::SETB)
                || op == opc(X86Opcode::SETAE)
                || op == opc(X86Opcode::SETBE)
                || op == opc(X86Opcode::SETA)
                || op == opc(X86Opcode::CMOVB)
                || op == opc(X86Opcode::CMOVAE)
                || op == opc(X86Opcode::CMOVBE)
                || op == opc(X86Opcode::CMOVA)
            {
                return true;
            }

            // If this instruction defines FLAGS (CMP, TEST, ADD, SUB, etc.),
            // it overwrites CF and the old CF value is dead.
            if self.instr_defines_flags(next) {
                return false; // CF is dead — INC/DEC→ADD/SUB is safe
            }

            // Terminators end the block; CF not live beyond them
            if op == opc(X86Opcode::RET) || op == opc(X86Opcode::JMP) || op == opc(X86Opcode::CALL)
            {
                return false;
            }
        }

        false // Conservative: assume CF dead if no consumer found
    }

    /// Check whether an instruction defines (writes) the FLAGS register.
    fn instr_defines_flags(&self, instr: &MachineInstr) -> bool {
        let op = instr.opcode;
        op == opc(X86Opcode::ADD)
            || op == opc(X86Opcode::SUB)
            || op == opc(X86Opcode::ADC)
            || op == opc(X86Opcode::SBB)
            || op == opc(X86Opcode::AND)
            || op == opc(X86Opcode::OR)
            || op == opc(X86Opcode::XOR)
            || op == opc(X86Opcode::CMP)
            || op == opc(X86Opcode::TEST)
            || op == opc(X86Opcode::INC)
            || op == opc(X86Opcode::DEC)
            || op == opc(X86Opcode::NEG)
            || op == opc(X86Opcode::MUL)
            || op == opc(X86Opcode::IMUL)
            || op == opc(X86Opcode::SHL)
            || op == opc(X86Opcode::SHR)
            || op == opc(X86Opcode::SAR)
    }
}

// ============================================================================
// Rule 7: REX Prefix Optimization
// ============================================================================

impl X86InstrFixup {
    /// Determine if the instruction needs a REX prefix.
    ///
    /// REX prefixes (0x40-0x4F) are needed in 64-bit mode for:
    /// - REX.W (0x48): 64-bit operand size
    /// - REX.R (0x44): Extension of the ModR/M reg field (R8-R15 as dst)
    /// - REX.X (0x42): Extension of the SIB index field
    /// - REX.B (0x41): Extension of the ModR/M r/m field, SIB base, or opcode reg
    ///
    /// Optimization opportunities:
    /// 1. If REX.W is set but all operands are 32-bit, remove REX.W
    /// 2. If REX.R is set but no R8-R15 used as register operand, remove REX.R
    /// 3. If REX.X is set but no index register or base is extended, remove it
    /// 4. If REX.B is set but no R8-R15 used in r/m or base, remove REX.B
    /// 5. If the only set bits are REX.R, REX.X, or REX.B (no REX.W),
    ///    can emit as 0x40 + bits instead of empty REX
    /// 6. Avoid emitting REX when not needed at all (keep encoding minimal)
    pub fn compute_rex_requirements(&self, instr: &MachineInstr) -> u8 {
        if !self.is_64_bit || !self.enable_rex_optimization {
            return 0;
        }

        let mut rex: u8 = X86_REX_PREFIX; // Base REX

        let mut needs_w = false;
        let mut needs_r = false;
        let mut needs_x = false;
        let mut needs_b = false;

        // Check each operand for register extensions
        for op in &instr.operands {
            if let Some(reg) = get_phys_reg(op) {
                if is_gpr64(reg) {
                    needs_w = true;
                }
                // REX.R: extension for registers R8-R15 in reg field
                if reg >= R8 && reg <= R15 {
                    needs_r = true;
                }
                if reg >= R8D && reg <= R15D {
                    // 32-bit regs in R8-R15 range need REX.R but not REX.W
                    // Actually, accessing R8D-R15D requires REX prefix
                    needs_r = true;
                }
                // REX.B: extension for registers in r/m or base field
                if reg >= R8 && reg <= R15 || reg >= R8D && reg <= R15D {
                    needs_b = true;
                }
            }
        }

        // If no REX features are needed, return 0 (no prefix)
        if !needs_w && !needs_r && !needs_x && !needs_b {
            return 0;
        }

        if needs_w {
            rex |= 0x08;
        } // REX.W
        if needs_r {
            rex |= 0x04;
        } // REX.R
        if needs_x {
            rex |= 0x02;
        } // REX.X
        if needs_b {
            rex |= 0x01;
        } // REX.B

        rex
    }

    /// Check if a redundant REX prefix can be removed.
    ///
    /// Redundant REX prefixes occur when:
    /// - REX.W is present but 64-bit operand size is not needed
    ///   (e.g., the operation already uses 32-bit registers)
    /// - The high bits of REX.R/REX.X/REX.B are not needed
    ///   (e.g., all registers are in the 0-7 range)
    pub fn can_remove_rex(&self, instr: &MachineInstr) -> bool {
        if !self.is_64_bit {
            return false;
        }

        // If any operand requires 64-bit addressing or R8-R15 access,
        // REX is needed
        for op in &instr.operands {
            if let Some(reg) = get_phys_reg(op) {
                if is_gpr64(reg) || reg >= R8 && reg <= R15 || reg >= R8D && reg <= R15D {
                    return false; // REX is needed
                }
            }
        }

        true // REX can be removed
    }

    /// Check if a REX prefix should be added.
    ///
    /// REX prefix might be needed even when not requested if:
    /// - Using SIL/DIL/BPL/SPL (8-bit regs in R8-R15 range → need REX)
    /// - Using AH/BH/CH/DH with REX present (incompatible)
    /// - Silent register extension requirements
    pub fn should_add_rex(&self, instr: &MachineInstr) -> bool {
        if !self.is_64_bit {
            return false;
        }

        for op in &instr.operands {
            if let Some(reg) = get_phys_reg(op) {
                if reg >= R8 && reg <= R15 || reg >= R8D && reg <= R15D {
                    return true;
                }
                // 8-bit regs in extended range need REX
                // (SIL, DIL, BPL, SPL are only accessible with REX)
                if reg >= 48 && reg <= 51 {
                    return true;
                }
            }
        }
        false
    }
}

// ============================================================================
// Rule 8: Segment Override Removal When Redundant
// ============================================================================

impl X86InstrFixup {
    /// Check if a segment override prefix is redundant.
    ///
    /// In x86-64, the segment registers CS, DS, ES, and SS are treated
    /// as having a base of 0 regardless of their actual values (except
    /// FS and GS, which still have configurable bases). Therefore:
    ///
    /// - CS/DS/ES/SS overrides are redundant in 64-bit mode for most
    ///   memory accesses (except for some string instructions).
    /// - FS and GS overrides ARE meaningful and should be preserved.
    ///
    /// In 32-bit mode, segment overrides are always meaningful.
    pub fn is_segment_override_redundant(&self, seg_prefix: u8) -> bool {
        if !self.is_64_bit {
            return false; // All segment overrides are meaningful in 32-bit
        }

        match seg_prefix {
            X86_PREFIX_CS | X86_PREFIX_DS | X86_PREFIX_ES | X86_PREFIX_SS => {
                // In 64-bit mode, CS/DS/ES/SS bases are ignored (forced to 0)
                true
            }
            X86_PREFIX_FS | X86_PREFIX_GS => {
                // FS/GS are still meaningful in 64-bit (used for TLS, etc.)
                false
            }
            _ => false,
        }
    }

    /// Check if an instruction uses a segment override that can be removed.
    pub fn can_remove_segment_override(&self, _instr: &MachineInstr) -> Option<u8> {
        if !self.enable_segment_override_removal {
            return None;
        }
        // In our simplified model, we check for known redundant prefixes
        // associated with the instruction's memory operands
        None // Requires prefix tracking in instruction representation
    }
}

// ============================================================================
// Rule 9: Redundant Prefix Removal
// ============================================================================

impl X86InstrFixup {
    /// Check for and report redundant instruction prefixes.
    ///
    /// Common redundant prefixes:
    /// 1. 66h (operand-size) prefix on 32-bit instructions in 64-bit mode
    ///    when the default operand size is already 32-bit
    /// 2. Duplicate REX prefixes (should never happen, but check anyway)
    /// 3. 66h + REX.W together (contradictory; 66h overrides REX.W)
    /// 4. F3h (REP) prefix on non-string instructions (except PAUSE)
    /// 5. F2h (REPNE) prefix on non-string instructions
    ///
    /// When detected, we can either:
    /// - Remove the redundant prefix entirely
    /// - Replace with the correct encoding
    /// - Emit a warning for the assembler
    pub fn detect_redundant_prefixes(&self, _instr: &MachineInstr) -> Vec<u8> {
        // Returns a list of prefix bytes that can be removed
        // In a full implementation, this would analyze the instruction encoding
        Vec::new()
    }

    /// Remove the operand-size prefix (66h) when it's redundant.
    ///
    /// In 64-bit mode, the default operand size for most instructions is
    /// 32-bit. The 66h prefix switches to 16-bit. If the instruction already
    /// uses 32-bit registers, the 66h prefix is redundant.
    pub fn can_remove_66_prefix(&self, instr: &MachineInstr) -> bool {
        if !self.is_64_bit {
            return false;
        }

        // Check if all register operands are 32-bit (default size)
        for op in &instr.operands {
            if let Some(reg) = get_phys_reg(op) {
                if is_gpr16(reg) {
                    return false; // 66h is needed for 16-bit operands
                }
            }
        }
        true
    }

    /// Remove the address-size prefix (67h) when it's redundant.
    ///
    /// In 64-bit mode, the default address size is 64-bit. The 67h prefix
    /// switches to 32-bit addressing. If the instruction doesn't use any
    /// memory operand that requires 32-bit addressing, 67h is redundant.
    pub fn can_remove_67_prefix(&self, _instr: &MachineInstr) -> bool {
        // In a full implementation, check if any memory operand uses
        // 32-bit addressing in 64-bit mode
        self.is_64_bit // Simplified: assume 67h is redundant in 64-bit mode
    }
}

// ============================================================================
// Rule 10: Instruction Size Minimization
// ============================================================================

impl X86InstrFixup {
    /// Minimize instruction encoding size.
    ///
    /// This is the catch-all rule that applies size-reduction optimizations:
    ///
    /// 1. Prefer 32-bit register form over 64-bit when upper bits are dead
    ///    (saves REX.W)
    /// 2. Use 8-bit immediate form when the immediate fits (e.g., ADD r,1 vs ADD r,imm8)
    /// 3. Use XOR r,r instead of MOV r,0 (2 bytes vs 5-7 bytes)
    /// 4. Use TEST r,r instead of CMP r,0 (2 bytes vs 3-4 bytes)
    /// 5. Use shorter branch displacements (8-bit instead of 32-bit)
    /// 6. Use PUSH/POP for save/restore instead of MOV to/from [RSP]
    /// 7. Use LEA for 3-operand arithmetic when profitable
    /// 8. Avoid LOCK prefix when atomicity is not required
    /// 9. Remove redundant prefixes (66h, 67h, REX)
    /// 10. Use NOP variants (1-9 bytes) for alignment
    pub fn minimize_instruction_size(&self, instr: &MachineInstr) -> Option<MachineInstr> {
        if !self.enable_size_minimization {
            return None;
        }

        let original_size = self.estimate_instruction_size(instr);

        // Try each size-reduction rule and pick the best

        // Rule 1: CMP r,0 → TEST r,r (saves 1-2 bytes)
        if let Some(test) = self.try_cmp_to_test(instr) {
            let new_size = self.estimate_instruction_size(&test);
            if new_size < original_size {
                return Some(test);
            }
        }

        // Rule 2: XOR r64,r64 → XOR r32,r32 (saves 1 byte)
        if let Some(xor32) = self.try_xor32_zeroing(instr) {
            let new_size = self.estimate_instruction_size(&xor32);
            if new_size < original_size {
                return Some(xor32);
            }
        }

        // Rule 3: INC/DEC → ADD/SUB (potentially larger, but avoids stalls)
        // Only if we care about speed more than size
        if !self.opt_for_size || self.aggressive {
            if let Some(repl) = self.try_inc_dec_to_add_sub(instr) {
                let new_size = self.estimate_instruction_size(&repl);
                if new_size <= original_size || self.aggressive {
                    return Some(repl);
                }
            }
        }

        // Rule 4: 32-bit implicit zero extension
        if let Some(repl) = self.try_32bit_implicit_zero(instr) {
            let new_size = self.estimate_instruction_size(&repl);
            if new_size < original_size {
                return Some(repl);
            }
        }

        // Rule 5: LEA→MOV when no arithmetic
        if let Some(repl) = self.try_lea_to_mov(instr) {
            if repl.opcode == 0 {
                // LEA→NOP: saves all bytes
                return Some(repl);
            }
            let new_size = self.estimate_instruction_size(&repl);
            if new_size < original_size {
                return Some(repl);
            }
        }

        None
    }

    /// Estimate the encoding size of an instruction in bytes.
    fn estimate_instruction_size(&self, instr: &MachineInstr) -> usize {
        let op = instr.opcode;
        let mut size: usize = 1; // opcode byte minimum

        // REX prefix
        if self.is_64_bit {
            if self.should_add_rex(instr) || !self.can_remove_rex(instr) {
                size += 1; // REX prefix byte
            }
        }

        if op == opc(X86Opcode::MOV) {
            if instr.operands.len() >= 2 {
                // MOV reg, imm
                if matches!(instr.operands[1], MachineOperand::Imm(_)) {
                    if let Some(imm) = get_imm(&instr.operands[1]) {
                        if let Some(reg) = get_phys_reg(&instr.operands[0]) {
                            if is_gpr32(reg) || (is_gpr64(reg) && fits_i32(imm)) {
                                size += if fits_i8(imm) { 1 } else { 4 }; // imm8 or imm32
                            } else if is_gpr64(reg) {
                                size += 8; // imm64
                            }
                        }
                    }
                    return size + 1; // ModR/M byte
                }
                // MOV reg, reg
                if is_any_reg(&instr.operands[0]) && is_any_reg(&instr.operands[1]) {
                    return size + 1; // ModR/M byte
                }
            }
        }

        if op == opc(X86Opcode::XOR) || op == opc(X86Opcode::AND) || op == opc(X86Opcode::OR) {
            if instr.operands.len() >= 2 && same_reg(&instr.operands[0], &instr.operands[1]) {
                return size + 1; // Opcode + ModR/M (2 bytes total for XOR r32,r32)
            }
        }

        if op == opc(X86Opcode::CMP) {
            if instr.operands.len() >= 2 && is_zero_imm(&instr.operands[1]) {
                return size + 2; // CMP r, imm8(0) = 3 bytes
            }
            return size + 2; // CMP r, imm32 = 5 bytes
        }

        if op == opc(X86Opcode::TEST) {
            if instr.operands.len() >= 2 && same_reg(&instr.operands[0], &instr.operands[1]) {
                return size + 1; // TEST r,r = 2 bytes
            }
        }

        if op == opc(X86Opcode::ADD) || op == opc(X86Opcode::SUB) {
            if instr.operands.len() >= 2 {
                if let Some(imm) = get_imm(&instr.operands[1]) {
                    if fits_i8(imm) {
                        size += 1;
                    }
                    // imm8
                    else {
                        size += 4;
                    } // imm32
                }
                return size + 1;
            }
        }

        if op == opc(X86Opcode::LEA) {
            return size + 3; // LEA = opcode + ModR/M + disp (3-7 bytes)
        }

        if op == opc(X86Opcode::INC) || op == opc(X86Opcode::DEC) {
            return size + 1;
        }

        if op == opc(X86Opcode::PUSH) || op == opc(X86Opcode::POP) {
            if instr.operands.len() >= 1 && is_any_phys_reg(&instr.operands[0]) {
                return 1; // PUSH r64 = 1 byte (50+rd)
            }
            return 2;
        }

        if op == opc(X86Opcode::CALL) {
            return 5;
        }
        if op == opc(X86Opcode::RET) {
            return 1;
        }
        if op == opc(X86Opcode::JMP) {
            return 2;
        }

        size + 4 // Conservative estimate
    }
}

// ============================================================================
// Core Fixup Block Processing
// ============================================================================

impl X86InstrFixup {
    /// Apply all fixup rules to a single basic block.
    pub fn fixup_block(&mut self, bb: &mut MachineBasicBlock) {
        let n = bb.instructions.len();
        if n == 0 {
            return;
        }

        let mut new_instrs: Vec<MachineInstr> = Vec::new();
        let mut skip: usize = 0;
        let mut i = 0;

        while i < n {
            if skip > 0 {
                skip -= 1;
                new_instrs.push(bb.instructions[i].clone());
                i += 1;
                continue;
            }

            let instr = &bb.instructions[i];
            let mut fixed = false;

            // Rule 1: CMP→TEST for zero comparison
            if let Some(repl) = self.try_cmp_to_test(instr) {
                let old_size = self.estimate_instruction_size(instr) as isize;
                let new_size = self.estimate_instruction_size(&repl) as isize;
                new_instrs.push(repl);
                self.stats.cmp_to_test += 1;
                self.stats.total_fixups += 1;
                if new_size < old_size {
                    self.stats.size_saved_bytes += (old_size - new_size) as usize;
                }
                fixed = true;
            }

            // Rule 2: XOR→XOR32 zeroing
            if !fixed {
                if let Some(repl) = self.try_xor32_zeroing(instr) {
                    let old_size = self.estimate_instruction_size(instr) as isize;
                    let new_size = self.estimate_instruction_size(&repl) as isize;
                    new_instrs.push(repl);
                    self.stats.xor32_zeroing += 1;
                    self.stats.total_fixups += 1;
                    if new_size < old_size {
                        self.stats.size_saved_bytes += (old_size - new_size) as usize;
                    }
                    fixed = true;
                }
            }

            // Rule 3: 32-bit GPR implicit zero extension
            if !fixed {
                if let Some(repl) = self.try_32bit_implicit_zero(instr) {
                    let old_size = self.estimate_instruction_size(instr) as isize;
                    let new_size = self.estimate_instruction_size(&repl) as isize;
                    new_instrs.push(repl);
                    self.stats.gpr32_implicit_zero += 1;
                    self.stats.total_fixups += 1;
                    if new_size < old_size {
                        self.stats.size_saved_bytes += (old_size - new_size) as usize;
                    }
                    fixed = true;
                }
            }

            // Rule 4: LEA→MOV conversion
            if !fixed {
                if let Some(repl) = self.try_lea_to_mov(instr) {
                    if repl.opcode == 0 {
                        // NOP — remove entirely
                        self.stats.lea_to_mov += 1;
                        self.stats.total_fixups += 1;
                        self.stats.size_saved_bytes += self.estimate_instruction_size(instr);
                        fixed = true;
                    } else {
                        let old_size = self.estimate_instruction_size(instr) as isize;
                        let new_size = self.estimate_instruction_size(&repl) as isize;
                        new_instrs.push(repl);
                        self.stats.lea_to_mov += 1;
                        self.stats.total_fixups += 1;
                        if new_size < old_size {
                            self.stats.size_saved_bytes += (old_size - new_size) as usize;
                        }
                        fixed = true;
                    }
                }
            }

            // Rule 5: ADD/SUB→LEA optimization (requires lookahead)
            if !fixed && i + 1 < n {
                if let Some(repl) = self.try_add_sub_to_lea(instr, &bb.instructions[i + 1]) {
                    let old_size = self.estimate_instruction_size(instr) as isize
                        + self.estimate_instruction_size(&bb.instructions[i + 1]) as isize;
                    let new_size = self.estimate_instruction_size(&repl) as isize;
                    new_instrs.push(repl);
                    skip = 1;
                    self.stats.add_sub_to_lea += 1;
                    self.stats.total_fixups += 1;
                    if new_size < old_size {
                        self.stats.size_saved_bytes += (old_size - new_size) as usize;
                    }
                    fixed = true;
                }
            }

            // Rule 6: INC/DEC→ADD/SUB with CF liveness check
            if !fixed {
                if (instr.opcode == opc(X86Opcode::INC) || instr.opcode == opc(X86Opcode::DEC))
                    && !self.is_cf_live_after(bb, i)
                {
                    if let Some(repl) = self.try_inc_dec_to_add_sub(instr) {
                        new_instrs.push(repl);
                        self.stats.inc_dec_to_add_sub += 1;
                        self.stats.total_fixups += 1;
                        fixed = true;
                    }
                }
            }

            // Rule 7: Size minimization (catch-all)
            if !fixed {
                if let Some(repl) = self.minimize_instruction_size(instr) {
                    let old_size = self.estimate_instruction_size(instr) as isize;
                    let new_size = self.estimate_instruction_size(&repl) as isize;
                    if repl.opcode == 0 {
                        // NOP — remove
                        self.stats.total_fixups += 1;
                        self.stats.size_saved_bytes += old_size as usize;
                    } else {
                        new_instrs.push(repl);
                        self.stats.total_fixups += 1;
                        if new_size < old_size {
                            self.stats.size_saved_bytes += (old_size - new_size) as usize;
                        }
                    }
                    fixed = true;
                }
            }

            if !fixed {
                new_instrs.push(instr.clone());
            }
            i += 1;
        }

        bb.instructions = new_instrs;
    }

    /// Apply all fixup rules to an entire machine function.
    pub fn fixup_function(&mut self, mf: &mut MachineFunction) {
        self.stats = FixupStats::default();
        for block in &mut mf.blocks {
            self.fixup_block(block);
        }
    }

    /// Apply only size-reducing fixups (safe for -Os).
    pub fn fixup_for_size(&mut self, mf: &mut MachineFunction) {
        let saved_inc_dec = self.enable_inc_dec_to_add_sub;
        let saved_add_sub_lea = self.enable_add_sub_to_lea;
        self.enable_inc_dec_to_add_sub = false; // ADD is larger than INC
        self.enable_add_sub_to_lea = false; // Not for -Os
        self.fixup_function(mf);
        self.enable_inc_dec_to_add_sub = saved_inc_dec;
        self.enable_add_sub_to_lea = saved_add_sub_lea;
    }

    /// Apply only speed-improving fixups (safe for -O2/-O3).
    pub fn fixup_for_speed(&mut self, mf: &mut MachineFunction) {
        self.fixup_function(mf);
    }
}

// ============================================================================
// REX-Related Analysis Utilities
// ============================================================================

impl X86InstrFixup {
    /// Count the number of REX prefixes that would be needed for an
    /// instruction in its current form.
    pub fn count_rex_prefixes(&self, instr: &MachineInstr) -> usize {
        let rex = self.compute_rex_requirements(instr);
        if rex == 0 {
            0
        } else {
            1
        }
    }

    /// Count how many bytes are saved by using 32-bit register form
    /// instead of 64-bit register form.
    pub fn count_rex_savings(&self, instr: &MachineInstr) -> usize {
        if !self.is_64_bit {
            return 0;
        }
        // If all operands are 32-bit or lower, and REX.W is present, we save 1 byte
        let mut has_64bit_operand = false;
        for op in &instr.operands {
            if let Some(reg) = get_phys_reg(op) {
                if is_gpr64(reg) {
                    has_64bit_operand = true;
                }
            }
        }
        if has_64bit_operand {
            0
        } else {
            1
        }
    }

    /// Check if an instruction can use the shorter 8-bit immediate form.
    pub fn can_use_imm8(&self, instr: &MachineInstr) -> bool {
        if instr.operands.len() < 2 {
            return false;
        }
        if let Some(imm) = get_imm(&instr.operands[1]) {
            return fits_i8(imm);
        }
        false
    }

    /// Count how many bytes are saved by using imm8 instead of imm32.
    pub fn imm8_savings(&self, instr: &MachineInstr) -> usize {
        if self.can_use_imm8(instr) {
            3 // imm32 (4 bytes) - imm8 (1 byte) = 3 bytes saved
        } else {
            0
        }
    }
}

// ============================================================================
// Cross-Block Analysis
// ============================================================================

impl X86InstrFixup {
    /// Perform cross-block analysis for more aggressive fixups.
    ///
    /// Some fixups require knowing if a register's value is live across
    /// block boundaries. This function performs a simple iterative
    /// dataflow analysis to determine liveness.
    pub fn compute_global_liveness(&self, mf: &MachineFunction) -> HashMap<String, HashSet<u32>> {
        // Live-out sets per block
        let mut live_out: HashMap<String, HashSet<u32>> = HashMap::new();

        // Initialize: empty live-out sets
        for block in &mf.blocks {
            live_out.insert(block.name.clone(), HashSet::new());
        }

        // Simple iterative backward dataflow
        let mut changed = true;
        let mut iter = 0;

        while changed && iter < 10 {
            changed = false;
            iter += 1;

            for block in &mf.blocks {
                // Start with union of successors' live-in sets
                let mut new_live_out: HashSet<u32> = HashSet::new();
                for &succ_id in &block.successors {
                    if let Some(succ_block) = mf.blocks.iter().find(|b| b.id == succ_id) {
                        if let Some(succ_live) = live_out.get(&succ_block.name) {
                            new_live_out.extend(succ_live);
                        }
                    }
                }

                // Update if changed
                let old = live_out.get(&block.name).cloned().unwrap_or_default();
                if new_live_out != old {
                    live_out.insert(block.name.clone(), new_live_out);
                    changed = true;
                }
            }
        }

        live_out
    }

    /// Propagate liveness information for more precise CF dead analysis.
    pub fn is_cf_live_out_of_block(&self, mf: &MachineFunction, block_name: &str) -> bool {
        let liveness = self.compute_global_liveness(mf);
        if let Some(live) = liveness.get(block_name) {
            live.contains(&FLAGS)
        } else {
            false
        }
    }
}

// ============================================================================
// Specialized Fixup Passes
// ============================================================================

impl X86InstrFixup {
    /// Fixup for AND→TEST conversion when result is dead.
    ///
    /// Pattern: AND r, r (result dead, only flags used) → TEST r, r
    ///
    /// AND r,r and TEST r,r produce the same flags (ZF, SF, PF based on r;
    /// CF=0, OF=0 for both). TEST is preferred because:
    /// 1. It's the canonical "check register" instruction
    /// 2. The assembler may have shorter encodings
    /// 3. It communicates intent better to the hardware
    pub fn convert_dead_and_to_test(&mut self, bb: &mut MachineBasicBlock) {
        let n = bb.instructions.len();
        let mut new_instrs: Vec<MachineInstr> = Vec::new();

        for i in 0..n {
            let instr = &bb.instructions[i];

            if instr.opcode == opc(X86Opcode::AND)
                && instr.operands.len() >= 2
                && same_reg(&instr.operands[0], &instr.operands[1])
            {
                // Check if the result is dead (overwritten before use)
                let dst_reg = get_any_reg(&instr.operands[0]);
                let mut result_dead = false;

                if let Some(dst) = dst_reg {
                    let mut overwritten_before_use = false;
                    let limit = (i + 1 + X86_FIXUP_MAX_SCAN).min(n);
                    for j in (i + 1)..limit {
                        let next = &bb.instructions[j];
                        if next.operands.iter().any(|op| get_any_reg(op) == Some(dst)) {
                            break; // Used — result is live
                        }
                        if next.def == Some(dst)
                            || next.operands.first().and_then(|o| get_any_reg(o)) == Some(dst)
                        {
                            overwritten_before_use = true;
                            break;
                        }
                    }
                    if overwritten_before_use || i == n - 1 {
                        result_dead = true;
                    }
                }

                if result_dead {
                    // Convert to TEST
                    let mut test = MachineInstr::new(opc(X86Opcode::TEST));
                    test.operands.push(instr.operands[0].clone());
                    test.operands.push(instr.operands[1].clone());
                    new_instrs.push(test);
                    self.stats.cmp_to_test += 1; // Reuse counter
                    self.stats.total_fixups += 1;
                    continue;
                }
            }

            new_instrs.push(instr.clone());
        }

        bb.instructions = new_instrs;
    }

    /// Fixup OR→TEST conversion when result is dead.
    /// Same logic as AND→TEST above.
    pub fn convert_dead_or_to_test(&mut self, bb: &mut MachineBasicBlock) {
        let n = bb.instructions.len();
        let mut new_instrs: Vec<MachineInstr> = Vec::new();

        for i in 0..n {
            let instr = &bb.instructions[i];

            if instr.opcode == opc(X86Opcode::OR)
                && instr.operands.len() >= 2
                && same_reg(&instr.operands[0], &instr.operands[1])
            {
                let dst_reg = get_any_reg(&instr.operands[0]);
                let mut result_dead = false;

                if let Some(dst) = dst_reg {
                    let limit = (i + 1 + X86_FIXUP_MAX_SCAN).min(n);
                    for j in (i + 1)..limit {
                        let next = &bb.instructions[j];
                        if next.operands.iter().any(|op| get_any_reg(op) == Some(dst)) {
                            break;
                        }
                        if next.def == Some(dst)
                            || next.operands.first().and_then(|o| get_any_reg(o)) == Some(dst)
                        {
                            result_dead = true;
                            break;
                        }
                    }
                    if i == n - 1 {
                        result_dead = true;
                    }
                }

                if result_dead {
                    let mut test = MachineInstr::new(opc(X86Opcode::TEST));
                    test.operands.push(instr.operands[0].clone());
                    test.operands.push(instr.operands[1].clone());
                    new_instrs.push(test);
                    self.stats.cmp_to_test += 1;
                    self.stats.total_fixups += 1;
                    continue;
                }
            }

            new_instrs.push(instr.clone());
        }

        bb.instructions = new_instrs;
    }

    /// Run specialized fixup passes.
    pub fn run_specialized_fixups(&mut self, mf: &mut MachineFunction) {
        for block in &mut mf.blocks {
            self.convert_dead_and_to_test(block);
            self.convert_dead_or_to_test(block);
        }
    }
}

// ============================================================================
// Reporting
// ============================================================================

impl X86InstrFixup {
    pub fn fixup_report(&self) -> String {
        format!(
            "=== X86 Instruction Fixup Report ===\n  Target: {}\n  64-bit: {}\n  Opt size: {}\n  CMP→TEST: {}\n  XOR32: {}\n  GPR32 ext: {}\n  LEA→MOV: {}\n  ADD/SUB→LEA: {}\n  INC/DEC: {}\n  REX removed: {}\n  Seg override removed: {}\n  Prefix removed: {}\n  Size saved: {}B\n\n  {}\n",
            self.subtarget.cpu, self.is_64_bit, self.opt_for_size,
            self.enable_cmp_to_test, self.enable_xor32_zeroing,
            self.enable_32bit_implicit_zero, self.enable_lea_to_mov,
            self.enable_add_sub_to_lea, self.enable_inc_dec_to_add_sub,
            self.stats.rex_removed, self.stats.segment_override_removed,
            self.stats.prefix_removed, self.stats.size_saved_bytes,
            self.stats.summary()
        )
    }

    pub fn reset_stats(&mut self) {
        self.stats = FixupStats::default();
    }
}

// ============================================================================
// Tests
// ============================================================================

#[cfg(test)]
mod tests {
    use super::*;
    use crate::codegen::{MachineBasicBlock, MachineFunction};

    fn make_sub() -> X86Subtarget {
        X86Subtarget::new("x86_64-unknown-linux-gnu", "skylake", "+sse2,+avx2")
    }
    fn make_bb(name: &str) -> MachineBasicBlock {
        MachineBasicBlock {
            name: name.to_string(),
            instructions: Vec::new(),
            successors: Vec::new(),
        }
    }

    #[test]
    fn test_creation() {
        let sub = make_sub();
        let fix = X86InstrFixup::new(&sub);
        assert!(fix.is_64_bit);
        assert!(fix.enable_cmp_to_test);
        assert!(fix.enable_xor32_zeroing);
        assert_eq!(fix.stats.total_fixups, 0);
    }

    #[test]
    fn test_cmp_to_test_zero() {
        let sub = make_sub();
        let fix = X86InstrFixup::new(&sub);
        let mut instr = MachineInstr::new(opc(X86Opcode::CMP));
        instr.operands.push(MachineOperand::PhysReg(RAX));
        instr.operands.push(MachineOperand::Imm(0));
        let result = fix.try_cmp_to_test(&instr);
        assert!(result.is_some());
        let test = result.unwrap();
        assert_eq!(test.opcode, opc(X86Opcode::TEST));
        // Should use 32-bit TEST (EAX,EAX) for RAX
        assert!(matches!(&test.operands[0], MachineOperand::PhysReg(r) if *r == EAX));
    }

    #[test]
    fn test_cmp_nonzero_not_converted() {
        let sub = make_sub();
        let fix = X86InstrFixup::new(&sub);
        let mut instr = MachineInstr::new(opc(X86Opcode::CMP));
        instr.operands.push(MachineOperand::PhysReg(RAX));
        instr.operands.push(MachineOperand::Imm(5));
        let result = fix.try_cmp_to_test(&instr);
        assert!(result.is_none());
    }

    #[test]
    fn test_xor32_zeroing() {
        let sub = make_sub();
        let fix = X86InstrFixup::new(&sub);
        let mut instr = MachineInstr::new(opc(X86Opcode::XOR));
        instr.operands.push(MachineOperand::PhysReg(RAX));
        instr.operands.push(MachineOperand::PhysReg(RAX));
        let result = fix.try_xor32_zeroing(&instr);
        assert!(result.is_some());
        let xor32 = result.unwrap();
        assert!(matches!(&xor32.operands[0], MachineOperand::PhysReg(r) if *r == EAX));
    }

    #[test]
    fn test_xor32_different_regs_not_converted() {
        let sub = make_sub();
        let fix = X86InstrFixup::new(&sub);
        let mut instr = MachineInstr::new(opc(X86Opcode::XOR));
        instr.operands.push(MachineOperand::PhysReg(RAX));
        instr.operands.push(MachineOperand::PhysReg(RBX));
        let result = fix.try_xor32_zeroing(&instr);
        assert!(result.is_none());
    }

    #[test]
    fn test_lea_to_mov_no_arithmetic() {
        let sub = make_sub();
        let fix = X86InstrFixup::new(&sub);
        // LEA RAX, [RBX+0] → MOV RAX, RBX
        let mut instr = MachineInstr::new(opc(X86Opcode::LEA));
        instr.operands.push(MachineOperand::PhysReg(RAX));
        instr.operands.push(MachineOperand::PhysReg(RBX));
        let result = fix.try_lea_to_mov(&instr);
        assert!(result.is_some());
        let mov = result.unwrap();
        assert_eq!(mov.opcode, opc(X86Opcode::MOV));
    }

    #[test]
    fn test_lea_same_reg_to_nop() {
        let sub = make_sub();
        let fix = X86InstrFixup::new(&sub);
        // LEA RAX, [RAX+0] → NOP
        let mut instr = MachineInstr::new(opc(X86Opcode::LEA));
        instr.operands.push(MachineOperand::PhysReg(RAX));
        instr.operands.push(MachineOperand::PhysReg(RAX));
        let result = fix.try_lea_to_mov(&instr);
        assert!(result.is_some());
        let nop = result.unwrap();
        assert_eq!(nop.opcode, 0); // NOP sentinel
    }

    #[test]
    fn test_inc_to_add() {
        let sub = make_sub();
        let fix = X86InstrFixup::new(&sub);
        let mut instr = MachineInstr::new(opc(X86Opcode::INC));
        instr.operands.push(MachineOperand::PhysReg(RAX));
        let result = fix.try_inc_dec_to_add_sub(&instr);
        assert!(result.is_some());
        let add = result.unwrap();
        assert_eq!(add.opcode, opc(X86Opcode::ADD));
        // Should use 32-bit ADD for RAX
        assert!(matches!(&add.operands[0], MachineOperand::PhysReg(r) if *r == EAX));
        assert!(matches!(&add.operands[1], MachineOperand::Imm(1)));
    }

    #[test]
    fn test_dec_to_sub() {
        let sub = make_sub();
        let fix = X86InstrFixup::new(&sub);
        let mut instr = MachineInstr::new(opc(X86Opcode::DEC));
        instr.operands.push(MachineOperand::PhysReg(RBX));
        let result = fix.try_inc_dec_to_add_sub(&instr);
        assert!(result.is_some());
        let sub_instr = result.unwrap();
        assert_eq!(sub_instr.opcode, opc(X86Opcode::SUB));
    }

    #[test]
    fn test_is_cf_live_no_consumer() {
        let sub = make_sub();
        let fix = X86InstrFixup::new(&sub);
        let mut bb = make_bb("test");
        // ADD rax, 1 — no CF consumer
        let mut add = MachineInstr::new(opc(X86Opcode::ADD));
        add.operands.push(MachineOperand::PhysReg(RAX));
        add.operands.push(MachineOperand::Imm(1));
        bb.instructions.push(add);
        assert!(!fix.is_cf_live_after(&bb, 0));
    }

    #[test]
    fn test_is_cf_live_with_adc() {
        let sub = make_sub();
        let fix = X86InstrFixup::new(&sub);
        let mut bb = make_bb("test");
        // ADD rax, 1
        let mut add = MachineInstr::new(opc(X86Opcode::ADD));
        add.operands.push(MachineOperand::PhysReg(RAX));
        add.operands.push(MachineOperand::Imm(1));
        bb.instructions.push(add);
        // ADC rbx, 0 — reads CF from ADD
        let mut adc = MachineInstr::new(opc(X86Opcode::ADC));
        adc.operands.push(MachineOperand::PhysReg(RBX));
        adc.operands.push(MachineOperand::Imm(0));
        bb.instructions.push(adc);
        assert!(fix.is_cf_live_after(&bb, 0));
    }

    #[test]
    fn test_compute_rex_requirements() {
        let sub = make_sub();
        let fix = X86InstrFixup::new(&sub);
        // MOV RAX, RBX — needs REX.W + REX.B (RBX uses r/m field extension?)
        // In practice: MOV with 64-bit registers needs REX.W
        let mut instr = MachineInstr::new(opc(X86Opcode::MOV));
        instr.operands.push(MachineOperand::PhysReg(RAX));
        instr.operands.push(MachineOperand::PhysReg(RBX));
        let rex = fix.compute_rex_requirements(&instr);
        assert!(rex & 0x08 != 0); // REX.W needed for 64-bit registers
    }

    #[test]
    fn test_no_rex_for_32bit() {
        let sub = make_sub();
        let fix = X86InstrFixup::new(&sub);
        // MOV EAX, ECX — no REX needed (32-bit, registers 0-7)
        let mut instr = MachineInstr::new(opc(X86Opcode::MOV));
        instr.operands.push(MachineOperand::PhysReg(EAX));
        instr.operands.push(MachineOperand::PhysReg(ECX));
        let rex = fix.compute_rex_requirements(&instr);
        assert_eq!(rex, 0); // No REX needed
    }

    #[test]
    fn test_size_estimate_cmp_vs_test() {
        let sub = make_sub();
        let fix = X86InstrFixup::new(&sub);
        let mut cmp = MachineInstr::new(opc(X86Opcode::CMP));
        cmp.operands.push(MachineOperand::PhysReg(EAX));
        cmp.operands.push(MachineOperand::Imm(0));
        let cmp_size = fix.estimate_instruction_size(&cmp);

        let mut test_instr = MachineInstr::new(opc(X86Opcode::TEST));
        test_instr.operands.push(MachineOperand::PhysReg(EAX));
        test_instr.operands.push(MachineOperand::PhysReg(EAX));
        let test_size = fix.estimate_instruction_size(&test_instr);

        assert!(test_size < cmp_size); // TEST is smaller
    }

    #[test]
    fn test_size_estimate_xor_64_vs_32() {
        let sub = make_sub();
        let fix = X86InstrFixup::new(&sub);
        let mut xor64 = MachineInstr::new(opc(X86Opcode::XOR));
        xor64.operands.push(MachineOperand::PhysReg(RAX));
        xor64.operands.push(MachineOperand::PhysReg(RAX));
        let size64 = fix.estimate_instruction_size(&xor64);

        let mut xor32 = MachineInstr::new(opc(X86Opcode::XOR));
        xor32.operands.push(MachineOperand::PhysReg(EAX));
        xor32.operands.push(MachineOperand::PhysReg(EAX));
        let size32 = fix.estimate_instruction_size(&xor32);

        assert!(size32 < size64); // 32-bit XOR is smaller
    }

    #[test]
    fn test_add_sub_to_lea() {
        let sub = make_sub();
        let fix = X86InstrFixup::new(&sub);
        // ADD rbx, 8; MOV rax, rbx → LEA rax, [rbx+8]
        let mut add = MachineInstr::new(opc(X86Opcode::ADD));
        add.operands.push(MachineOperand::PhysReg(RBX));
        add.operands.push(MachineOperand::Imm(8));
        let mut mov = MachineInstr::new(opc(X86Opcode::MOV));
        mov.operands.push(MachineOperand::PhysReg(RAX));
        mov.operands.push(MachineOperand::PhysReg(RBX));
        let result = fix.try_add_sub_to_lea(&add, &mov);
        assert!(result.is_some());
        let lea = result.unwrap();
        assert_eq!(lea.opcode, opc(X86Opcode::LEA));
    }

    #[test]
    fn test_32bit_implicit_zero_mov_imm() {
        let sub = make_sub();
        let fix = X86InstrFixup::new(&sub);
        let mut mov = MachineInstr::new(opc(X86Opcode::MOV));
        mov.operands.push(MachineOperand::PhysReg(RAX));
        mov.operands.push(MachineOperand::Imm(0x42));
        let result = fix.try_32bit_implicit_zero(&mov);
        assert!(result.is_some());
        let mov32 = result.unwrap();
        assert!(matches!(&mov32.operands[0], MachineOperand::PhysReg(r) if *r == EAX));
    }

    #[test]
    fn test_stats_merge() {
        let mut s1 = FixupStats::default();
        s1.cmp_to_test = 5;
        s1.total_fixups = 5;
        let mut s2 = FixupStats::default();
        s2.xor32_zeroing = 3;
        s2.total_fixups = 3;
        s1.merge(&s2);
        assert_eq!(s1.cmp_to_test, 5);
        assert_eq!(s1.xor32_zeroing, 3);
        assert_eq!(s1.total_fixups, 8);
    }

    #[test]
    fn test_fixup_report() {
        let sub = make_sub();
        let fix = X86InstrFixup::new(&sub);
        let report = fix.fixup_report();
        assert!(report.contains("skylake"));
        assert!(report.contains("X86 Instruction Fixup"));
    }

    #[test]
    fn test_instr_defines_flags() {
        let sub = make_sub();
        let fix = X86InstrFixup::new(&sub);
        assert!(fix.instr_defines_flags(&MachineInstr::new(opc(X86Opcode::ADD))));
        assert!(fix.instr_defines_flags(&MachineInstr::new(opc(X86Opcode::CMP))));
        assert!(fix.instr_defines_flags(&MachineInstr::new(opc(X86Opcode::TEST))));
        assert!(!fix.instr_defines_flags(&MachineInstr::new(opc(X86Opcode::MOV))));
    }

    #[test]
    fn test_can_use_imm8() {
        let sub = make_sub();
        let fix = X86InstrFixup::new(&sub);
        let mut instr = MachineInstr::new(opc(X86Opcode::ADD));
        instr.operands.push(MachineOperand::PhysReg(EAX));
        instr.operands.push(MachineOperand::Imm(5));
        assert!(fix.can_use_imm8(&instr));
        let mut instr2 = MachineInstr::new(opc(X86Opcode::ADD));
        instr2.operands.push(MachineOperand::PhysReg(EAX));
        instr2.operands.push(MachineOperand::Imm(1000));
        assert!(!fix.can_use_imm8(&instr2));
    }

    #[test]
    fn test_fixup_block_cmp_to_test() {
        let sub = make_sub();
        let mut fix = X86InstrFixup::new(&sub);
        let mut bb = make_bb("test");
        // CMP rax, 0 — should be converted to TEST eax, eax
        let mut cmp = MachineInstr::new(opc(X86Opcode::CMP));
        cmp.operands.push(MachineOperand::PhysReg(RAX));
        cmp.operands.push(MachineOperand::Imm(0));
        bb.instructions.push(cmp);
        fix.fixup_block(&mut bb);
        assert_eq!(fix.stats.cmp_to_test, 1);
        assert!(bb.instructions[0].opcode == opc(X86Opcode::TEST));
    }

    #[test]
    fn test_fixup_block_lea_to_mov() {
        let sub = make_sub();
        let mut fix = X86InstrFixup::new(&sub);
        let mut bb = make_bb("test");
        // LEA rax, [rbx] — should be MOV rax, rbx
        let mut lea = MachineInstr::new(opc(X86Opcode::LEA));
        lea.operands.push(MachineOperand::PhysReg(RAX));
        lea.operands.push(MachineOperand::PhysReg(RBX));
        bb.instructions.push(lea);
        fix.fixup_block(&mut bb);
        assert_eq!(fix.stats.lea_to_mov, 1);
    }

    #[test]
    fn test_can_remove_rex() {
        let sub = make_sub();
        let fix = X86InstrFixup::new(&sub);
        // 32-bit instruction with no extended regs — REX can be removed
        let mut instr = MachineInstr::new(opc(X86Opcode::MOV));
        instr.operands.push(MachineOperand::PhysReg(EAX));
        instr.operands.push(MachineOperand::PhysReg(ECX));
        assert!(fix.can_remove_rex(&instr));
        // 64-bit instruction — REX needed
        let mut instr64 = MachineInstr::new(opc(X86Opcode::MOV));
        instr64.operands.push(MachineOperand::PhysReg(RAX));
        instr64.operands.push(MachineOperand::PhysReg(RCX));
        assert!(!fix.can_remove_rex(&instr64));
    }

    #[test]
    fn test_size_minimization_prefers_smaller() {
        let sub = make_sub();
        let fix = X86InstrFixup::new(&sub);
        // CMP r, 0 → TEST r, r
        let mut cmp = MachineInstr::new(opc(X86Opcode::CMP));
        cmp.operands.push(MachineOperand::PhysReg(EAX));
        cmp.operands.push(MachineOperand::Imm(0));
        let result = fix.minimize_instruction_size(&cmp);
        assert!(result.is_some());
        let optimized = result.unwrap();
        assert_eq!(optimized.opcode, opc(X86Opcode::TEST));
    }
}