public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v1 0/2] Add support for "pcaddi rd, label"
@ 2023-08-09  1:39 mengqinggang
  2023-08-09  1:39 ` [PATCH v1 1/2] LoongArch: Fix pcaddi format string mengqinggang
  2023-08-09  1:39 ` [PATCH v1 2/2] LoongArch: Add support for "pcaddi rd, label" mengqinggang
  0 siblings, 2 replies; 10+ messages in thread
From: mengqinggang @ 2023-08-09  1:39 UTC (permalink / raw)
  To: binutils
  Cc: xuchenghua, chenglulu, liuzhensong, xry111, i.swmail, maskray,
	mengqinggang

Changes:
  According to Wang Xuerui's suggestion, change "pc20" to "pcrel_20".

mengqinggang (2):
  LoongArch: Fix pcaddi format string
  LoongArch: Add support for "pcaddi rd, label"

 bfd/elfxx-loongarch.c                 |   4 +-
 gas/testsuite/gas/loongarch/imm_ins.d | 137 +++++++++++++-------------
 gas/testsuite/gas/loongarch/imm_op.d  |  82 +++++++--------
 gas/testsuite/gas/loongarch/imm_op.s  |   4 +-
 gas/testsuite/gas/loongarch/pcaddi.d  |  13 +++
 gas/testsuite/gas/loongarch/pcaddi.s  |   4 +
 opcodes/loongarch-opc.c               |   3 +-
 7 files changed, 133 insertions(+), 114 deletions(-)
 create mode 100644 gas/testsuite/gas/loongarch/pcaddi.d
 create mode 100644 gas/testsuite/gas/loongarch/pcaddi.s

-- 
2.36.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v1 1/2] LoongArch: Fix pcaddi format string
  2023-08-09  1:39 [PATCH v1 0/2] Add support for "pcaddi rd, label" mengqinggang
@ 2023-08-09  1:39 ` mengqinggang
  2023-08-09 11:37   ` Xi Ruoyao
  2023-08-09  1:39 ` [PATCH v1 2/2] LoongArch: Add support for "pcaddi rd, label" mengqinggang
  1 sibling, 1 reply; 10+ messages in thread
From: mengqinggang @ 2023-08-09  1:39 UTC (permalink / raw)
  To: binutils
  Cc: xuchenghua, chenglulu, liuzhensong, xry111, i.swmail, maskray,
	mengqinggang

Add "<<2" for pcaddi format string
---
 opcodes/loongarch-opc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/opcodes/loongarch-opc.c b/opcodes/loongarch-opc.c
index 2f02e33dbec..7d110683e93 100644
--- a/opcodes/loongarch-opc.c
+++ b/opcodes/loongarch-opc.c
@@ -564,7 +564,7 @@ static struct loongarch_opcode loongarch_imm_opcodes[] =
   { 0x10000000, 0xfc000000,	"addu16i.d",	"r0:5,r5:5,s10:16",		0,			0,	0,	0 },
   { 0x14000000, 0xfe000000,	"lu12i.w",	"r0:5,s5:20",			0,			0,	0,	0 },
   { 0x16000000, 0xfe000000,	"lu32i.d",	"r0:5,s5:20",			0,			0,	0,	0 },
-  { 0x18000000, 0xfe000000,	"pcaddi",	"r0:5,s5:20",			0,			0,	0,	0 },
+  { 0x18000000, 0xfe000000,	"pcaddi",	"r0:5,s5:20<<2",		0,			0,	0,	0 },
   { 0x1a000000, 0xfe000000,	"pcalau12i",	"r0:5,s5:20",			0,			0,	0,	0 },
   { 0x1c000000, 0xfe000000,	"pcaddu12i",	"r0:5,s5:20",			0,			0,	0,	0 },
   { 0x1e000000, 0xfe000000,	"pcaddu18i",	"r0:5,s5:20",			0,			0,	0,	0 },
-- 
2.36.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v1 2/2] LoongArch: Add support for "pcaddi rd, label"
  2023-08-09  1:39 [PATCH v1 0/2] Add support for "pcaddi rd, label" mengqinggang
  2023-08-09  1:39 ` [PATCH v1 1/2] LoongArch: Fix pcaddi format string mengqinggang
