ARM/Thumb question

[rva3]

Hi, I'm working on ARM/Thumb support for this project. The ARMv7 and Thumb are a bit tricky because they have 3 encodings for the Thumb mode and 2 for ARM. For example, the MOV r0, #0xff can be treated by GAS as:

• MOV r0, #0xff (T1 encoding, ARMv4T+)
• MOV.W r0, #0xff (T2 encoding, ARMv6T2+, if the register is higher than r7 or immediate value is more than u8)
• MOVW r0, #0xff (T3 encoding, ARMv6T2+, if the register is higher than r7 or immediate value is more than 12 bits)

Should the pseudo-instructions be exposed to the end user, i.e.

    /// Generate MOV instruction for immediate value
    ///
    /// If `rd` is R8-R15 or the immediate value is bigger than 12 bits then MOVW from T3 is used
    ///
    /// If the immediate value fits into 12 bits or `rd` is R8-R15, T2 encoding is used
    ///
    /// If the immediate value fits into u8 then T1 encoding is used
    pub fn mov_imm(&mut self, rd: Register, imm16: u16) -> &mut Self {
        self.track_written_register(rd);
        let instr = if rd > reg::R7 {
            if imm16 > 0x7ff {
                encode_thumb_mov_imm_t3(rd, imm16)
            } else {
                encode_thumb_mov_imm_t2(false, rd, imm16 & 0x7ff)
            }
        } else {
            if imm16 > 0x7ff {
                encode_thumb_mov_imm_t3(rd, imm16)
            } else if imm16 > u8::MAX as u16 {
                encode_thumb_mov_imm_t2(false, rd, imm16 & 0x7ff)
            } else {
                encode_thumb_mov_imm_t1(rd, (imm16 & 0xff) as u8)
            }
        };
        self.push(instr);
        self
    }

or the low-level instructions (mov, mov_wide, movw; or mov_t1, movw_t2, movw_t3) instead?

[petitstrawberry]

Hi @rva3, thanks for working on the ARM/Thumb support!

To answer your question: I think exposing both would be the best approach.

If we look at how inline assembly and GNU Assembler (GAS) handle this, developers usually expect to have both options:

1. High-level (Auto-selection): Having a pseudo-instruction like your mov_imm is great. It lets the assembler automatically pick the most optimal encoding, which makes it very easy to use for general cases.
2. Low-level (Explicit control): However, advanced JIT developers often need strict control over instruction size and alignment. Exposing the exact instructions is crucial for this.

Regarding the naming of those low-level instructions: aligning them with standard GAS syntax (e.g., mov_n, mov_w, movw) would be much more intuitive for end users than exposing the manual's internal encoding names (mov_t1, mov_t2, mov_t3).

So, providing mov_imm for convenience, while also exposing mov_n / mov_w / movw for fine-grained control, would offer the best of both worlds. What do you think?

[rva3]

providing mov_imm for convenience, while also exposing mov_n / mov_w / movw for fine-grained control, would offer the best of both worlds.

