Re: [PATCH v2] tcg/loongarch64: Fix tcg_out_movi vs some pcrel pointers

2024-06-19 Thread gaosong 

在 2024/6/19 下午1:50, Richard Henderson 写道:

Simplify the logic for two-part, 32-bit pc-relative addresses.
Rather than assume all such fit in int32_t, do some arithmetic
and assert a result, do some arithmetic first and then check
to see if the pieces are in range.

Reported-by: Song Gao 
Signed-off-by: Richard Henderson 
---

Hi Song.  I was not thrilled by the "goto out" that you introduced in

   20240618125044.687443-1-gaos...@loongson.cn

Instead, I copied the logic from tcg/aarch64/ for adrp+add.

  

Thank you.

Cc: qemu-sta...@nongnu.org
Reviewed-by: Song Gao 

Thanks.
Song Gao

r~

---
  tcg/loongarch64/tcg-target.c.inc | 32 +++-
  1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/tcg/loongarch64/tcg-target.c.inc b/tcg/loongarch64/tcg-target.c.inc
index 7ca52d0248..e915e97bba 100644
--- a/tcg/loongarch64/tcg-target.c.inc
+++ b/tcg/loongarch64/tcg-target.c.inc
@@ -382,8 +382,7 @@ static void tcg_out_movi(TCGContext *s, TCGType type, 
TCGReg rd,
   * back to the slow path.
   */
  
-intptr_t pc_offset;

-tcg_target_long val_lo, val_hi, pc_hi, offset_hi;
+intptr_t src_rx, pc_offset;
  tcg_target_long hi12, hi32, hi52;
  
  /* Value fits in signed i32.  */

@@ -393,24 +392,23 @@ static void tcg_out_movi(TCGContext *s, TCGType type, 
TCGReg rd,
  }
  
  /* PC-relative cases.  */

-pc_offset = tcg_pcrel_diff(s, (void *)val);
-if (pc_offset == sextreg(pc_offset, 0, 22) && (pc_offset & 3) == 0) {
-/* Single pcaddu2i.  */
-tcg_out_opc_pcaddu2i(s, rd, pc_offset >> 2);
-return;
+src_rx = (intptr_t)tcg_splitwx_to_rx(s->code_ptr);
+if ((val & 3) == 0) {
+pc_offset = val - src_rx;
+if (pc_offset == sextreg(pc_offset, 0, 22)) {
+/* Single pcaddu2i.  */
+tcg_out_opc_pcaddu2i(s, rd, pc_offset >> 2);
+return;
+}
  }
  
-if (pc_offset == (int32_t)pc_offset) {

-/* Offset within 32 bits; load with pcalau12i + ori.  */
-val_lo = sextreg(val, 0, 12);
-val_hi = val >> 12;
-pc_hi = (val - pc_offset) >> 12;
-offset_hi = val_hi - pc_hi;
-
-tcg_debug_assert(offset_hi == sextreg(offset_hi, 0, 20));
-tcg_out_opc_pcalau12i(s, rd, offset_hi);
+pc_offset = (val >> 12) - (src_rx >> 12);
+if (pc_offset == sextreg(pc_offset, 0, 20)) {
+/* Load with pcalau12i + ori.  */
+tcg_target_long val_lo = val & 0xfff;
+tcg_out_opc_pcalau12i(s, rd, pc_offset);
  if (val_lo != 0) {
-tcg_out_opc_ori(s, rd, rd, val_lo & 0xfff);
+tcg_out_opc_ori(s, rd, rd, val_lo);
  }
  return;
  }

[PATCH v2] tcg/loongarch64: Fix tcg_out_movi vs some pcrel pointers

2024-06-18 Thread Richard Henderson 
Simplify the logic for two-part, 32-bit pc-relative addresses.
Rather than assume all such fit in int32_t, do some arithmetic
and assert a result, do some arithmetic first and then check
to see if the pieces are in range.

Reported-by: Song Gao 
Signed-off-by: Richard Henderson 
---

Hi Song.  I was not thrilled by the "goto out" that you introduced in

  20240618125044.687443-1-gaos...@loongson.cn

Instead, I copied the logic from tcg/aarch64/ for adrp+add.


r~

---
 tcg/loongarch64/tcg-target.c.inc | 32 +++-
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/tcg/loongarch64/tcg-target.c.inc b/tcg/loongarch64/tcg-target.c.inc
index 7ca52d0248..e915e97bba 100644
--- a/tcg/loongarch64/tcg-target.c.inc
+++ b/tcg/loongarch64/tcg-target.c.inc
@@ -382,8 +382,7 @@ static void tcg_out_movi(TCGContext *s, TCGType type, 
TCGReg rd,
  * back to the slow path.
  */
 
-intptr_t pc_offset;
-tcg_target_long val_lo, val_hi, pc_hi, offset_hi;
+intptr_t src_rx, pc_offset;
 tcg_target_long hi12, hi32, hi52;
 
 /* Value fits in signed i32.  */
@@ -393,24 +392,23 @@ static void tcg_out_movi(TCGContext *s, TCGType type, 
TCGReg rd,
 }
 
 /* PC-relative cases.  */
-pc_offset = tcg_pcrel_diff(s, (void *)val);
-if (pc_offset == sextreg(pc_offset, 0, 22) && (pc_offset & 3) == 0) {
-/* Single pcaddu2i.  */
-tcg_out_opc_pcaddu2i(s, rd, pc_offset >> 2);
-return;
+src_rx = (intptr_t)tcg_splitwx_to_rx(s->code_ptr);
+if ((val & 3) == 0) {
+pc_offset = val - src_rx;
+if (pc_offset == sextreg(pc_offset, 0, 22)) {
+/* Single pcaddu2i.  */
+tcg_out_opc_pcaddu2i(s, rd, pc_offset >> 2);
+return;
+}
 }
 
-if (pc_offset == (int32_t)pc_offset) {
-/* Offset within 32 bits; load with pcalau12i + ori.  */
-val_lo = sextreg(val, 0, 12);
-val_hi = val >> 12;
-pc_hi = (val - pc_offset) >> 12;
-offset_hi = val_hi - pc_hi;
-
-tcg_debug_assert(offset_hi == sextreg(offset_hi, 0, 20));
-tcg_out_opc_pcalau12i(s, rd, offset_hi);
+pc_offset = (val >> 12) - (src_rx >> 12);
+if (pc_offset == sextreg(pc_offset, 0, 20)) {
+/* Load with pcalau12i + ori.  */
+tcg_target_long val_lo = val & 0xfff;
+tcg_out_opc_pcalau12i(s, rd, pc_offset);
 if (val_lo != 0) {
-tcg_out_opc_ori(s, rd, rd, val_lo & 0xfff);
+tcg_out_opc_ori(s, rd, rd, val_lo);
 }
 return;
 }
-- 
2.34.1