@ 2023-08-09  1:39 ` mengqinggang
  1 sibling, 0 replies; 10+ messages in thread
From: mengqinggang @ 2023-08-09  1:39 UTC (permalink / raw)
  To: binutils
  Cc: xuchenghua, chenglulu, liuzhensong, xry111, i.swmail, maskray,
	mengqinggang

Add a macro pcaddi instruction to support "pcaddi rd, label".

pcaddi has a 20-bit signed immediate, it can address a +/- 2MB pc relative
address, and the address should be 4-byte aligned.
---
 bfd/elfxx-loongarch.c                 |   4 +-
 gas/testsuite/gas/loongarch/imm_ins.d | 137 +++++++++++++-------------
 gas/testsuite/gas/loongarch/imm_op.d  |  82 +++++++--------
 gas/testsuite/gas/loongarch/imm_op.s  |   4 +-
 gas/testsuite/gas/loongarch/pcaddi.d  |  13 +++
 gas/testsuite/gas/loongarch/pcaddi.s  |   4 +
 opcodes/loongarch-opc.c               |   1 +
 7 files changed, 132 insertions(+), 113 deletions(-)
 create mode 100644 gas/testsuite/gas/loongarch/pcaddi.d
 create mode 100644 gas/testsuite/gas/loongarch/pcaddi.s

diff --git a/bfd/elfxx-loongarch.c b/bfd/elfxx-loongarch.c
index f27c9fdba6a..f20b3eafca0 100644
--- a/bfd/elfxx-loongarch.c
+++ b/bfd/elfxx-loongarch.c
@@ -1415,7 +1415,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] =
 	 NULL,					/* adjust_reloc_bits.  */
 	 NULL),					/* larch_reloc_type_name.  */
 
-  /* pcala_hi20 + pcala_lo12 relaxed to pcrel20_s2.  */
+  /* For pcaddi, and pcala_hi20 + pcala_lo12 can relax to pcrel_20.  */
   LOONGARCH_HOWTO (R_LARCH_PCREL20_S2,		/* type (103).  */
 	 2,					/* rightshift.  */
 	 4,					/* size.  */
@@ -1431,7 +1431,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] =
 	 false,					/* pcrel_offset.  */
 	 BFD_RELOC_LARCH_PCREL20_S2,		/* bfd_reloc_code_real_type.  */
 	 reloc_sign_bits,			/* adjust_reloc_bits.  */
-	 NULL),					/* larch_reloc_type_name.  */
+	 "pcrel_20"),				/* larch_reloc_type_name.  */
 
   /* Canonical Frame Address.  */
   LOONGARCH_HOWTO (R_LARCH_CFA,			/* type (104).  */
diff --git a/gas/testsuite/gas/loongarch/imm_ins.d b/gas/testsuite/gas/loongarch/imm_ins.d
index f00110cd8a3..b54df8735a8 100644
--- a/gas/testsuite/gas/loongarch/imm_ins.d
+++ b/gas/testsuite/gas/loongarch/imm_ins.d
@@ -7,74 +7,75 @@
 
 Disassembly of section .text:
 
-00000000.* <.text>:
-[ 	]+0:[ 	]+03848c0c[ 	]+li.w[ 	]+\$t0,[ 	]+0x123
-[ 	]+4:[ 	]+15ffe00d[ 	]+lu12i.w[ 	]+\$t1,[ 	]+-256
-[ 	]+8:[ 	]+16001fed[ 	]+lu32i.d[ 	]+\$t1,[ 	]+255
-[ 	]+c:[ 	]+02bffc0e[ 	]+li.w[ 	]+\$t2,[ 	]+-1
-[ 	]+10:[ 	]+1601ffee[ 	]+lu32i.d[ 	]+\$t2,[ 	]+4095
-[ 	]+14:[ 	]+0004b58b[ 	]+alsl.w[ 	]+\$a7,[ 	]+\$t0,[ 	]+\$t1,[ 	]+0x2
-[ 	]+18:[ 	]+0006b58b[ 	]+alsl.wu[ 	]+\$a7,[ 	]+\$t0,[ 	]+\$t1,[ 	]+0x2
-[ 	]+1c:[ 	]+0009358b[ 	]+bytepick.w[ 	]+\$a7,[ 	]+\$t0,[ 	]+\$t1,[ 	]+0x2
-[ 	]+20:[ 	]+000d358b[ 	]+bytepick.d[ 	]+\$a7,[ 	]+\$t0,[ 	]+\$t1,[ 	]+0x2
+[ 	]*0000000000000000 <.text>:
+[ 	]+0:[ 	]+03848c0c[ 	]+li.w[ 	]+\$t0, 0x123
+[ 	]+4:[ 	]+15ffe00d[ 	]+lu12i.w[ 	]+\$t1, -256
+[ 	]+8:[ 	]+16001fed[ 	]+lu32i.d[ 	]+\$t1, 255
+[ 	]+c:[ 	]+02bffc0e[ 	]+li.w[ 	]+\$t2, -1
+[ 	]+10:[ 	]+1601ffee[ 	]+lu32i.d[ 	]+\$t2, 4095
+[ 	]+14:[ 	]+0004b58b[ 	]+alsl.w[ 	]+\$a7, \$t0, \$t1, 0x2
+[ 	]+18:[ 	]+0006b58b[ 	]+alsl.wu[ 	]+\$a7, \$t0, \$t1, 0x2
+[ 	]+1c:[ 	]+0009358b[ 	]+bytepick.w[ 	]+\$a7, \$t0, \$t1, 0x2
+[ 	]+20:[ 	]+000d358b[ 	]+bytepick.d[ 	]+\$a7, \$t0, \$t1, 0x2
 [ 	]+24:[ 	]+002a0002[ 	]+break[ 	]+0x2
 [ 	]+28:[ 	]+002a8002[ 	]+dbcl[ 	]+0x2
 [ 	]+2c:[ 	]+002b0002[ 	]+syscall[ 	]+0x2
-[ 	]+30:[ 	]+002cb58b[ 	]+alsl.d[ 	]+\$a7,[ 	]+\$t0,[ 	]+\$t1,[ 	]+0x2
-[ 	]+34:[ 	]+0040898b[ 	]+slli.w[ 	]+\$a7,[ 	]+\$t0,[ 	]+0x2
-[ 	]+38:[ 	]+0041098b[ 	]+slli.d[ 	]+\$a7,[ 	]+\$t0,[ 	]+0x2
-[ 	]+3c:[ 	]+0044898b[ 	]+srli.w[ 	]+\$a7,[ 	]+\$t0,[ 	]+0x2
-[ 	]+40:[ 	]+004509ac[ 	]+srli.d[ 	]+\$t0,[ 	]+\$t1,[ 	]+0x2
-[ 	]+44:[ 	]+004889ac[ 	]+srai.w[ 	]+\$t0,[ 	]+\$t1,[ 	]+0x2
-[ 	]+48:[ 	]+004909ac[ 	]+srai.d[ 	]+\$t0,[ 	]+\$t1,[ 	]+0x2
-[ 	]+4c:[ 	]+006209ac[ 	]+bstrins.w[ 	]+\$t0,[ 	]+\$t1,[ 	]+0x2,[ 	]+0x2
-[ 	]+50:[ 	]+008209ac[ 	]+bstrins.d[ 	]+\$t0,[ 	]+\$t1,[ 	]+0x2,[ 	]+0x2
-[ 	]+54:[ 	]+00c209ac[ 	]+bstrpick.d[ 	]+\$t0,[ 	]+\$t1,[ 	]+0x2,[ 	]+0x2
-[ 	]+58:[ 	]+00c209ac[ 	]+bstrpick.d[ 	]+\$t0,[ 	]+\$t1,[ 	]+0x2,[ 	]+0x2
-[ 	]+5c:[ 	]+02048dac[ 	]+slti[ 	]+\$t0,[ 	]+\$t1,[ 	]+291
-[ 	]+60:[ 	]+02448dac[ 	]+sltui[ 	]+\$t0,[ 	]+\$t1,[ 	]+291
-[ 	]+64:[ 	]+02848dac[ 	]+addi.w[ 	]+\$t0,[ 	]+\$t1,[ 	]+291
-[ 	]+68:[ 	]+02c48dac[ 	]+addi.d[ 	]+\$t0,[ 	]+\$t1,[ 	]+291
-[ 	]+6c:[ 	]+03048dac[ 	]+lu52i.d[ 	]+\$t0,[ 	]+\$t1,[ 	]+291
-[ 	]+70:[ 	]+034009ac[ 	]+andi[ 	]+\$t0,[ 	]+\$t1,[ 	]+0x2
-[ 	]+74:[ 	]+038009ac[ 	]+ori[ 	]+\$t0,[ 	]+\$t1,[ 	]+0x2
-[ 	]+78:[ 	]+03c009ac[ 	]+xori[ 	]+\$t0,[ 	]+\$t1,[ 	]+0x2
-[ 	]+7c:[ 	]+100009ac[ 	]+addu16i.d[ 	]+\$t0,[ 	]+\$t1,[ 	]+2
-[ 	]+80:[ 	]+1400246c[ 	]+lu12i.w[ 	]+\$t0,[ 	]+291
-[ 	]+84:[ 	]+1600246c[ 	]+lu32i.d[ 	]+\$t0,[ 	]+291
-[ 	]+88:[ 	]+1800246c[ 	]+pcaddi[ 	]+\$t0,[ 	]+291
-[ 	]+8c:[ 	]+1a00246c[ 	]+pcalau12i[ 	]+\$t0,[ 	]+291
-[ 	]+90:[ 	]+1c00246c[ 	]+pcaddu12i[ 	]+\$t0,[ 	]+291
-[ 	]+94:[ 	]+1e00246c[ 	]+pcaddu18i[ 	]+\$t0,[ 	]+291
-[ 	]+98:[ 	]+04048c0c[ 	]+csrrd[ 	]+\$t0,[ 	]+0x123
-[ 	]+9c:[ 	]+04048c2c[ 	]+csrwr[ 	]+\$t0,[ 	]+0x123
-[ 	]+a0:[ 	]+040009ac[ 	]+csrxchg[ 	]+\$t0,[ 	]+\$t1,[ 	]+0x2
-[ 	]+a4:[ 	]+060009a2[ 	]+cacop[ 	]+0x2,[ 	]+\$t1,[ 	]+2
-[ 	]+a8:[ 	]+064009ac[ 	]+lddir[ 	]+\$t0,[ 	]+\$t1,[ 	]+0x2
-[ 	]+ac:[ 	]+06440980[ 	]+ldpte[ 	]+\$t0,[ 	]+0x2
-[ 	]+b0:[ 	]+0649b9a2[ 	]+invtlb[ 	]+0x2,[ 	]+\$t1,[ 	]+\$t2
-[ 	]+b4:[ 	]+200101ac[ 	]+ll.w[ 	]+\$t0,[ 	]+\$t1,[ 	]+256
-[ 	]+b8:[ 	]+210101ac[ 	]+sc.w[ 	]+\$t0,[ 	]+\$t1,[ 	]+256
-[ 	]+bc:[ 	]+220101ac[ 	]+ll.d[ 	]+\$t0,[ 	]+\$t1,[ 	]+256
-[ 	]+c0:[ 	]+230101ac[ 	]+sc.d[ 	]+\$t0,[ 	]+\$t1,[ 	]+256
-[ 	]+c4:[ 	]+240101ac[ 	]+ldptr.w[ 	]+\$t0,[ 	]+\$t1,[ 	]+256
-[ 	]+c8:[ 	]+250101ac[ 	]+stptr.w[ 	]+\$t0,[ 	]+\$t1,[ 	]+256
-[ 	]+cc:[ 	]+260101ac[ 	]+ldptr.d[ 	]+\$t0,[ 	]+\$t1,[ 	]+256
-[ 	]+d0:[ 	]+270101ac[ 	]+stptr.d[ 	]+\$t0,[ 	]+\$t1,[ 	]+256
-[ 	]+d4:[ 	]+280401ac[ 	]+ld.b[ 	]+\$t0,[ 	]+\$t1,[ 	]+256
-[ 	]+d8:[ 	]+284401ac[ 	]+ld.h[ 	]+\$t0,[ 	]+\$t1,[ 	]+256
-[ 	]+dc:[ 	]+288401ac[ 	]+ld.w[ 	]+\$t0,[ 	]+\$t1,[ 	]+256
-[ 	]+e0:[ 	]+28c401ac[ 	]+ld.d[ 	]+\$t0,[ 	]+\$t1,[ 	]+256
-[ 	]+e4:[ 	]+290401ac[ 	]+st.b[ 	]+\$t0,[ 	]+\$t1,[ 	]+256
-[ 	]+e8:[ 	]+294401ac[ 	]+st.h[ 	]+\$t0,[ 	]+\$t1,[ 	]+256
-[ 	]+ec:[ 	]+298401ac[ 	]+st.w[ 	]+\$t0,[ 	]+\$t1,[ 	]+256
-[ 	]+f0:[ 	]+29c401ac[ 	]+st.d[ 	]+\$t0,[ 	]+\$t1,[ 	]+256
-[ 	]+f4:[ 	]+2a0401ac[ 	]+ld.bu[ 	]+\$t0,[ 	]+\$t1,[ 	]+256
-[ 	]+f8:[ 	]+2a4401ac[ 	]+ld.hu[ 	]+\$t0,[ 	]+\$t1,[ 	]+256
-[ 	]+fc:[ 	]+2a8401ac[ 	]+ld.wu[ 	]+\$t0,[ 	]+\$t1,[ 	]+256
-[ 	]+100:[ 	]+2ac401a2[ 	]+preld[ 	]+0x2,[ 	]+\$t1,[ 	]+256
-[ 	]+104:[ 	]+382c39a2[ 	]+preldx[ 	]+0x2,[ 	]+\$t1,[ 	]+\$t2
-[ 	]+108:[ 	]+2b048d8a[ 	]+fld.s[ 	]+\$ft2,[ 	]+\$t0,[ 	]+291
-[ 	]+10c:[ 	]+2b448d8a[ 	]+fst.s[ 	]+\$ft2,[ 	]+\$t0,[ 	]+291
-[ 	]+110:[ 	]+2b848d8a[ 	]+fld.d[ 	]+\$ft2,[ 	]+\$t0,[ 	]+291
-[ 	]+114:[ 	]+2bc48d8a[ 	]+fst.d[ 	]+\$ft2,[ 	]+\$t0,[ 	]+291
+[ 	]+30:[ 	]+002cb58b[ 	]+alsl.d[ 	]+\$a7, \$t0, \$t1, 0x2
+[ 	]+34:[ 	]+0040898b[ 	]+slli.w[ 	]+\$a7, \$t0, 0x2
+[ 	]+38:[ 	]+0041098b[ 	]+slli.d[ 	]+\$a7, \$t0, 0x2
+[ 	]+3c:[ 	]+0044898b[ 	]+srli.w[ 	]+\$a7, \$t0, 0x2
+[ 	]+40:[ 	]+004509ac[ 	]+srli.d[ 	]+\$t0, \$t1, 0x2
+[ 	]+44:[ 	]+004889ac[ 	]+srai.w[ 	]+\$t0, \$t1, 0x2
+[ 	]+48:[ 	]+004909ac[ 	]+srai.d[ 	]+\$t0, \$t1, 0x2
+[ 	]+4c:[ 	]+006209ac[ 	]+bstrins.w[ 	]+\$t0, \$t1, 0x2, 0x2
+[ 	]+50:[ 	]+008209ac[ 	]+bstrins.d[ 	]+\$t0, \$t1, 0x2, 0x2
+[ 	]+54:[ 	]+00c209ac[ 	]+bstrpick.d[ 	]+\$t0, \$t1, 0x2, 0x2
+[ 	]+58:[ 	]+00c209ac[ 	]+bstrpick.d[ 	]+\$t0, \$t1, 0x2, 0x2
+[ 	]+5c:[ 	]+02048dac[ 	]+slti[ 	]+\$t0, \$t1, 291
+[ 	]+60:[ 	]+02448dac[ 	]+sltui[ 	]+\$t0, \$t1, 291
+[ 	]+64:[ 	]+02848dac[ 	]+addi.w[ 	]+\$t0, \$t1, 291
+[ 	]+68:[ 	]+02c48dac[ 	]+addi.d[ 	]+\$t0, \$t1, 291
+[ 	]+6c:[ 	]+03048dac[ 	]+lu52i.d[ 	]+\$t0, \$t1, 291
+[ 	]+70:[ 	]+034009ac[ 	]+andi[ 	]+\$t0, \$t1, 0x2
+[ 	]+74:[ 	]+038009ac[ 	]+ori[ 	]+\$t0, \$t1, 0x2
+[ 	]+78:[ 	]+03c009ac[ 	]+xori[ 	]+\$t0, \$t1, 0x2
+[ 	]+7c:[ 	]+100009ac[ 	]+addu16i.d[ 	]+\$t0, \$t1, 2
+[ 	]+80:[ 	]+1400246c[ 	]+lu12i.w[ 	]+\$t0, 291
+[ 	]+84:[ 	]+1600246c[ 	]+lu32i.d[ 	]+\$t0, 291
+[ 	]+88:[ 	]+1800000c[ 	]+pcaddi[ 	]+\$t0, 0
+[ 	]+88: R_LARCH_PCREL20_S2[ 	]+\*ABS\*\+0x123
+[ 	]+8c:[ 	]+1a00246c[ 	]+pcalau12i[ 	]+\$t0, 291
+[ 	]+90:[ 	]+1c00246c[ 	]+pcaddu12i[ 	]+\$t0, 291
+[ 	]+94:[ 	]+1e00246c[ 	]+pcaddu18i[ 	]+\$t0, 291
+[ 	]+98:[ 	]+04048c0c[ 	]+csrrd[ 	]+\$t0, 0x123
+[ 	]+9c:[ 	]+04048c2c[ 	]+csrwr[ 	]+\$t0, 0x123
+[ 	]+a0:[ 	]+040009ac[ 	]+csrxchg[ 	]+\$t0, \$t1, 0x2
+[ 	]+a4:[ 	]+060009a2[ 	]+cacop[ 	]+0x2, \$t1, 2
+[ 	]+a8:[ 	]+064009ac[ 	]+lddir[ 	]+\$t0, \$t1, 0x2
+[ 	]+ac:[ 	]+06440980[ 	]+ldpte[ 	]+\$t0, 0x2
+[ 	]+b0:[ 	]+0649b9a2[ 	]+invtlb[ 	]+0x2, \$t1, \$t2
+[ 	]+b4:[ 	]+200101ac[ 	]+ll.w[ 	]+\$t0, \$t1, 256
+[ 	]+b8:[ 	]+210101ac[ 	]+sc.w[ 	]+\$t0, \$t1, 256
+[ 	]+bc:[ 	]+220101ac[ 	]+ll.d[ 	]+\$t0, \$t1, 256
+[ 	]+c0:[ 	]+230101ac[ 	]+sc.d[ 	]+\$t0, \$t1, 256
+[ 	]+c4:[ 	]+240101ac[ 	]+ldptr.w[ 	]+\$t0, \$t1, 256
+[ 	]+c8:[ 	]+250101ac[ 	]+stptr.w[ 	]+\$t0, \$t1, 256
+[ 	]+cc:[ 	]+260101ac[ 	]+ldptr.d[ 	]+\$t0, \$t1, 256
+[ 	]+d0:[ 	]+270101ac[ 	]+stptr.d[ 	]+\$t0, \$t1, 256
+[ 	]+d4:[ 	]+280401ac[ 	]+ld.b[ 	]+\$t0, \$t1, 256
+[ 	]+d8:[ 	]+284401ac[ 	]+ld.h[ 	]+\$t0, \$t1, 256
+[ 	]+dc:[ 	]+288401ac[ 	]+ld.w[ 	]+\$t0, \$t1, 256
+[ 	]+e0:[ 	]+28c401ac[ 	]+ld.d[ 	]+\$t0, \$t1, 256
+[ 	]+e4:[ 	]+290401ac[ 	]+st.b[ 	]+\$t0, \$t1, 256
+[ 	]+e8:[ 	]+294401ac[ 	]+st.h[ 	]+\$t0, \$t1, 256
+[ 	]+ec:[ 	]+298401ac[ 	]+st.w[ 	]+\$t0, \$t1, 256
+[ 	]+f0:[ 	]+29c401ac[ 	]+st.d[ 	]+\$t0, \$t1, 256
+[ 	]+f4:[ 	]+2a0401ac[ 	]+ld.bu[ 	]+\$t0, \$t1, 256
+[ 	]+f8:[ 	]+2a4401ac[ 	]+ld.hu[ 	]+\$t0, \$t1, 256
+[ 	]+fc:[ 	]+2a8401ac[ 	]+ld.wu[ 	]+\$t0, \$t1, 256
+[ 	]+100:[ 	]+2ac401a2[ 	]+preld[ 	]+0x2, \$t1, 256
+[ 	]+104:[ 	]+382c39a2[ 	]+preldx[ 	]+0x2, \$t1, \$t2
+[ 	]+108:[ 	]+2b048d8a[ 	]+fld.s[ 	]+\$ft2, \$t0, 291
+[ 	]+10c:[ 	]+2b448d8a[ 	]+fst.s[ 	]+\$ft2, \$t0, 291
+[ 	]+110:[ 	]+2b848d8a[ 	]+fld.d[ 	]+\$ft2, \$t0, 291
+[ 	]+114:[ 	]+2bc48d8a[ 	]+fst.d[ 	]+\$ft2, \$t0, 291
diff --git a/gas/testsuite/gas/loongarch/imm_op.d b/gas/testsuite/gas/loongarch/imm_op.d
index 3d4cba45586..4c5cffc7101 100644
--- a/gas/testsuite/gas/loongarch/imm_op.d
+++ b/gas/testsuite/gas/loongarch/imm_op.d
@@ -1,48 +1,48 @@
 #as:
 #objdump: -dr
 
-.*:[ 	]+file format .*
+.*:[    ]+file format .*
 
 
 Disassembly of section .text:
 
-00000000.* <.text>:
-[ 	]+0:[ 	]+020000a4 [ 	]+slti[ 	]+[ 	]+\$a0, \$a1, 0
-[ 	]+4:[ 	]+021ffca4 [ 	]+slti[ 	]+[ 	]+\$a0, \$a1, 2047
-[ 	]+8:[ 	]+022004a4 [ 	]+slti[ 	]+[ 	]+\$a0, \$a1, -2047
-[ 	]+c:[ 	]+024000a4 [ 	]+sltui[ 	]+[ 	]+\$a0, \$a1, 0
-[ 	]+10:[ 	]+025ffca4 [ 	]+sltui[ 	]+[ 	]+\$a0, \$a1, 2047
-[ 	]+14:[ 	]+026004a4 [ 	]+sltui[ 	]+[ 	]+\$a0, \$a1, -2047
-[ 	]+18:[ 	]+028000a4 [ 	]+addi.w[ 	]+[ 	]+\$a0, \$a1, 0
-[ 	]+1c:[ 	]+029ffca4 [ 	]+addi.w[ 	]+[ 	]+\$a0, \$a1, 2047
-[ 	]+20:[ 	]+02a004a4 [ 	]+addi.w[ 	]+[ 	]+\$a0, \$a1, -2047
-[ 	]+24:[ 	]+02c000a4 [ 	]+addi.d[ 	]+[ 	]+\$a0, \$a1, 0
-[ 	]+28:[ 	]+02dffca4 [ 	]+addi.d[ 	]+[ 	]+\$a0, \$a1, 2047
-[ 	]+2c:[ 	]+02e004a4 [ 	]+addi.d[ 	]+[ 	]+\$a0, \$a1, -2047
-[ 	]+30:[ 	]+030000a4 [ 	]+lu52i.d[ 	]+[ 	]+\$a0, \$a1, 0
-[ 	]+34:[ 	]+031ffca4 [ 	]+lu52i.d[ 	]+[ 	]+\$a0, \$a1, 2047
-[ 	]+38:[ 	]+032004a4 [ 	]+lu52i.d[ 	]+[ 	]+\$a0, \$a1, -2047
-[ 	]+3c:[ 	]+034000a4 [ 	]+andi[ 	]+[ 	]+\$a0, \$a1, 0x0
-[ 	]+40:[ 	]+035ffca4 [ 	]+andi[ 	]+[ 	]+\$a0, \$a1, 0x7ff
-[ 	]+44:[ 	]+038000a4 [ 	]+ori[ 	]+[ 	]+\$a0, \$a1, 0x0
-[ 	]+48:[ 	]+039ffca4 [ 	]+ori[ 	]+[ 	]+\$a0, \$a1, 0x7ff
-[ 	]+4c:[ 	]+03c000a4 [ 	]+xori[ 	]+[ 	]+\$a0, \$a1, 0x0
-[ 	]+50:[ 	]+03dffca4 [ 	]+xori[ 	]+[ 	]+\$a0, \$a1, 0x7ff
-[ 	]+54:[ 	]+100000a4 [ 	]+addu16i.d[ 	]+[ 	]+\$a0, \$a1, 0
-[ 	]+58:[ 	]+11fffca4 [ 	]+addu16i.d[ 	]+[ 	]+\$a0, \$a1, 32767
-[ 	]+5c:[ 	]+120004a4 [ 	]+addu16i.d[ 	]+[ 	]+\$a0, \$a1, -32767
-[ 	]+60:[ 	]+14000004 [ 	]+lu12i.w[ 	]+[ 	]+\$a0, 0
-[ 	]+64:[ 	]+14ffffe4 [ 	]+lu12i.w[ 	]+[ 	]+\$a0, 524287
-[ 	]+68:[ 	]+17000024 [ 	]+lu32i.d[ 	]+[ 	]+\$a0, -524287
-[ 	]+6c:[ 	]+18000004 [ 	]+pcaddi[ 	]+[ 	]+\$a0, 0
-[ 	]+70:[ 	]+18ffffe4 [ 	]+pcaddi[ 	]+[ 	]+\$a0, 524287
-[ 	]+74:[ 	]+19000024 [ 	]+pcaddi[ 	]+[ 	]+\$a0, -524287
-[ 	]+78:[ 	]+1a000004 [ 	]+pcalau12i[ 	]+[ 	]+\$a0, 0
-[ 	]+7c:[ 	]+1affffe4 [ 	]+pcalau12i[ 	]+[ 	]+\$a0, 524287
-[ 	]+80:[ 	]+1b000024 [ 	]+pcalau12i[ 	]+[ 	]+\$a0, -524287
-[ 	]+84:[ 	]+1c000004 [ 	]+pcaddu12i[ 	]+[ 	]+\$a0, 0
-[ 	]+88:[ 	]+1cffffe4 [ 	]+pcaddu12i[ 	]+[ 	]+\$a0, 524287
-[ 	]+8c:[ 	]+1d000024 [ 	]+pcaddu12i[ 	]+[ 	]+\$a0, -524287
-[ 	]+90:[ 	]+1e000004 [ 	]+pcaddu18i[ 	]+[ 	]+\$a0, 0
-[ 	]+94:[ 	]+1effffe4 [ 	]+pcaddu18i[ 	]+[ 	]+\$a0, 524287
-[ 	]+98:[ 	]+1f000024 [ 	]+pcaddu18i[ 	]+[ 	]+\$a0, -524287
+[ 	]*0000000000000000 <.text>:
+[ 	]+0:[ 	]+020000a4[ 	]+slti[ 	]+\$a0, \$a1, 0
+[ 	]+4:[ 	]+021ffca4[ 	]+slti[ 	]+\$a0, \$a1, 2047
+[ 	]+8:[ 	]+022004a4[ 	]+slti[ 	]+\$a0, \$a1, -2047
+[ 	]+c:[ 	]+024000a4[ 	]+sltui[ 	]+\$a0, \$a1, 0
+[ 	]+10:[ 	]+025ffca4[ 	]+sltui[ 	]+\$a0, \$a1, 2047
+[ 	]+14:[ 	]+026004a4[ 	]+sltui[ 	]+\$a0, \$a1, -2047
+[ 	]+18:[ 	]+028000a4[ 	]+addi.w[ 	]+\$a0, \$a1, 0
+[ 	]+1c:[ 	]+029ffca4[ 	]+addi.w[ 	]+\$a0, \$a1, 2047
+[ 	]+20:[ 	]+02a004a4[ 	]+addi.w[ 	]+\$a0, \$a1, -2047
+[ 	]+24:[ 	]+02c000a4[ 	]+addi.d[ 	]+\$a0, \$a1, 0
+[ 	]+28:[ 	]+02dffca4[ 	]+addi.d[ 	]+\$a0, \$a1, 2047
+[ 	]+2c:[ 	]+02e004a4[ 	]+addi.d[ 	]+\$a0, \$a1, -2047
+[ 	]+30:[ 	]+030000a4[ 	]+lu52i.d[ 	]+\$a0, \$a1, 0
+[ 	]+34:[ 	]+031ffca4[ 	]+lu52i.d[ 	]+\$a0, \$a1, 2047
+[ 	]+38:[ 	]+032004a4[ 	]+lu52i.d[ 	]+\$a0, \$a1, -2047
+[ 	]+3c:[ 	]+034000a4[ 	]+andi[ 	]+\$a0, \$a1, 0x0
+[ 	]+40:[ 	]+035ffca4[ 	]+andi[ 	]+\$a0, \$a1, 0x7ff
+[ 	]+44:[ 	]+038000a4[ 	]+ori[ 	]+\$a0, \$a1, 0x0
+[ 	]+48:[ 	]+039ffca4[ 	]+ori[ 	]+\$a0, \$a1, 0x7ff
+[ 	]+4c:[ 	]+03c000a4[ 	]+xori[ 	]+\$a0, \$a1, 0x0
+[ 	]+50:[ 	]+03dffca4[ 	]+xori[ 	]+\$a0, \$a1, 0x7ff
+[ 	]+54:[ 	]+100000a4[ 	]+addu16i.d[ 	]+\$a0, \$a1, 0
+[ 	]+58:[ 	]+11fffca4[ 	]+addu16i.d[ 	]+\$a0, \$a1, 32767
+[ 	]+5c:[ 	]+120004a4[ 	]+addu16i.d[ 	]+\$a0, \$a1, -32767
+[ 	]+60:[ 	]+14000004[ 	]+lu12i.w[ 	]+\$a0, 0
+[ 	]+64:[ 	]+14ffffe4[ 	]+lu12i.w[ 	]+\$a0, 524287
+[ 	]+68:[ 	]+17000024[ 	]+lu32i.d[ 	]+\$a0, -524287
+[ 	]+6c:[ 	]+18000004[ 	]+pcaddi[ 	]+\$a0, 0
+[ 	]+70:[ 	]+18ffffe4[ 	]+pcaddi[ 	]+\$a0, 2097148
+[ 	]+74:[ 	]+19000004[ 	]+pcaddi[ 	]+\$a0, -2097152
+[ 	]+78:[ 	]+1a000004[ 	]+pcalau12i[ 	]+\$a0, 0
+[ 	]+7c:[ 	]+1affffe4[ 	]+pcalau12i[ 	]+\$a0, 524287
+[ 	]+80:[ 	]+1b000024[ 	]+pcalau12i[ 	]+\$a0, -524287
+[ 	]+84:[ 	]+1c000004[ 	]+pcaddu12i[ 	]+\$a0, 0
+[ 	]+88:[ 	]+1cffffe4[ 	]+pcaddu12i[ 	]+\$a0, 524287
+[ 	]+8c:[ 	]+1d000024[ 	]+pcaddu12i[ 	]+\$a0, -524287
+[ 	]+90:[ 	]+1e000004[ 	]+pcaddu18i[ 	]+\$a0, 0
+[ 	]+94:[ 	]+1effffe4[ 	]+pcaddu18i[ 	]+\$a0, 524287
+[ 	]+98:[ 	]+1f000024[ 	]+pcaddu18i[ 	]+\$a0, -524287
diff --git a/gas/testsuite/gas/loongarch/imm_op.s b/gas/testsuite/gas/loongarch/imm_op.s
index 7e1c5518cba..329486cad55 100644
--- a/gas/testsuite/gas/loongarch/imm_op.s
+++ b/gas/testsuite/gas/loongarch/imm_op.s
@@ -26,8 +26,8 @@ lu12i.w  $r4,0
 lu12i.w  $r4,0x7ffff
 lu32i.d  $r4,-0x7ffff
 pcaddi  $r4,0
-pcaddi  $r4,0x7ffff
-pcaddi  $r4,-0x7ffff
+pcaddi  $r4,0x1ffffc
+pcaddi  $r4,-0x200000
 pcalau12i  $r4,0
 pcalau12i  $r4,0x7ffff
 pcalau12i  $r4,-0x7ffff
diff --git a/gas/testsuite/gas/loongarch/pcaddi.d b/gas/testsuite/gas/loongarch/pcaddi.d
new file mode 100644
index 00000000000..06dcee1a60c
--- /dev/null
+++ b/gas/testsuite/gas/loongarch/pcaddi.d
@@ -0,0 +1,13 @@
+#as:
+#objdump: -dr
+
+.*:[    ]+file format .*
+
+
+Disassembly of section .text:
+
+[ 	]*0000000000000000 <.L1>:
+[ 	]+0:[ 	]+1800000c[ 	]+pcaddi[ 	]+\$t0, 0
+[ 	]+0: R_LARCH_PCREL20_S2[ 	]+.L1
+[ 	]+4:[ 	]+1800000c[ 	]+pcaddi[ 	]+\$t0, 0
+[ 	]+4: R_LARCH_PCREL20_S2[ 	]+.L2
diff --git a/gas/testsuite/gas/loongarch/pcaddi.s b/gas/testsuite/gas/loongarch/pcaddi.s
new file mode 100644
index 00000000000..46b56306ce4
--- /dev/null
+++ b/gas/testsuite/gas/loongarch/pcaddi.s
@@ -0,0 +1,4 @@
+.L1:
+  pcaddi $t0, .L1
+  pcaddi $t0, .L2
+.L2:
diff --git a/opcodes/loongarch-opc.c b/opcodes/loongarch-opc.c
index 7d110683e93..2bb7b069ccf 100644
--- a/opcodes/loongarch-opc.c
+++ b/opcodes/loongarch-opc.c
@@ -564,6 +564,7 @@ static struct loongarch_opcode loongarch_imm_opcodes[] =
   { 0x10000000, 0xfc000000,	"addu16i.d",	"r0:5,r5:5,s10:16",		0,			0,	0,	0 },
   { 0x14000000, 0xfe000000,	"lu12i.w",	"r0:5,s5:20",			0,			0,	0,	0 },
   { 0x16000000, 0xfe000000,	"lu32i.d",	"r0:5,s5:20",			0,			0,	0,	0 },
+  { 0x0,	0x0,		"pcaddi",	"r,la",				"pcaddi %1, %%pcrel_20(%2)",	0,	0,	0 },
   { 0x18000000, 0xfe000000,	"pcaddi",	"r0:5,s5:20<<2",		0,			0,	0,	0 },
   { 0x1a000000, 0xfe000000,	"pcalau12i",	"r0:5,s5:20",			0,			0,	0,	0 },
   { 0x1c000000, 0xfe000000,	"pcaddu12i",	"r0:5,s5:20",			0,			0,	0,	0 },
-- 
2.36.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 1/2] LoongArch: Fix pcaddi format string
  2023-08-09  1:39 ` [PATCH v1 1/2] LoongArch: Fix pcaddi format string mengqinggang
