On Saturday, December 28, 2019, Michael Rolnik wrote: > Hi Aleksandar. > > This seems less logical to me. > Then next thing will be to partition disassember part right? > > > Please respond inline in future, since inline redponding is standard for this mailing list. What is not logical to you? I am trying to teach you how to create patches that are logical units easy to understand and review. In my view, yout series doesn't satisfy that basic requirement of being organized in proper logical units that are expected from any series to be accepted. (The worst part is patch 1) Regards, Aleksandsr > > > On Sat, Dec 21, 2019 at 7:15 PM Aleksandar Markovic < > aleksandar.m.mail@gmail.com> wrote: > >> >> >> On Saturday, December 21, 2019, Aleksandar Markovic < >> aleksandar.m.mail@gmail.com> wrote: >> >>> >>> >>> On Saturday, December 21, 2019, Michael Rolnik >>> wrote: >>> >>>> Hi Aleksandar. >>>> >>>> please explain. >>>> >>>>> >>>>> >>> Hi, Michael. >>> >>> I wanted to say: >>> >>> >>> 1. Cut the parts of insn.decode that describe coding of arithmetic and >>> logic instructions and include them in the patch: >>> >>> target/avr: Add instruction translation - Arithmetic and Logic Instructions >>> >>> >>> Since that would be the first time insn.decode is mentioned in the new >> organization of the series, the license preamble of insn.decode can be >> included in that patch, of course. >> >> Best wishes, >> Aleksandar >> >> >> >>> 2. Cut the parts of insn.decode that describe coding of branch instructions and include them in the patch: >>> >>> target/avr: Add instruction translation - Branch Instructions >>> >>> 3. Cut the parts of insn.decode that describe coding of data transfer instructions >>> and include them in the patch: >>> >>> target/avr: Add instruction translation - Data Transfer Instructions >>> >>> >>> 4. Cut the parts of insn.decode that describe coding of bit and bit-test >>> instructions and include them in the patch: >>> >>> target/avr: Add instruction translation - Bit and Bit-test Instructions >>> >>> >>> 5. Cut the parts of insn.decode that describe coding of MCU control instructions >>> and include them in the patch: >>> >>> target/avr: Add instruction translation - MCU Control Instructions >>> >>> >>> This way, your patches become logicaly-organized rather than file >>> organized. The patch on, let's say, arithmetic and logic instructions will >>> contain all elements needed for their implementation, rather than those >>> elements being split between decode and omplementation parts . >>> >>> >>> >>> Regards, >>> >>> Aleksandar >>> >>> >>> >>>> On Sat, Dec 21, 2019 at 1:18 PM Aleksandar Markovic < >>>> aleksandar.m.mail@gmail.com> wrote: >>>> >>>>> >>>>> >>>>> On Wednesday, December 18, 2019, Michael Rolnik >>>>> wrote: >>>>> >>>>>> This includes: >>>>>> - encoding of all 16 bit instructions >>>>>> - encoding of all 32 bit instructions >>>>>> >>>>>> Signed-off-by: Michael Rolnik >>>>>> Tested-by: Philippe Mathieu-Daudé >>>>>> --- >>>>> >>>>> >>>>> Michael, >>>>> >>>>> I am very pleased that you rearranged the order to be in sync with the >>>>> documentation. >>>>> >>>>> Now, for the next version, I would ask you to make this patch >>>>> disappear. >>>>> >>>>> More precisely, "MCU Control Instructions" section of insn.decode file >>>>> move to be a part of "Add MCU Control Instructions" (not sure abiut the >>>>> title, but it is 6 or 7 patches after this one) patch, and so on, in the >>>>> same fashion, for all groups of instructions. >>>>> >>>>> Kind regards, >>>>> >>>>> Aleksandar >>>>> >>>>> >>>>> >>>>> target/avr/insn.decode | 183 ++++++++++++++++++++++++++++++ >>>>>> +++++++++++ >>>>>> 1 file changed, 183 insertions(+) >>>>>> create mode 100644 target/avr/insn.decode >>>>>> >>>>>> diff --git a/target/avr/insn.decode b/target/avr/insn.decode >>>>>> new file mode 100644 >>>>>> index 0000000000..0e4ec9ddf0 >>>>>> --- /dev/null >>>>>> +++ b/target/avr/insn.decode >>>>>> @@ -0,0 +1,183 @@ >>>>>> +# >>>>>> +# AVR instruction decode definitions. >>>>>> +# >>>>>> +# Copyright (c) 2019 Michael Rolnik >>>>>> +# >>>>>> +# This library is free software; you can redistribute it and/or >>>>>> +# modify it under the terms of the GNU Lesser General Public >>>>>> +# License as published by the Free Software Foundation; either >>>>>> +# version 2.1 of the License, or (at your option) any later version. >>>>>> +# >>>>>> +# This library is distributed in the hope that it will be useful, >>>>>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of >>>>>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >>>>>> +# Lesser General Public License for more details. >>>>>> +# >>>>>> +# You should have received a copy of the GNU Lesser General Public >>>>>> +# License along with this library; if not, see < >>>>>> http://www.gnu.org/licenses/>. >>>>>> +# >>>>>> + >>>>>> +# >>>>>> +# regs_16_31_by_one = [16 .. 31] >>>>>> +# regs_16_23_by_one = [16 .. 23] >>>>>> +# regs_24_30_by_two = [24, 26, 28, 30] >>>>>> +# regs_00_30_by_two = [0, 2, 4, 6, 8, .. 30] >>>>>> + >>>>>> +%rd 4:5 >>>>>> +%rr 9:1 0:4 >>>>>> + >>>>>> +%rd_a 4:4 >>>>>> !function=to_regs_16_31_by_one >>>>>> +%rd_b 4:3 >>>>>> !function=to_regs_16_23_by_one >>>>>> +%rd_c 4:2 >>>>>> !function=to_regs_24_30_by_two >>>>>> +%rd_d 4:4 >>>>>> !function=to_regs_00_30_by_two >>>>>> +%rr_a 0:4 >>>>>> !function=to_regs_16_31_by_one >>>>>> +%rr_b 0:3 >>>>>> !function=to_regs_16_23_by_one >>>>>> +%rr_d 0:4 >>>>>> !function=to_regs_00_30_by_two >>>>>> + >>>>>> +%imm6 6:2 0:4 >>>>>> +%imm8 8:4 0:4 >>>>>> + >>>>>> +%io_imm 9:2 0:4 >>>>>> +%ldst_d_imm 13:1 10:2 0:3 >>>>>> + >>>>>> +# The 22-bit immediate is partially in the opcode word, >>>>>> +# and partially in the next. Use append_16 to build the >>>>>> +# complete 22-bit value. >>>>>> +%imm_call 4:5 0:1 !function=append_16 >>>>>> + >>>>>> + >>>>>> +&rd_rr rd rr >>>>>> +&rd_imm rd imm >>>>>> + >>>>>> +@op_rd_rr .... .. . ..... .... &rd_rr rd=%rd rr=%rr >>>>>> +@op_rd_imm6 .... .... .. .. .... &rd_imm rd=%rd_c >>>>>> imm=%imm6 >>>>>> +@op_rd_imm8 .... .... .... .... &rd_imm rd=%rd_a >>>>>> imm=%imm8 >>>>>> +@op_bit .... .... . bit:3 .... >>>>>> +@op_bit_imm .... .. imm:s7 bit:3 >>>>>> +@fmul .... .... . ... . ... &rd_rr rd=%rd_b >>>>>> rr=%rr_b >>>>>> +@io_rd_imm .... . .. ..... .... &rd_imm rd=%rd >>>>>> imm=%io_imm >>>>>> +@ldst_d .. . . .. . rd:5 . ... &rd_imm >>>>>> imm=%ldst_d_imm >>>>>> + >>>>>> +# The 16-bit immediate is completely in the next word. >>>>>> +# Fields cannot be defined with no bits, so we cannot play >>>>>> +# the same trick and append to a zero-bit value. >>>>>> +# Defer reading the immediate until trans_{LDS,STS}. >>>>>> +@ldst_s .... ... rd:5 .... imm=0 >>>>>> + >>>>>> +# >>>>>> +# Arithmetic Instructions >>>>>> +# >>>>>> +ADD 0000 11 . ..... .... @op_rd_rr >>>>>> +ADC 0001 11 . ..... .... @op_rd_rr >>>>>> +ADIW 1001 0110 .. .. .... @op_rd_imm6 >>>>>> +SUB 0001 10 . ..... .... @op_rd_rr >>>>>> +SUBI 0101 .... .... .... @op_rd_imm8 >>>>>> +SBC 0000 10 . ..... .... @op_rd_rr >>>>>> +SBCI 0100 .... .... .... @op_rd_imm8 >>>>>> +SBIW 1001 0111 .. .. .... @op_rd_imm6 >>>>>> +AND 0010 00 . ..... .... @op_rd_rr >>>>>> +ANDI 0111 .... .... .... @op_rd_imm8 >>>>>> +OR 0010 10 . ..... .... @op_rd_rr >>>>>> +ORI 0110 .... .... .... @op_rd_imm8 >>>>>> +EOR 0010 01 . ..... .... @op_rd_rr >>>>>> +COM 1001 010 rd:5 0000 >>>>>> +NEG 1001 010 rd:5 0001 >>>>>> +INC 1001 010 rd:5 0011 >>>>>> +DEC 1001 010 rd:5 1010 >>>>>> +MUL 1001 11 . ..... .... @op_rd_rr >>>>>> +MULS 0000 0010 .... .... &rd_rr rd=%rd_a >>>>>> rr=%rr_a >>>>>> +MULSU 0000 0011 0 ... 0 ... @fmul >>>>>> +FMUL 0000 0011 0 ... 1 ... @fmul >>>>>> +FMULS 0000 0011 1 ... 0 ... @fmul >>>>>> +FMULSU 0000 0011 1 ... 1 ... @fmul >>>>>> +DES 1001 0100 imm:4 1011 >>>>>> + >>>>>> +# >>>>>> +# Branch Instructions >>>>>> +# >>>>>> +RJMP 1100 imm:s12 >>>>>> +IJMP 1001 0100 0000 1001 >>>>>> +EIJMP 1001 0100 0001 1001 >>>>>> +JMP 1001 010 ..... 110 . imm=%imm_call >>>>>> +RCALL 1101 imm:s12 >>>>>> +ICALL 1001 0101 0000 1001 >>>>>> +EICALL 1001 0101 0001 1001 >>>>>> +CALL 1001 010 ..... 111 . imm=%imm_call >>>>>> +RET 1001 0101 0000 1000 >>>>>> +RETI 1001 0101 0001 1000 >>>>>> +CPSE 0001 00 . ..... .... @op_rd_rr >>>>>> +CP 0001 01 . ..... .... @op_rd_rr >>>>>> +CPC 0000 01 . ..... .... @op_rd_rr >>>>>> +CPI 0011 .... .... .... @op_rd_imm8 >>>>>> +SBRC 1111 110 rr:5 0 bit:3 >>>>>> +SBRS 1111 111 rr:5 0 bit:3 >>>>>> +SBIC 1001 1001 reg:5 bit:3 >>>>>> +SBIS 1001 1011 reg:5 bit:3 >>>>>> +BRBS 1111 00 ....... ... @op_bit_imm >>>>>> +BRBC 1111 01 ....... ... @op_bit_imm >>>>>> + >>>>>> +# >>>>>> +# Data Transfer Instructions >>>>>> +# >>>>>> +MOV 0010 11 . ..... .... @op_rd_rr >>>>>> +MOVW 0000 0001 .... .... &rd_rr rd=%rd_d >>>>>> rr=%rr_d >>>>>> +LDI 1110 .... .... .... @op_rd_imm8 >>>>>> +LDS 1001 000 ..... 0000 @ldst_s >>>>>> +LDX1 1001 000 rd:5 1100 >>>>>> +LDX2 1001 000 rd:5 1101 >>>>>> +LDX3 1001 000 rd:5 1110 >>>>>> +LDY2 1001 000 rd:5 1001 >>>>>> +LDY3 1001 000 rd:5 1010 >>>>>> +LDZ2 1001 000 rd:5 0001 >>>>>> +LDZ3 1001 000 rd:5 0010 >>>>>> +LDDY 10 . 0 .. 0 ..... 1 ... @ldst_d >>>>>> +LDDZ 10 . 0 .. 0 ..... 0 ... @ldst_d >>>>>> +STS 1001 001 ..... 0000 @ldst_s >>>>>> +STX1 1001 001 rr:5 1100 >>>>>> +STX2 1001 001 rr:5 1101 >>>>>> +STX3 1001 001 rr:5 1110 >>>>>> +STY2 1001 001 rd:5 1001 >>>>>> +STY3 1001 001 rd:5 1010 >>>>>> +STZ2 1001 001 rd:5 0001 >>>>>> +STZ3 1001 001 rd:5 0010 >>>>>> +STDY 10 . 0 .. 1 ..... 1 ... @ldst_d >>>>>> +STDZ 10 . 0 .. 1 ..... 0 ... @ldst_d >>>>>> +LPM1 1001 0101 1100 1000 >>>>>> +LPM2 1001 000 rd:5 0100 >>>>>> +LPMX 1001 000 rd:5 0101 >>>>>> +ELPM1 1001 0101 1101 1000 >>>>>> +ELPM2 1001 000 rd:5 0110 >>>>>> +ELPMX 1001 000 rd:5 0111 >>>>>> +SPM 1001 0101 1110 1000 >>>>>> +SPMX 1001 0101 1111 1000 >>>>>> +IN 1011 0 .. ..... .... @io_rd_imm >>>>>> +OUT 1011 1 .. ..... .... @io_rd_imm >>>>>> +PUSH 1001 001 rd:5 1111 >>>>>> +POP 1001 000 rd:5 1111 >>>>>> +XCH 1001 001 rd:5 0100 >>>>>> +LAC 1001 001 rd:5 0110 >>>>>> +LAS 1001 001 rd:5 0101 >>>>>> +LAT 1001 001 rd:5 0111 >>>>>> + >>>>>> +# >>>>>> +# Bit and Bit-test Instructions >>>>>> +# >>>>>> +LSR 1001 010 rd:5 0110 >>>>>> +ROR 1001 010 rd:5 0111 >>>>>> +ASR 1001 010 rd:5 0101 >>>>>> +SWAP 1001 010 rd:5 0010 >>>>>> +SBI 1001 1010 reg:5 bit:3 >>>>>> +CBI 1001 1000 reg:5 bit:3 >>>>>> +BST 1111 101 rd:5 0 bit:3 >>>>>> +BLD 1111 100 rd:5 0 bit:3 >>>>>> +BSET 1001 0100 0 bit:3 1000 >>>>>> +BCLR 1001 0100 1 bit:3 1000 >>>>>> + >>>>>> +# >>>>>> +# MCU Control Instructions >>>>>> +# >>>>>> +BREAK 1001 0101 1001 1000 >>>>>> +NOP 0000 0000 0000 0000 >>>>>> +SLEEP 1001 0101 1000 1000 >>>>>> +WDR 1001 0101 1010 1000 >>>>>> + >>>>>> -- >>>>>> 2.17.2 (Apple Git-113) >>>>>> >>>>>> >>>> >>>> -- >>>> Best Regards, >>>> Michael Rolnik >>>> >>> > > -- > Best Regards, > Michael Rolnik >