Re: [PATCH v2 07/22] target/loongarch: Add fixed point arithmetic instruction translation

[Song Gao]

Hi, Philippe,

On 07/22/2021 01:49 AM, Philippe Mathieu-Daudé wrote:
> On 7/21/21 7:38 PM, Philippe Mathieu-Daudé wrote:
>> On 7/21/21 11:53 AM, Song Gao wrote:
>>> This patch implement fixed point arithemtic instruction translation.
> 
> Typo arithmetic.
> 
>>>
>>> This includes:
>>> - ADD.{W/D}, SUB.{W/D}
>>> - ADDI.{W/D}, ADDU16ID
>>> - ALSL.{W[U]/D}
>>> - LU12I.W, LU32I.D LU52I.D
>>> - SLT[U], SLT[U]I
>>> - PCADDI, PCADDU12I, PCADDU18I, PCALAU12I
>>> - AND, OR, NOR, XOR, ANDN, ORN
>>> - MUL.{W/D}, MULH.{W[U]/D[U]}
>>> - MULW.D.W[U]
>>> - DIV.{W[U]/D[U]}, MOD.{W[U]/D[U]}
>>> - ANDI, ORI, XORI
>>>
>>> Signed-off-by: Song Gao <gaosong@loongson.cn>
>>> ---
>>>  target/loongarch/insns.decode |   89 ++++
>>>  target/loongarch/trans.inc.c  | 1090 
>>> +++++++++++++++++++++++++++++++++++++++++
>>>  target/loongarch/translate.c  |   12 +
>>>  target/loongarch/translate.h  |    1 +
>>>  4 files changed, 1192 insertions(+)
>>>  create mode 100644 target/loongarch/insns.decode
>>>  create mode 100644 target/loongarch/trans.inc.c
>>
>> Please don't include all .inc.c in one big translate.c...
>>
>>> diff --git a/target/loongarch/translate.c b/target/loongarch/translate.c
>>> index 531f7e1..b60bdc2 100644
>>> --- a/target/loongarch/translate.c
>>> +++ b/target/loongarch/translate.c
>>> @@ -57,6 +57,15 @@ void gen_load_gpr(TCGv t, int reg)
>>>      }
>>>  }
>>>  
>>> +TCGv get_gpr(int regno)
>>> +{
>>> +    if (regno == 0) {
>>> +        return tcg_constant_tl(0);
>>> +    } else {
>>> +        return cpu_gpr[regno];
>>> +    }
>>> +}
>>> +
>>>  static inline void gen_save_pc(target_ulong pc)
>>
>> ... expose this one ...
>>
>>>  {
>>>      tcg_gen_movi_tl(cpu_PC, pc);
>>> @@ -287,6 +296,9 @@ static bool 
>>> loongarch_tr_breakpoint_check(DisasContextBase *dcbase,
>>>      return true;
>>>  }
>>>  
>>> +#include "decode-insns.c.inc"
>>
>> ... and move this include to "trans.c".
> 
> Since you have the luck to add a new architecture, you could
> start cleanly from scratch and add group of instructions, so
> this patch would add "trans_arithmetic.c", etc.. in the series.
> 

Got it,  The file trans.inc.c seems too big ...

Thansk,
Song Gao