@ 2023-08-09 11:37   ` Xi Ruoyao
  2023-08-09 11:48     ` Xi Ruoyao
  2023-08-11  3:08     ` WANG Xuerui
  0 siblings, 2 replies; 10+ messages in thread
From: Xi Ruoyao @ 2023-08-09 11:37 UTC (permalink / raw)
  To: mengqinggang, binutils
  Cc: xuchenghua, chenglulu, liuzhensong, i.swmail, maskray

On Wed, 2023-08-09 at 09:39 +0800, mengqinggang wrote:
> Add "<<2" for pcaddi format string
> ---
>  opcodes/loongarch-opc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/opcodes/loongarch-opc.c b/opcodes/loongarch-opc.c
> index 2f02e33dbec..7d110683e93 100644
> --- a/opcodes/loongarch-opc.c
> +++ b/opcodes/loongarch-opc.c
> @@ -564,7 +564,7 @@ static struct loongarch_opcode loongarch_imm_opcodes[] =
>    { 0x10000000, 0xfc000000,    "addu16i.d",    "r0:5,r5:5,s10:16",             0,                      0,      0,      0 },
>    { 0x14000000, 0xfe000000,    "lu12i.w",      "r0:5,s5:20",                   0,                      0,      0,      0 },
>    { 0x16000000, 0xfe000000,    "lu32i.d",      "r0:5,s5:20",                   0,                      0,      0,      0 },
> -  { 0x18000000, 0xfe000000,    "pcaddi",       "r0:5,s5:20",                   0,                      0,      0,      0 },
> +  { 0x18000000, 0xfe000000,    "pcaddi",       "r0:5,s5:20<<2",                0,                      0,      0,      0 },

The Linux kernel already uses things like "pcaddi t0, 4".  To me this
change will break them completely, and fixing it on the kernel side will
be difficult (we'll need to create some nasty gas version check).

So I don't think we should make such a backward incompatible change
without a very compelling reason.  You may argue that "<<2" has a better
readability, but if we really need the readability we can write

pcaddi t0, 4 # << 2

in the code anyway.

>    { 0x1a000000, 0xfe000000,    "pcalau12i",    "r0:5,s5:20",                   0,                      0,      0,      0 },
>    { 0x1c000000, 0xfe000000,    "pcaddu12i",    "r0:5,s5:20",                   0,                      0,      0,      0 },
>    { 0x1e000000, 0xfe000000,    "pcaddu18i",    "r0:5,s5:20",                   0,                      0,      0,      0 },

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 1/2] LoongArch: Fix pcaddi format string
  2023-08-09 11:37   ` Xi Ruoyao
