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

Richard Henderson posted 1 patch 5 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20240619055002.119042-1-richard.henderson@linaro.org
Maintainers: WANG Xuerui <git@xen0n.name>, Richard Henderson <richard.henderson@linaro.org>
tcg/loongarch64/tcg-target.c.inc | 32 +++++++++++++++-----------------
1 file changed, 15 insertions(+), 17 deletions(-)
[PATCH v2] tcg/loongarch64: Fix tcg_out_movi vs some pcrel pointers
Posted by Richard Henderson 5 months, 1 week ago
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 <gaosong@loongson.cn>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---

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

  20240618125044.687443-1-gaosong@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
Re: [PATCH v2] tcg/loongarch64: Fix tcg_out_movi vs some pcrel pointers
Posted by gaosong 5 months, 1 week ago
在 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 <gaosong@loongson.cn>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>
> Hi Song.  I was not thrilled by the "goto out" that you introduced in
>
>    20240618125044.687443-1-gaosong@loongson.cn
>
> Instead, I copied the logic from tcg/aarch64/ for adrp+add.
>
>   
Thank you.

Cc: qemu-stable@nongnu.org
Reviewed-by: Song Gao <gaosong@loongson.cn>

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;
>       }