[PATCH 0/4] target/ppc: Use probe_access

Richard Henderson posted 4 patches 5 years, 9 months ago
Test docker-quick@centos7 passed
Test FreeBSD passed
Test docker-mingw@fedora passed
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200129235040.24022-1-richard.henderson@linaro.org
Maintainers: David Gibson <david@gibson.dropbear.id.au>
target/ppc/mem_helper.c | 197 +++++++++++++++++++++++++++++++++-------
1 file changed, 162 insertions(+), 35 deletions(-)
[PATCH 0/4] target/ppc: Use probe_access
Posted by Richard Henderson 5 years, 9 months ago
The first two address the performance regression noticed
by Howard Spoelstra.  The last two are just something I
noticed at the same time.


r~


Richard Henderson (4):
  target/ppc: Use probe_access for LSW, STSW
  target/ppc: Use probe_access for LMW, STMW
  target/ppc: Remove redundant mask in DCBZ
  target/ppc: Use probe_write for DCBZ

 target/ppc/mem_helper.c | 197 +++++++++++++++++++++++++++++++++-------
 1 file changed, 162 insertions(+), 35 deletions(-)

-- 
2.20.1


Re: [PATCH 0/4] target/ppc: Use probe_access
Posted by Aleksandar Markovic 5 years, 9 months ago
00:51 Čet, 30.01.2020. Richard Henderson <richard.henderson@linaro.org> је
написао/ла:
>
> The first two address the performance regression noticed
> by Howard Spoelstra.  The last two are just something I
> noticed at the same time.
>

But, performance regression, according to Howard bisect analysis, happened
because of the change in target-independant code, and the fix presented
here is in target-specific code. This defies basic logic and deserves clear
and detailed explanation.

My additional concern, of course, is: Are other targets exposed to
performance degradation, and why?

Thanks,
Aleksandar

>
> r~
>
>
> Richard Henderson (4):
>   target/ppc: Use probe_access for LSW, STSW
>   target/ppc: Use probe_access for LMW, STMW
>   target/ppc: Remove redundant mask in DCBZ
>   target/ppc: Use probe_write for DCBZ
>
>  target/ppc/mem_helper.c | 197 +++++++++++++++++++++++++++++++++-------
>  1 file changed, 162 insertions(+), 35 deletions(-)
>
> --
> 2.20.1
>
>
Re: [PATCH 0/4] target/ppc: Use probe_access
Posted by Richard Henderson 5 years, 9 months ago
On 1/29/20 5:35 PM, Aleksandar Markovic wrote:
> 00:51 Čet, 30.01.2020. Richard Henderson <richard.henderson@linaro.org
> <mailto:richard.henderson@linaro.org>> је написао/ла:
>>
>> The first two address the performance regression noticed
>> by Howard Spoelstra.  The last two are just something I
>> noticed at the same time.
>>
> 
> But, performance regression, according to Howard bisect analysis, happened
> because of the change in target-independant code, and the fix presented here is
> in target-specific code. This defies basic logic and deserves clear and
> detailed explanation.
> 
> My additional concern, of course, is: Are other targets exposed to performance
> degradation, and why?

Potentially, yes.  However:

It requires lots of loads in a loop, on a hot path.  I would not have guessed
that the ppc32 Load Multiple Word (et al) was on a hot path at all, since the
instructions are deprecated.  But that's what an ancient os gets you, I suppose.


r~

Re: [PATCH 0/4] target/ppc: Use probe_access
Posted by Aleksandar Markovic 5 years, 9 months ago
On Thu, Jan 30, 2020 at 5:09 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 1/29/20 5:35 PM, Aleksandar Markovic wrote:

> > My additional concern, of course, is: Are other targets exposed to performance
> > degradation, and why?
>
> Potentially, yes.  However:
>
> It requires lots of loads in a loop, on a hot path.  I would not have guessed
> that the ppc32 Load Multiple Word (et al) was on a hot path at all, since the
> instructions are deprecated.  But that's what an ancient os gets you, I suppose.
>

OK.

>
> r~