@ 2023-08-09 11:48     ` Xi Ruoyao
  2023-08-11  3:08     ` WANG Xuerui
  1 sibling, 0 replies; 10+ messages in thread
From: Xi Ruoyao @ 2023-08-09 11:48 UTC (permalink / raw)
  To: mengqinggang, binutils
  Cc: xuchenghua, chenglulu, liuzhensong, i.swmail, maskray,
	Tiezhu Yang, Huacai Chen

On Wed, 2023-08-09 at 19:37 +0800, Xi Ruoyao wrote:

> > -  { 0x18000000, 0xfe000000,    "pcaddi",       "r0:5,s5:20",                   0,                      0,      0,      0 },
> > +  { 0x18000000, 0xfe000000,    "pcaddi",       "r0:5,s5:20<<2",                0,                      0,      0,      0 },
> 
> The Linux kernel already uses things like "pcaddi t0, 4".  To me this
> change will break them completely, and fixing it on the kernel side will
> be difficult (we'll need to create some nasty gas version check).
> 
> So I don't think we should make such a backward incompatible change
> without a very compelling reason.  You may argue that "<<2" has a better
> readability, but if we really need the readability we can write
> 
> pcaddi t0, 4 # << 2
> 
> in the code anyway.

CC some kernel developers.

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 1/2] LoongArch: Fix pcaddi format string
  2023-08-09 11:37   ` Xi Ruoyao
  2023-08-09 11:48     ` Xi Ruoyao
