Re: Re: [Patch] New: CR16 port

[Pompapathi V Gadad <pompapathi@gmail.com>]

Hello Rask,

First of all, I sincerely thank you for taking time to review the patch.
Second, I am extremely sorry for delayed response.

Rask Ingemann Lambertsen wrote:
> Don't include the ChangeLog entry in the patch, post it separately. The
> only exception is if you've already committed the patch. Then it is ok to
> include the ChangeLog entry as part of the patch.
I will remember this now on.
>
>> +;; Negation Instructions
>> +; move 0,r sub x,r. More efficient than xor -1,x add 1,x
> You can probably leave this out as the middle end will generate "op0 = 0
> - op1" if there's no negm2 pattern.
The above pattern in md file is redundant then. I will remove this.
>
>> +(define_insn "mov<mode>_regs"
>> + [(set (match_operand:ALLMT 0 "register_operand" "=r, r")
>> + (match_operand:ALLMT 1 "nonmemory_operand" "r, <iF>"))]
>> + ""
>> + "mov<tIsa>\t%1, %0"
>> + [(set_attr "length" "2,<lImmArith>")]
>> +)
>> +
>> +; Memory => Reg
>> +(define_insn "mov<mode>_load"
>> + [(set (match_operand:ALLMT 0 "register_operand" "=r")
>> + (match_operand:ALLMT 1 "memory_operand" "m"))]
>> + ""
>> + "load<tIsa>\t%1, %0"
>> + [(set_attr "length" "<lImmArith>")]
>> +)
>> +
>> +(define_insn "mov<mode>_store_reg"
>> + [(set (match_operand:ALLMT 0 "store_operand" "=m")
>> + (match_operand:ALLMT 1 "register_operand" "r"))]
>> + ""
>> + "stor<tIsa>\t%1, %0"
>> + [(set_attr "length" "<lImmArith>")]
>> +)
>> +
>> +(define_insn "mov<mode>_store_imm4"
>> + [(set (match_operand:SHORT 0 "store_operand" "=m")
>> + (match_operand:SHORT 1 "u4bits_operand" "<LL>"))]
>> + ""
>> + "stor<tIsaShort>\t%1, %0"
>> + [(set_attr "length" "<lImmShort>")]
>> +)
>
> These would normally be one pattern with five alternatives. A reg->reg
> copy can turn into a reg->mem copy during reload, for example.
>
Well, I started with suggestion you had: Having 5 alternatives in a 
single pattern.
It turns out that some of the pattern generated pass through predicate 
test but won't find
an appropriate alternative. For example, in CR16 ISA, immediate memory 
store instruction
does not have 32-bit store operation variant. Therefore, I had to break 
up the single pattern into
multiple patterns. The store immediate pattern have "SHORT" (stands for 
QI and HI) macro
expansion while others have ALLMT (stands for QI, HI, SI, SF)

Thanks,
Pompa