diff mbox series

[1/4] target/arm: Fixup special cross page case for sve continuous load/store

Message ID 20201207044655.2312-2-zhiwei_liu@c-sky.com (mailing list archive)
State New, archived
Headers show
Series target/arm bug fix | expand

Commit Message

LIU Zhiwei Dec. 7, 2020, 4:46 a.m. UTC
If the split element is also the first active element of the vector,
mem_off_first[0] should equal to mem_off_split.

Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
---
 target/arm/sve_helper.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Richard Henderson Dec. 8, 2020, 8:13 p.m. UTC | #1
On 12/6/20 10:46 PM, LIU Zhiwei wrote:
> If the split element is also the first active element of the vector,
> mem_off_first[0] should equal to mem_off_split.
> 
> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
> ---
>  target/arm/sve_helper.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
> index 5f037c3a8f..91d1d24725 100644
> --- a/target/arm/sve_helper.c
> +++ b/target/arm/sve_helper.c
> @@ -4282,9 +4282,10 @@ static bool sve_cont_ldst_pages(SVEContLdSt *info, SVEContFault fault,
>           * to generate faults for the second page.  For no-fault,
>           * we have work only if the second page is valid.
>           */
> -        if (info->mem_off_first[0] < info->mem_off_split) {
> -            nofault = FAULT_FIRST;
> -            have_work = false;
> +        if (info->mem_off_first[0] == info->mem_off_split) {
> +            if (nofault) {
> +                have_work = false;
> +            }

There are two separate bugs here.  The first is as you describe, and applies
only to the first line.

The second is the assignment of an enumerator to a boolean, and the incorrect
unconditional clearing of have_work.  The change could more consisely be

-  nofault = FAULT_FIRST;
-  have_work = false;
+  /* For nofault, we only have work if the second page is valid. */
+  have_work = !nofault;

We can either split the two changes, or improve the patch comment.


r~
diff mbox series

Patch

diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
index 5f037c3a8f..91d1d24725 100644
--- a/target/arm/sve_helper.c
+++ b/target/arm/sve_helper.c
@@ -4282,9 +4282,10 @@  static bool sve_cont_ldst_pages(SVEContLdSt *info, SVEContFault fault,
          * to generate faults for the second page.  For no-fault,
          * we have work only if the second page is valid.
          */
-        if (info->mem_off_first[0] < info->mem_off_split) {
-            nofault = FAULT_FIRST;
-            have_work = false;
+        if (info->mem_off_first[0] == info->mem_off_split) {
+            if (nofault) {
+                have_work = false;
+            }
         }
     } else {
         /*