@ 2023-08-11  3:08     ` WANG Xuerui
  2023-08-15 10:00       ` mengqinggang
  1 sibling, 1 reply; 10+ messages in thread
From: WANG Xuerui @ 2023-08-11  3:08 UTC (permalink / raw)
  To: Xi Ruoyao, mengqinggang, binutils
  Cc: xuchenghua, chenglulu, liuzhensong, i.swmail, maskray

On 2023/8/9 19:37, Xi Ruoyao wrote:
> On Wed, 2023-08-09 at 09:39 +0800, mengqinggang wrote:
>> Add "<<2" for pcaddi format string
>> ---
>>   opcodes/loongarch-opc.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/opcodes/loongarch-opc.c b/opcodes/loongarch-opc.c
>> index 2f02e33dbec..7d110683e93 100644
>> --- a/opcodes/loongarch-opc.c
>> +++ b/opcodes/loongarch-opc.c
>> @@ -564,7 +564,7 @@ static struct loongarch_opcode loongarch_imm_opcodes[] =
>>     { 0x10000000, 0xfc000000,    "addu16i.d",    "r0:5,r5:5,s10:16",             0,                      0,      0,      0 },
>>     { 0x14000000, 0xfe000000,    "lu12i.w",      "r0:5,s5:20",                   0,                      0,      0,      0 },
>>     { 0x16000000, 0xfe000000,    "lu32i.d",      "r0:5,s5:20",                   0,                      0,      0,      0 },
>> -  { 0x18000000, 0xfe000000,    "pcaddi",       "r0:5,s5:20",                   0,                      0,      0,      0 },
>> +  { 0x18000000, 0xfe000000,    "pcaddi",       "r0:5,s5:20<<2",                0,                      0,      0,      0 },
> 
> The Linux kernel already uses things like "pcaddi t0, 4".  To me this
> change will break them completely, and fixing it on the kernel side will
> be difficult (we'll need to create some nasty gas version check).

Thanks for the heads-up, I initially was inclined to accept this patch 
but was hindered by real life, then saw this. The asymmetry is 
unfortunately real and here to stay, because in this case code would be 
broken without any build-time errors or warnings, and such breakage 
would go unnoticed until the moment the wrong instruction gets executed.

> So I don't think we should make such a backward incompatible change
> without a very compelling reason.  You may argue that "<<2" has a better
> readability, but if we really need the readability we can write
> 
> pcaddi t0, 4 # << 2
> 
> in the code anyway.
The immediate operand means "delta in # of instructions":

 > pcaddi t0, 4  # insns

Which is *arguably more* intuitive than the new semantics implied by 
"<<2", since IMO it's more natural to think in terms of instruction 
words when we talk about PC-relative tricks with instruction fetch in mind.

IMO a better way forward could be to document this wart in an upcoming 
revision of the LoongArch ISA manual. (It already contains assembler 
syntax tips for insns like alsl.* so another similar addition should be 
appropriate.) It's not sure at this moment whether an overhaul of 
LoongArch assembler is going to happen, so we have little choice if we 
want to keep downstream fuss to a minimum.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 1/2] LoongArch: Fix pcaddi format string
  2023-08-11  3:08     ` WANG Xuerui
@ 2023-08-15 10:00       ` mengqinggang
  2023-08-15 10:44         ` Xi Ruoyao
  0 siblings, 1 reply; 10+ messages in thread
From: mengqinggang @ 2023-08-15 10:00 UTC (permalink / raw)
  To: WANG Xuerui, Xi Ruoyao, binutils
  Cc: xuchenghua, chenglulu, liuzhensong, maskray, chenhuacai, yangtiezhu

Maybe we can just add support for "pcaddi rd, label" this time.
And the hard coded immediate in pcaddi can be changed to label.
Add "<<2" to format string can be done after several versions.


在 2023/8/11 上午11:08, WANG Xuerui 写道:
> On 2023/8/9 19:37, Xi Ruoyao wrote:
>> On Wed, 2023-08-09 at 09:39 +0800, mengqinggang wrote:
>>> Add "<<2" for pcaddi format string
>>> ---
>>>   opcodes/loongarch-opc.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/opcodes/loongarch-opc.c b/opcodes/loongarch-opc.c
>>> index 2f02e33dbec..7d110683e93 100644
>>> --- a/opcodes/loongarch-opc.c
>>> +++ b/opcodes/loongarch-opc.c
>>> @@ -564,7 +564,7 @@ static struct loongarch_opcode 
>>> loongarch_imm_opcodes[] =
>>>     { 0x10000000, 
>>> 0xfc000000,    "addu16i.d",    "r0:5,r5:5,s10:16",             0,                      0,      0,      0 
>>> },
>>>     { 0x14000000, 
>>> 0xfe000000,    "lu12i.w",      "r0:5,s5:20",                   0,                      0,      0,      0 
>>> },
>>>     { 0x16000000, 
>>> 0xfe000000,    "lu32i.d",      "r0:5,s5:20",                   0,                      0,      0,      0 
>>> },
>>> -  { 0x18000000, 
>>> 0xfe000000,    "pcaddi",       "r0:5,s5:20",                   0,                      0,      0,      0 
>>> },
>>> +  { 0x18000000, 
>>> 0xfe000000,    "pcaddi",       "r0:5,s5:20<<2",                0,                      0,      0,      0 
>>> },
>>
>> The Linux kernel already uses things like "pcaddi t0, 4".  To me this
>> change will break them completely, and fixing it on the kernel side will
>> be difficult (we'll need to create some nasty gas version check).
>
> Thanks for the heads-up, I initially was inclined to accept this patch 
> but was hindered by real life, then saw this. The asymmetry is 
> unfortunately real and here to stay, because in this case code would 
> be broken without any build-time errors or warnings, and such breakage 
> would go unnoticed until the moment the wrong instruction gets executed.
>
>> So I don't think we should make such a backward incompatible change
>> without a very compelling reason.  You may argue that "<<2" has a better
>> readability, but if we really need the readability we can write
>>
>> pcaddi t0, 4 # << 2
>>
>> in the code anyway.
> The immediate operand means "delta in # of instructions":
>
> > pcaddi t0, 4  # insns
>
> Which is *arguably more* intuitive than the new semantics implied by 
> "<<2", since IMO it's more natural to think in terms of instruction 
> words when we talk about PC-relative tricks with instruction fetch in 
> mind.
>
> IMO a better way forward could be to document this wart in an upcoming 
> revision of the LoongArch ISA manual. (It already contains assembler 
> syntax tips for insns like alsl.* so another similar addition should 
> be appropriate.) It's not sure at this moment whether an overhaul of 
> LoongArch assembler is going to happen, so we have little choice if we 
> want to keep downstream fuss to a minimum.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 1/2] LoongArch: Fix pcaddi format string
  2023-08-15 10:00       ` mengqinggang
