Re: [Qemu-devel] [PATCH v4 06/33] tcg-aarch64: Merge enum aarch64_srr_opc with AArch64Insn

[Claudio Fontana]

On 16.09.2013 17:06, Richard Henderson wrote:
> On 09/16/2013 12:56 AM, Claudio Fontana wrote:
>>>>      case INDEX_op_shl_i64:
>>>>      case INDEX_op_shl_i32:
>>>> -        if (c2) {    /* LSL / UBFM Wd, Wn, (32 - m) */
>>>> +        if (c2) {
>>>>              tcg_out_shl(s, ext, a0, a1, a2);
>>>> -        } else {                /* LSL / LSLV */
>>>> -            tcg_out_shiftrot_reg(s, SRR_SHL, ext, a0, a1, a2);
>>>> +        } else {
>>>> +            tcg_out_shiftrot_reg(s, INSN_LSLV, ext, a0, a1, a2);
>>>>          }
>>>>          break;
>> Any reason you strip all comments out?
>> They are supposed to hint the reader about how the tcg operation is 
>> implemented.
>>
> 
> Well, LSLV is now there in the code in the form of INSN_LSLV.  The comment 
> does
> nothing additional to hint at what's going on; indeed, "LSL / LSLV" is more
> confusing than not.
> 
> As for tcg_out_shl, comments about its implementation are with that function.
> 
> 

Fair enough.

C.