[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/4] target/arm: Fixup special cross page case for sve contin
From: |
Richard Henderson |
Subject: |
Re: [PATCH 1/4] target/arm: Fixup special cross page case for sve continuous load/store |
Date: |
Tue, 8 Dec 2020 14:13:59 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 |
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~