On 11/12/21 22:05, Richard Henderson wrote:
On 11/12/21 7:53 AM, Song Gao wrote:
+#
+# Fields
+#
+%rd 0:5
+%rj 5:5
+%rk 10:5
+%sa2 15:2
+%si12 10:s12
+%ui12 10:12
+%si16 10:s16
+%si20 5:s20
You should only create separate field definitions like this when they
are complex: e.g. the logical field is disjoint or there's a need for
!function.
+
+#
+# Argument sets
+#
+&fmt_rdrjrk rd rj rk
+&fmt_rdrjsi12 rd rj si12
+&fmt_rdrjrksa2 rd rj rk sa2
+&fmt_rdrjsi16 rd rj si16
+&fmt_rdrjui12 rd rj ui12
+&fmt_rdsi20 rd si20
Some of these should be combined. The width of the immediate is a
detail of the format, not the decoded argument set. Thus you should have
&fmt_rdimm rd imm
&fmt_rdrjimm rd rj imm
&fmt_rdrjrk rd rj rk
&fmt_rdrjrksa rd rj rk sa
I'd like to add, that the organization of the whole decodetree file
closely resembles that of the ISA manual, most likely on purpose (while
not stated anywhere in the patch). However the manual itself is not
without errors or inconsistencies; for example, the 9 "base instruction
formats" classification is nowhere near accurate, and here we can see
the author is forced to create ad-hoc names (repeating the operand
slots). I suggest just generating the descriptions from the
loongarch-opcodes project [1]; no need to duplicate work. I'll happily
help if you decide to do that.
[1]: https://github.com/loongson-community/loongarch-opcodes
+alsl_w 0000 00000000 010 .. ..... ..... .....
@fmt_rdrjrksa2
+alsl_wu 0000 00000000 011 .. ..... ..... ..... @fmt_rdrjrksa2
+alsl_d 0000 00000010 110 .. ..... ..... ..... @fmt_rdrjrksa2
The encoding of these insns is that the shift is sa+1.
While you compensate for this in gen_alsl_*, we print the "wrong"
number in the disassembly. I think it would be better to do
%sa2p1 15:2 !function=plus_1
@fmt_rdrjrksa2p1 .... ........ ... .. rk:5 rj:5 rd:5 \
&fmt_rdrjrksa sa=%sa2p1
Here again, the manual was inconsistent with the binutils
implementation; the manual says (for ALSL.W, it's SLADD in
loongarch-opcodes project's revised mnemonics):
"ALSL.W logically left-shifts rj[31:0] by (sa2+1) bits, [snip]"
(translation mine, not copied from the official translation)
Clearly the "+1" part is not meant to show up in disassembly. Yet the
binutils implementation acts as if the operand should be pre-added 1 in
source code, and disassembles and prints as such, obvious mismatch here.
I'd suggest fixing the disassembly code to remove this inconsistency.
And the "+1" "feature" is not used anywhere else AFAIK, so it wouldn't
hurt to just delete everything about it.
r~