Re: [Qemu-devel] [PATCH] Target-arm: Add the Cortex-M4 CPU

[Peter Crosthwaite]

On Sat, May 30, 2015 at 3:08 PM, aurelio remonda
<address@hidden> 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