On 09/26/2013 05:48 PM, Alexander Graf wrote:
> +static int get_reg(uint32_t inst)
> +{
> + return get_bits(inst, 0, 5);
> +}
Surely get_rt or some such related to the actual field name, not something
so generic as "reg" which applies equally to any of several fields.
> +static void ldst_do_vec(DisasContext *s, int dest, TCGv_i64 tcg_addr_real,
> + int size, bool is_store)
> +{
> + TCGv_i64 tcg_addr = tcg_temp_new_i64();
> + int freg_offs = offsetof(CPUARMState, vfp.regs[dest * 2]);
> +
> + /* we don't want to modify the caller's tcg_addr */
> + tcg_gen_mov_i64(tcg_addr, tcg_addr_real);
Surely merge with ldst_do_vec_int, for the single case of 16 byte size?
> +
> + if (!is_store) {
> + /* normal ldst clears non-loaded bits */
> + clear_fpreg(dest);
> + }
Surely merge with ldst_do_vec_int, to avoid redundant stores into the
vfp register set?
> +static void handle_stp(DisasContext *s, uint32_t insn)
> +{
This handles both ldp and stp though, right?
> + ldst_do(s, rt, tcg_addr, size, is_store, is_signed, is_vector);
> + tcg_gen_addi_i64(tcg_addr, tcg_addr, 1 << size);
> + ldst_do(s, rt2, tcg_addr, size, is_store, is_signed, is_vector);
> + tcg_gen_subi_i64(tcg_addr, tcg_addr, 1 << size);
Why subtract instead of using another temporary?
> + /* Typical major opcode encoding */
> switch ((insn >> 24) & 0x1f) {
> + case 0x08:
> + case 0x09:
> + if (get_bits(insn, 29, 1)) {
> + handle_stp(s, insn);
> + } else {
> + unallocated_encoding(s);
> + }
> + break;
> + case 0x0c:
> + if (get_bits(insn, 29, 1)) {
> + handle_stp(s, insn);
> + } else {
> + unallocated_encoding(s);
> + }
> + break;
> + case 0x0d:
> + if (get_bits(insn, 29, 1)) {
> + handle_stp(s, insn);
> + } else {
> + unallocated_encoding(s);
> + }
> + break;
This leads me to believe that this isn't the best arrangement of the decoder.
I would expect the structure to look more like the arrangement in chapter C3,
especially the major encoding groups in table C3-1.
r~