@ 2023-08-15 10:44         ` Xi Ruoyao
  2023-08-15 11:35           ` mengqinggang
  0 siblings, 1 reply; 10+ messages in thread
From: Xi Ruoyao @ 2023-08-15 10:44 UTC (permalink / raw)
  To: mengqinggang, WANG Xuerui, binutils
  Cc: xuchenghua, chenglulu, liuzhensong, maskray, chenhuacai, yangtiezhu

On Tue, 2023-08-15 at 18:00 +0800, mengqinggang wrote:
> Maybe we can just add support for "pcaddi rd, label" this time.
> And the hard coded immediate in pcaddi can be changed to label.
> Add "<<2" to format string can be done after several versions.

It does not fix the issue.  The kernel still needs nasty binutils
version checks unless it removes the support for building with binutils
< (the first version supports using the label in pcaddi).

I think we should just *never* change the semantics of an assemble
grammar construct.  I don't think "pcaddi rd, 16" is really better than
"pcaddi rd, 4" (unless we'll add some 2-byte or 1-byte instructions like
RVC, but then we'll need a new instruction and we should not reuse the
pcaddi mnemonic anyway).

If you really need "16" instead of "4" you can add it as a pseudo
instruction with a new name.  The pseudo instruction can even support
misaligned values like 14 (expanding it to pcaddi rd, 3 and addi rd, rd,
2).  It can be named "la.pcaddi" or something (following "la.pcrel"
etc).