It's indeed a good idea, but it gets tricky with something like MOVS. MOVS has T1 (imm8) and T2 (MOVS.W, imm12 via ThumbExpandImm_C; it actually can pack u32, but only very specific patterns). This way we need either global Result in the struct (doesn't fit because InstructionBuilder wants type without Result), or Result for every method in the builder, i.e.

    // XXX: this should be done with mov_n, mov_w, movw instead of enum
    pub fn movs_imm_with_encoding(
        &mut self,
        rd: Register,
        imm16: u16, // XXX: type
        encoding: ThumbEncoding,
    ) -> Result<&mut Self> {
        let instr = match encoding {
            // XXX: inside IT block: MOV
            //
            // Currently we don't track if there's IT block, so MOVS should be fine...
            ThumbEncoding::T1 => {
                if rd.is_low() && imm16 <= 0xff {
                    Ok(encode_thumb_mov_imm_t1(rd, (imm16 & 0xff) as u8))
                } else {
                    if !rd.is_low() {
                        Err(Error::HighRegister(rd))
                    } else {
                        Err(Error::ImmediateOutOfRange {
                            value: imm16,
                            max: 0xff,
                        })
                    }
                }
            }
            ThumbEncoding::T2 => {
                if let Some(flattened) = thumb_try_flatten_imm(imm16 as u32) {
                    Ok(encode_thumb_mov_imm_t2(true, rd, flattened))
                } else {
                    // XXX: bad error type
                    Err(Error::ImmediateOutOfRange {
                        value: imm16,
                        max: 0x7ff,
                    })
                }
            }
            ThumbEncoding::T3 => Err(Error::InvalidEncoding(encoding)),
            ThumbEncoding::T4 => Err(Error::InvalidEncoding(encoding)),
        }?;

        self.track_written_register(rd);
        self.push(instr);
        Ok(self)
    }

    /// Generate MOVS instruction for immediate value
    ///
    /// If the immediate value fits into 12 bits or `rd` is R8-R15, T2 encoding is used
    ///
    /// If the immediate value fits into u8 then T1 encoding is used
    pub fn movs_imm(&mut self, rd: Register, imm12: u16) -> Result<&mut Self> {
        if imm12 > 0x7ff {
            Err(Error::ImmediateOutOfRange {
                value: imm12,
                max: 0x7ff,
            })
        } else {
            if rd.is_low() {
                if imm12 > u8::MAX as u16 {
                    // XXX: this is actually wrong and should be used with ThumbExpandImm_C reverse operation instead
                    self.movs_imm_with_encoding(rd, imm12, ThumbEncoding::T2) // gives Result
                } else {
                    self.movs_imm_with_encoding(rd, imm12, ThumbEncoding::T1) // gives Result
                }
            } else {
                self.movs_imm_with_encoding(rd, imm12, ThumbEncoding::T2) // gives Result
            }
        }
    }

Since thumb_try_flatten_imm (I called it like this because of reverse operation for ThumbExpandImm_C) can pack certain u32 values, the input type (imm12: u16) lies (it should be u32). The workaround for this would look like MOVW rd, #lower_value_of_imm; MOVT rd, #upper_value_of_imm; MOVS r0, r0, but it still requires Result for low-level T1/T2 calls.

I don't think panic is a reliable way to handle this (catching panics pulls core::fmt which is pretty heavy for no_std). Do you have some other idea about design? I'm not sure if the macro would play fine with Result for each method.

@petitstrawberry

[petitstrawberry]

Hi @rva3, thanks for the details. You are absolutely right that returning Result from every method breaks the fluent builder and macros, and using panic! is not an option for no_std.

I will implement an error handling mechanism to solve these issues on the core side, so it would be best to align the ARM/Thumb implementation with that.

Specifically, I plan to introduce an Error Accumulator pattern. The builder will store the error internally, and we will only return a Result at the very end when generating the instruction collection (via the instructions() method).

Here is what the core trait changes will look like:

pub trait InstructionBuilder<I> {
    type Error;

    // ... existing methods ...
    
    // A standard way to report an error without breaking the fluent chain
    fn set_error(&mut self, error: Self::Error);

    // [Breaking Change] The Result is returned only when extracting the final collection
    fn instructions(&self) -> Result<InstructionCollection<I>, Self::Error>;
}

With this design, if an invalid encoding is requested (like an out-of-range immediate for MOVS), you can simply call self.set_error(Error::...) and return &mut Self. This completely preserves the fluent API and the macros.

Since changing the core InstructionBuilder trait is a breaking change and requires updating existing architectures (such as RISC-V), I will implement this core mechanism.

I'll let you know once I push the updates to the main branch. Then, you can base your Thumb implementation on this new API!

[petitstrawberry]

@rva3, I have implemented the core error handling mechanism and pushed the changes to the develop branch!

Here are the key updates:

1. Error Accumulator Pattern: The InstructionBuilder trait now has a set_error(&mut self, error: Self::Error) method. If you encounter an invalid encoding (like an out-of-range immediate), just call this method and return &mut Self.
2. Result at the End: The instructions() method now returns a Result<InstructionCollection<I>, Self::Error>, returning the accumulated error only at the very end of the building process. This preserves our fluent API and macros.

Please pull the latest develop branch and base your ARM/Thumb implementation on this new API.