On Sat, May 30, 2015 at 3:08 PM, aurelio remonda <aurelioremo...@gmail.com> wrote: >>> if (op < 4) { >>> /* Saturating add/subtract. */ >>> + if (!(arm_dc_feature(s, ARM_FEATURE_THUMB_DSP))){ >>> + /* qsub, qadd, qdadd, qdsub are DSP instructions. */ >>> + goto illegal_op; >>> + } >>> tmp = load_reg(s, rn); >>> tmp2 = load_reg(s, rm); >>> if (op & 1) >>> @@ -9559,6 +9598,12 @@ static int disas_thumb2_insn(CPUARMState *env, >>> DisasContext *s, uint16_t insn_hw >>> gen_revsh(tmp); >>> break; >>> case 0x10: /* sel */ >>> + if (!(arm_dc_feature(s, ARM_FEATURE_THUMB_DSP))){ >>> + /* sel is a DSP instruction. */ >>> + /* need to free this so there's no TCG temporary leak >>> */ >>> + tcg_temp_free_i32(tmp); >>> + goto illegal_op; >>> + } >>> tmp2 = load_reg(s, rm); >>> tmp3 = tcg_temp_new_i32(); >>> tcg_gen_ld_i32(tmp3, cpu_env, offsetof(CPUARMState, >>> GE)); >>> @@ -9624,6 +9669,15 @@ static int disas_thumb2_insn(CPUARMState *env, >>> DisasContext *s, uint16_t insn_hw >>> } >>> break; >>> case 1: /* 16 x 16 -> 32 */ >>> + if (!(arm_dc_feature(s, ARM_FEATURE_THUMB_DSP))){ >>> + /* smlabb, smlabt, smlatb, smlatt, smulbb, smulbt, >>> smultt >>> + * and smultb are DSP instructions >>> + */ >>> + /* need to free this so there's no TCG temporary leak >>> */ >> >> Comment un-needed. > > Why do you think it's not needed? Any comments are helpful as long as > they're correct. Thanks >
Not always. Comments on code are a maintainance burden. It's preferrable to have the code self document as much as possible so that changes only need to be applied once - to the code itself. The only reason to ever call tcg_temp_free is to avoid a leak so its self explanatory IMO. Regards, Peter