Re: [PATCH 0/4] target/ppc: Use probe_access
Posted by Howard Spoelstra 5 years, 9 months ago
On Thu, Jan 30, 2020 at 12:50 AM Richard Henderson <
richard.henderson@linaro.org> wrote:

> The first two address the performance regression noticed
> by Howard Spoelstra.  The last two are just something I
> noticed at the same time.
>
>
> r~
>
>
> Richard Henderson (4):
>   target/ppc: Use probe_access for LSW, STSW
>   target/ppc: Use probe_access for LMW, STMW
>   target/ppc: Remove redundant mask in DCBZ
>   target/ppc: Use probe_write for DCBZ
>
>  target/ppc/mem_helper.c | 197 +++++++++++++++++++++++++++++++++-------
>  1 file changed, 162 insertions(+), 35 deletions(-)
>
> --
> 2.20.1
>
> Hi,

I can confirm these patches fix the performance issue I reported.

Thanks,
Howard
Re: [PATCH 0/4] target/ppc: Use probe_access
Posted by David Gibson 5 years, 9 months ago
On Wed, Jan 29, 2020 at 03:50:36PM -0800, Richard Henderson wrote:
> The first two address the performance regression noticed
> by Howard Spoelstra.  The last two are just something I
> noticed at the same time.

Applied to ppc-for-5.0, thanks.

> 
> 
> r~
> 
> 
> Richard Henderson (4):
>   target/ppc: Use probe_access for LSW, STSW
>   target/ppc: Use probe_access for LMW, STMW
>   target/ppc: Remove redundant mask in DCBZ
>   target/ppc: Use probe_write for DCBZ
> 
>  target/ppc/mem_helper.c | 197 +++++++++++++++++++++++++++++++++-------
>  1 file changed, 162 insertions(+), 35 deletions(-)
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
Re: [PATCH 0/4] target/ppc: Use probe_access
Posted by Howard Spoelstra 5 years, 9 months ago
As this patch set solved the performance issue and even led to the highest
scores I ever saw on the benchmark tool I used, let me add a:

Tested-by: Howard Spoelstra <hsp.cat7@gmail.com>

On Thu, Jan 30, 2020 at 12:50 AM Richard Henderson <
richard.henderson@linaro.org> wrote:

> The first two address the performance regression noticed
> by Howard Spoelstra.  The last two are just something I
> noticed at the same time.
>
>
> r~
>
>
> Richard Henderson (4):
>   target/ppc: Use probe_access for LSW, STSW
>   target/ppc: Use probe_access for LMW, STMW
>   target/ppc: Remove redundant mask in DCBZ
>   target/ppc: Use probe_write for DCBZ
>
>  target/ppc/mem_helper.c | 197 +++++++++++++++++++++++++++++++++-------
>  1 file changed, 162 insertions(+), 35 deletions(-)
>
> --
> 2.20.1
>
>
Re: [PATCH 0/4] target/ppc: Use probe_access
Posted by Aleksandar Markovic 5 years, 9 months ago
On Thu, Jan 30, 2020 at 4:09 PM Howard Spoelstra <hsp.cat7@gmail.com> wrote:
>
> As this patch set solved the performance issue and even led to the highest scores I ever saw on the benchmark tool I used, let me add a:
>

This makes my question to Richard more important:

Are other targets exposed to performance degradation, and why?

> Tested-by: Howard Spoelstra <hsp.cat7@gmail.com>
>
> On Thu, Jan 30, 2020 at 12:50 AM Richard Henderson <richard.henderson@linaro.org> wrote:
>>
>> The first two address the performance regression noticed
>> by Howard Spoelstra.  The last two are just something I
>> noticed at the same time.
>>
>>
>> r~
>>
>>
>> Richard Henderson (4):
>>   target/ppc: Use probe_access for LSW, STSW
>>   target/ppc: Use probe_access for LMW, STMW
>>   target/ppc: Remove redundant mask in DCBZ
>>   target/ppc: Use probe_write for DCBZ
>>
>>  target/ppc/mem_helper.c | 197 +++++++++++++++++++++++++++++++++-------
>>  1 file changed, 162 insertions(+), 35 deletions(-)
>>
>> --
>> 2.20.1
>>