> 在 2023/8/11 上午11:08, WANG Xuerui 写道:
> > On 2023/8/9 19:37, Xi Ruoyao wrote:
> > > On Wed, 2023-08-09 at 09:39 +0800, mengqinggang wrote:
> > > > Add "<<2" for pcaddi format string
> > > > ---
> > > >   opcodes/loongarch-opc.c | 2 +-
> > > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/opcodes/loongarch-opc.c b/opcodes/loongarch-opc.c
> > > > index 2f02e33dbec..7d110683e93 100644
> > > > --- a/opcodes/loongarch-opc.c
> > > > +++ b/opcodes/loongarch-opc.c
> > > > @@ -564,7 +564,7 @@ static struct loongarch_opcode 
> > > > loongarch_imm_opcodes[] =
> > > >     { 0x10000000, 
> > > > 0xfc000000,    "addu16i.d",    "r0:5,r5:5,s10:16",             0,                      0,      0,      0 
> > > > },
> > > >     { 0x14000000, 
> > > > 0xfe000000,    "lu12i.w",      "r0:5,s5:20",                   0,                      0,      0,      0 
> > > > },
> > > >     { 0x16000000, 
> > > > 0xfe000000,    "lu32i.d",      "r0:5,s5:20",                   0,                      0,      0,      0 
> > > > },
> > > > -  { 0x18000000, 
> > > > 0xfe000000,    "pcaddi",       "r0:5,s5:20",                   0,                      0,      0,      0 
> > > > },
> > > > +  { 0x18000000, 
> > > > 0xfe000000,    "pcaddi",       "r0:5,s5:20<<2",                0,                      0,      0,      0 
> > > > },
> > > 
> > > The Linux kernel already uses things like "pcaddi t0, 4".  To me this
> > > change will break them completely, and fixing it on the kernel side will
> > > be difficult (we'll need to create some nasty gas version check).
> > 
> > Thanks for the heads-up, I initially was inclined to accept this patch 
> > but was hindered by real life, then saw this. The asymmetry is 
> > unfortunately real and here to stay, because in this case code would
> > be broken without any build-time errors or warnings, and such breakage 
> > would go unnoticed until the moment the wrong instruction gets executed.
> > 
> > > So I don't think we should make such a backward incompatible change
> > > without a very compelling reason.  You may argue that "<<2" has a better
> > > readability, but if we really need the readability we can write
> > > 
> > > pcaddi t0, 4 # << 2
> > > 
> > > in the code anyway.
> > The immediate operand means "delta in # of instructions":
> > 
> > > pcaddi t0, 4  # insns
> > 
> > Which is *arguably more* intuitive than the new semantics implied by
> > "<<2", since IMO it's more natural to think in terms of instruction 
> > words when we talk about PC-relative tricks with instruction fetch in 
> > mind.
> > 
> > IMO a better way forward could be to document this wart in an upcoming 
> > revision of the LoongArch ISA manual. (It already contains assembler
> > syntax tips for insns like alsl.* so another similar addition should
> > be appropriate.) It's not sure at this moment whether an overhaul of
> > LoongArch assembler is going to happen, so we have little choice if we 
> > want to keep downstream fuss to a minimum.
> 

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 1/2] LoongArch: Fix pcaddi format string
  2023-08-15 10:44         ` Xi Ruoyao
@ 2023-08-15 11:35           ` mengqinggang
  2023-08-15 15:03             ` Xi Ruoyao
  0 siblings, 1 reply; 10+ messages in thread
From: mengqinggang @ 2023-08-15 11:35 UTC (permalink / raw)
  To: Xi Ruoyao, WANG Xuerui, binutils
  Cc: xuchenghua, chenglulu, liuzhensong, maskray, chenhuacai, yangtiezhu

The main reason for adding "<<2"  is that without "<<2" one same immediate
for pcaddi and other 4-byts aligned instructions has a different meaning.
The offset of "pcaddi $t0, 4" and "bl 4" are 16 and 4, this difference may
cause confusion for users.


在 2023/8/15 下午6:44, Xi Ruoyao 写道:
> On Tue, 2023-08-15 at 18:00 +0800, mengqinggang wrote:
>> Maybe we can just add support for "pcaddi rd, label" this time.
>> And the hard coded immediate in pcaddi can be changed to label.
>> Add "<<2" to format string can be done after several versions.
> It does not fix the issue.  The kernel still needs nasty binutils
> version checks unless it removes the support for building with binutils
> < (the first version supports using the label in pcaddi).
>
> I think we should just *never* change the semantics of an assemble
> grammar construct.  I don't think "pcaddi rd, 16" is really better than
> "pcaddi rd, 4" (unless we'll add some 2-byte or 1-byte instructions like
> RVC, but then we'll need a new instruction and we should not reuse the
> pcaddi mnemonic anyway).
>
> If you really need "16" instead of "4" you can add it as a pseudo
> instruction with a new name.  The pseudo instruction can even support
> misaligned values like 14 (expanding it to pcaddi rd, 3 and addi rd, rd,
> 2).  It can be named "la.pcaddi" or something (following "la.pcrel"
> etc).
>
>> 在 2023/8/11 上午11:08, WANG Xuerui 写道:
>>> On 2023/8/9 19:37, Xi Ruoyao wrote:
>>>> On Wed, 2023-08-09 at 09:39 +0800, mengqinggang wrote:
>>>>> Add "<<2" for pcaddi format string
>>>>> ---
>>>>>    opcodes/loongarch-opc.c | 2 +-
>>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/opcodes/loongarch-opc.c b/opcodes/loongarch-opc.c
>>>>> index 2f02e33dbec..7d110683e93 100644
>>>>> --- a/opcodes/loongarch-opc.c
>>>>> +++ b/opcodes/loongarch-opc.c
>>>>> @@ -564,7 +564,7 @@ static struct loongarch_opcode
>>>>> loongarch_imm_opcodes[] =
>>>>>      { 0x10000000,
>>>>> 0xfc000000,    "addu16i.d",    "r0:5,r5:5,s10:16",             0,                      0,      0,      0
>>>>> },
>>>>>      { 0x14000000,
>>>>> 0xfe000000,    "lu12i.w",      "r0:5,s5:20",                   0,                      0,      0,      0
>>>>> },
>>>>>      { 0x16000000,
>>>>> 0xfe000000,    "lu32i.d",      "r0:5,s5:20",                   0,                      0,      0,      0
>>>>> },
>>>>> -  { 0x18000000,
>>>>> 0xfe000000,    "pcaddi",       "r0:5,s5:20",                   0,                      0,      0,      0
>>>>> },
>>>>> +  { 0x18000000,
>>>>> 0xfe000000,    "pcaddi",       "r0:5,s5:20<<2",                0,                      0,      0,      0
>>>>> },
>>>> The Linux kernel already uses things like "pcaddi t0, 4".  To me this
>>>> change will break them completely, and fixing it on the kernel side will
>>>> be difficult (we'll need to create some nasty gas version check).
>>> Thanks for the heads-up, I initially was inclined to accept this patch
>>> but was hindered by real life, then saw this. The asymmetry is
>>> unfortunately real and here to stay, because in this case code would
>>> be broken without any build-time errors or warnings, and such breakage
>>> would go unnoticed until the moment the wrong instruction gets executed.
>>>
>>>> So I don't think we should make such a backward incompatible change
>>>> without a very compelling reason.  You may argue that "<<2" has a better
>>>> readability, but if we really need the readability we can write
>>>>
>>>> pcaddi t0, 4 # << 2
>>>>
>>>> in the code anyway.
>>> The immediate operand means "delta in # of instructions":
>>>
>>>> pcaddi t0, 4  # insns
>>> Which is *arguably more* intuitive than the new semantics implied by
>>> "<<2", since IMO it's more natural to think in terms of instruction
>>> words when we talk about PC-relative tricks with instruction fetch in
>>> mind.
>>>
>>> IMO a better way forward could be to document this wart in an upcoming
>>> revision of the LoongArch ISA manual. (It already contains assembler
>>> syntax tips for insns like alsl.* so another similar addition should
>>> be appropriate.) It's not sure at this moment whether an overhaul of
>>> LoongArch assembler is going to happen, so we have little choice if we
>>> want to keep downstream fuss to a minimum.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 1/2] LoongArch: Fix pcaddi format string
  2023-08-15 11:35           ` mengqinggang
@ 2023-08-15 15:03             ` Xi Ruoyao
  0 siblings, 0 replies; 10+ messages in thread
From: Xi Ruoyao @ 2023-08-15 15:03 UTC (permalink / raw)
  To: mengqinggang, WANG Xuerui, binutils
  Cc: xuchenghua, chenglulu, liuzhensong, maskray, chenhuacai, yangtiezhu

On Tue, 2023-08-15 at 19:35 +0800, mengqinggang wrote:
> The main reason for adding "<<2"  is that without "<<2" one same immediate
> for pcaddi and other 4-byts aligned instructions has a different meaning.
> The offset of "pcaddi $t0, 4" and "bl 4" are 16 and 4, this difference may
> cause confusion for users.

Users writing some new code can always adapt, but old code won't change
itself miraculously.  In software development the stability of interface
is much more important than consistency.  Most API existing in the world
are not consistent, but they just exist as they are.

If we must do this, I think we can add an option (maybe named "-mpcaddi-
shift=0/2") to control the behavior and emit a warning like

"warning: use pcaddi and an immediate input w/o -mpcaddi-shift= setting
is deprecated; use a label or specifying -mpcaddi-shift=0/2 explicitly"

In 2.42 we can keep -mpcaddi-shift=2 (old behavior) as the default, and
in 2.43 we can turn the warning into an error (i. e. if you want to code
pcaddi with an immediate instead of a label, you must specify -mpcaddi-
shift=0 or 2).  This will make the "old" code fail to assemble
immediately, instead of producing a subtle mine blowing up at runtime.

For kernel the fix will also be simpler: KBUILD_AFLAGS += $(call cc-
option,-Wa$(comma)-mpcaddi-shift=2).

> 在 2023/8/15 下午6:44, Xi Ruoyao 写道:
> > On Tue, 2023-08-15 at 18:00 +0800, mengqinggang wrote:
> > > Maybe we can just add support for "pcaddi rd, label" this time.
> > > And the hard coded immediate in pcaddi can be changed to label.
> > > Add "<<2" to format string can be done after several versions.
> > It does not fix the issue.  The kernel still needs nasty binutils
> > version checks unless it removes the support for building with
> > binutils
> > < (the first version supports using the label in pcaddi).
> > 
> > I think we should just *never* change the semantics of an assemble
> > grammar construct.  I don't think "pcaddi rd, 16" is really better
> > than
> > "pcaddi rd, 4" (unless we'll add some 2-byte or 1-byte instructions
> > like
> > RVC, but then we'll need a new instruction and we should not reuse
> > the
> > pcaddi mnemonic anyway).
> > 
> > If you really need "16" instead of "4" you can add it as a pseudo
> > instruction with a new name.  The pseudo instruction can even
> > support
> > misaligned values like 14 (expanding it to pcaddi rd, 3 and addi rd,
> > rd,
> > 2).  It can be named "la.pcaddi" or something (following "la.pcrel"
> > etc).
> > 
> > > 在 2023/8/11 上午11:08, WANG Xuerui 写道:
> > > > On 2023/8/9 19:37, Xi Ruoyao wrote:
> > > > > On Wed, 2023-08-09 at 09:39 +0800, mengqinggang wrote:
> > > > > > Add "<<2" for pcaddi format string
> > > > > > ---
> > > > > >    opcodes/loongarch-opc.c | 2 +-
> > > > > >    1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > 
> > > > > > diff --git a/opcodes/loongarch-opc.c b/opcodes/loongarch-
> > > > > > opc.c
> > > > > > index 2f02e33dbec..7d110683e93 100644
> > > > > > --- a/opcodes/loongarch-opc.c
> > > > > > +++ b/opcodes/loongarch-opc.c
> > > > > > @@ -564,7 +564,7 @@ static struct loongarch_opcode
> > > > > > loongarch_imm_opcodes[] =
> > > > > >      { 0x10000000,
> > > > > > 0xfc000000,    "addu16i.d",    "r0:5,r5:5,s10:16",          
> > > > > >    0,                      0,      0,      0
> > > > > > },
> > > > > >      { 0x14000000,
> > > > > > 0xfe000000,    "lu12i.w",      "r0:5,s5:20",                
> > > > > >    0,                      0,      0,      0
> > > > > > },
> > > > > >      { 0x16000000,
> > > > > > 0xfe000000,    "lu32i.d",      "r0:5,s5:20",                
> > > > > >    0,                      0,      0,      0
> > > > > > },
> > > > > > -  { 0x18000000,
> > > > > > 0xfe000000,    "pcaddi",       "r0:5,s5:20",                
> > > > > >    0,                      0,      0,      0
> > > > > > },
> > > > > > +  { 0x18000000,
> > > > > > 0xfe000000,    "pcaddi",       "r0:5,s5:20<<2",             
> > > > > >    0,                      0,      0,      0
> > > > > > },
> > > > > The Linux kernel already uses things like "pcaddi t0, 4".  To
> > > > > me this
> > > > > change will break them completely, and fixing it on the kernel
> > > > > side will
> > > > > be difficult (we'll need to create some nasty gas version
> > > > > check).
> > > > Thanks for the heads-up, I initially was inclined to accept this
> > > > patch
> > > > but was hindered by real life, then saw this. The asymmetry is
> > > > unfortunately real and here to stay, because in this case code
> > > > would
> > > > be broken without any build-time errors or warnings, and such
> > > > breakage
> > > > would go unnoticed until the moment the wrong instruction gets
> > > > executed.
> > > > 
> > > > > So I don't think we should make such a backward incompatible
> > > > > change
> > > > > without a very compelling reason.  You may argue that "<<2"
> > > > > has a better
> > > > > readability, but if we really need the readability we can
> > > > > write
> > > > > 
> > > > > pcaddi t0, 4 # << 2
> > > > > 
> > > > > in the code anyway.
> > > > The immediate operand means "delta in # of instructions":
> > > > 
> > > > > pcaddi t0, 4  # insns
> > > > Which is *arguably more* intuitive than the new semantics
> > > > implied by
> > > > "<<2", since IMO it's more natural to think in terms of
> > > > instruction
> > > > words when we talk about PC-relative tricks with instruction
> > > > fetch in
> > > > mind.
> > > > 
> > > > IMO a better way forward could be to document this wart in an
> > > > upcoming
> > > > revision of the LoongArch ISA manual. (It already contains
> > > > assembler
> > > > syntax tips for insns like alsl.* so another similar addition
> > > > should
> > > > be appropriate.) It's not sure at this moment whether an
> > > > overhaul of
> > > > LoongArch assembler is going to happen, so we have little choice
> > > > if we
> > > > want to keep downstream fuss to a minimum.
> 

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-08-15 15:04 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-09  1:39 [PATCH v1 0/2] Add support for "pcaddi rd, label" mengqinggang
2023-08-09  1:39 ` [PATCH v1 1/2] LoongArch: Fix pcaddi format string mengqinggang
2023-08-09 11:37   ` Xi Ruoyao
2023-08-09 11:48     ` Xi Ruoyao
2023-08-11  3:08     ` WANG Xuerui
2023-08-15 10:00       ` mengqinggang
2023-08-15 10:44         ` Xi Ruoyao
2023-08-15 11:35           ` mengqinggang
2023-08-15 15:03             ` Xi Ruoyao
2023-08-09  1:39 ` [PATCH v1 2/2] LoongArch: Add support for "pcaddi rd, label" mengqinggang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).