| Message ID | 20170621201933.GB26847@tsuri.lan |
|---|---|
| State | New |
| Headers | show |
On Wed, Jun 21, 2017 at 10:19:33PM +0200, Matthieu Bouron wrote: > On Wed, Jun 21, 2017 at 04:57:53PM -0300, James Almer wrote: > > On 6/19/2017 6:08 AM, Matthieu Bouron wrote: > > > Avoids overriding v0 (which containins the result of the tested > > > function) in checkasm_call_checked. > > > > > > Also properly calls checkasm_call_checked. > > > --- > > > tests/checkasm/aarch64/checkasm.S | 8 ++++---- > > > tests/checkasm/checkasm.h | 2 ++ > > > 2 files changed, 6 insertions(+), 4 deletions(-) > > > > > > diff --git a/tests/checkasm/aarch64/checkasm.S b/tests/checkasm/aarch64/checkasm.S > > > index 53a2a478dc..75a9a56143 100644 > > > --- a/tests/checkasm/aarch64/checkasm.S > > > +++ b/tests/checkasm/aarch64/checkasm.S > > > @@ -112,10 +112,10 @@ function checkasm_checked_call, export=1 > > > movi v3.8h, #0 > > > > > > .macro check_reg_neon reg1, reg2 > > > - ldr q0, [x9], #16 > > > - uzp1 v1.2d, v\reg1\().2d, v\reg2\().2d > > > - eor v0.16b, v0.16b, v1.16b > > > - orr v3.16b, v3.16b, v0.16b > > > + ldr q1, [x9], #16 > > > + uzp1 v2.2d, v\reg1\().2d, v\reg2\().2d > > > + eor v1.16b, v1.16b, v2.16b > > > + orr v3.16b, v3.16b, v1.16b > > > .endm > > > check_reg_neon 8, 9 > > > check_reg_neon 10, 11 > > > diff --git a/tests/checkasm/checkasm.h b/tests/checkasm/checkasm.h > > > index 695d871220..5249669fba 100644 > > > --- a/tests/checkasm/checkasm.h > > > +++ b/tests/checkasm/checkasm.h > > > @@ -145,6 +145,8 @@ void checkasm_stack_clobber(uint64_t clobber, ...); > > > void checkasm_checked_call(void *func, ...); > > > #define declare_new(ret, ...) ret (*checked_call)(void *, int, int, int, int, int, int, int, __VA_ARGS__)\ > > > = (void *)checkasm_checked_call; > > > +#define declare_new_float(ret, ...) ret (*checked_call)(void *, int, int, int, int, int, int, int, __VA_ARGS__)\ > > > + = (void *)checkasm_checked_call; > > > > Isn't this doing the same as the generic "#define declare_new_float" > > about 15 lines below? If declare_new_float() is no different than > > declare_new() for a given target, then just let the aforementioned check > > handle it. > > You are right (I missed this define), so this chunk is removed from the > patch. > > > > > > #define CLOB (UINT64_C(0xdeadbeefdeadbeef)) > > > #define call_new(...) (checkasm_stack_clobber(CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,\ > > > CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB),\ > > > > > > > Should be ok either way if tested (The only aarch64 FATE client using > > git head hasn't run in twenty days, so no way for me to check if this > > fixes the problem anyway). > > I tested on my Odroid-C2 and fate passes with this patch (otherwise it > fails on checkasm float_dsp.scalarproduct_float test). > > I've attached the updated patch. I'll push it in a few hours if there is > no objection. > > -- > Matthieu B. > From ee4bd4fdb9ac01bd331a43222e9029f820bec4e5 Mon Sep 17 00:00:00 2001 > From: Matthieu Bouron <matthieu.bouron@gmail.com> > Date: Mon, 19 Jun 2017 10:55:28 +0200 > Subject: [PATCH] checkasm/aarch64: fix tests returning a float > > Avoids overriding v0 (which containins the result of the tested > function) in checkasm_call_checked. > --- > tests/checkasm/aarch64/checkasm.S | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/tests/checkasm/aarch64/checkasm.S b/tests/checkasm/aarch64/checkasm.S > index 53a2a478dc..75a9a56143 100644 > --- a/tests/checkasm/aarch64/checkasm.S > +++ b/tests/checkasm/aarch64/checkasm.S > @@ -112,10 +112,10 @@ function checkasm_checked_call, export=1 > movi v3.8h, #0 > > .macro check_reg_neon reg1, reg2 > - ldr q0, [x9], #16 > - uzp1 v1.2d, v\reg1\().2d, v\reg2\().2d > - eor v0.16b, v0.16b, v1.16b > - orr v3.16b, v3.16b, v0.16b > + ldr q1, [x9], #16 > + uzp1 v2.2d, v\reg1\().2d, v\reg2\().2d > + eor v1.16b, v1.16b, v2.16b > + orr v3.16b, v3.16b, v1.16b > .endm > check_reg_neon 8, 9 > check_reg_neon 10, 11 > -- > 2.13.1 > Patch applied. Thanks.
On 6/22/2017 5:35 AM, Matthieu Bouron wrote: > On Wed, Jun 21, 2017 at 10:19:33PM +0200, Matthieu Bouron wrote: >> On Wed, Jun 21, 2017 at 04:57:53PM -0300, James Almer wrote: >>> On 6/19/2017 6:08 AM, Matthieu Bouron wrote: >>>> Avoids overriding v0 (which containins the result of the tested >>>> function) in checkasm_call_checked. >>>> >>>> Also properly calls checkasm_call_checked. >>>> --- >>>> tests/checkasm/aarch64/checkasm.S | 8 ++++---- >>>> tests/checkasm/checkasm.h | 2 ++ >>>> 2 files changed, 6 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/tests/checkasm/aarch64/checkasm.S b/tests/checkasm/aarch64/checkasm.S >>>> index 53a2a478dc..75a9a56143 100644 >>>> --- a/tests/checkasm/aarch64/checkasm.S >>>> +++ b/tests/checkasm/aarch64/checkasm.S >>>> @@ -112,10 +112,10 @@ function checkasm_checked_call, export=1 >>>> movi v3.8h, #0 >>>> >>>> .macro check_reg_neon reg1, reg2 >>>> - ldr q0, [x9], #16 >>>> - uzp1 v1.2d, v\reg1\().2d, v\reg2\().2d >>>> - eor v0.16b, v0.16b, v1.16b >>>> - orr v3.16b, v3.16b, v0.16b >>>> + ldr q1, [x9], #16 >>>> + uzp1 v2.2d, v\reg1\().2d, v\reg2\().2d >>>> + eor v1.16b, v1.16b, v2.16b >>>> + orr v3.16b, v3.16b, v1.16b >>>> .endm >>>> check_reg_neon 8, 9 >>>> check_reg_neon 10, 11 >>>> diff --git a/tests/checkasm/checkasm.h b/tests/checkasm/checkasm.h >>>> index 695d871220..5249669fba 100644 >>>> --- a/tests/checkasm/checkasm.h >>>> +++ b/tests/checkasm/checkasm.h >>>> @@ -145,6 +145,8 @@ void checkasm_stack_clobber(uint64_t clobber, ...); >>>> void checkasm_checked_call(void *func, ...); >>>> #define declare_new(ret, ...) ret (*checked_call)(void *, int, int, int, int, int, int, int, __VA_ARGS__)\ >>>> = (void *)checkasm_checked_call; >>>> +#define declare_new_float(ret, ...) ret (*checked_call)(void *, int, int, int, int, int, int, int, __VA_ARGS__)\ >>>> + = (void *)checkasm_checked_call; >>> >>> Isn't this doing the same as the generic "#define declare_new_float" >>> about 15 lines below? If declare_new_float() is no different than >>> declare_new() for a given target, then just let the aforementioned check >>> handle it. >> >> You are right (I missed this define), so this chunk is removed from the >> patch. >> >>> >>>> #define CLOB (UINT64_C(0xdeadbeefdeadbeef)) >>>> #define call_new(...) (checkasm_stack_clobber(CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,\ >>>> CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB),\ >>>> >>> >>> Should be ok either way if tested (The only aarch64 FATE client using >>> git head hasn't run in twenty days, so no way for me to check if this >>> fixes the problem anyway). >> >> I tested on my Odroid-C2 and fate passes with this patch (otherwise it >> fails on checkasm float_dsp.scalarproduct_float test). >> >> I've attached the updated patch. I'll push it in a few hours if there is >> no objection. >> >> -- >> Matthieu B. > >> From ee4bd4fdb9ac01bd331a43222e9029f820bec4e5 Mon Sep 17 00:00:00 2001 >> From: Matthieu Bouron <matthieu.bouron@gmail.com> >> Date: Mon, 19 Jun 2017 10:55:28 +0200 >> Subject: [PATCH] checkasm/aarch64: fix tests returning a float >> >> Avoids overriding v0 (which containins the result of the tested >> function) in checkasm_call_checked. >> --- >> tests/checkasm/aarch64/checkasm.S | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/tests/checkasm/aarch64/checkasm.S b/tests/checkasm/aarch64/checkasm.S >> index 53a2a478dc..75a9a56143 100644 >> --- a/tests/checkasm/aarch64/checkasm.S >> +++ b/tests/checkasm/aarch64/checkasm.S >> @@ -112,10 +112,10 @@ function checkasm_checked_call, export=1 >> movi v3.8h, #0 >> >> .macro check_reg_neon reg1, reg2 >> - ldr q0, [x9], #16 >> - uzp1 v1.2d, v\reg1\().2d, v\reg2\().2d >> - eor v0.16b, v0.16b, v1.16b >> - orr v3.16b, v3.16b, v0.16b >> + ldr q1, [x9], #16 >> + uzp1 v2.2d, v\reg1\().2d, v\reg2\().2d >> + eor v1.16b, v1.16b, v2.16b >> + orr v3.16b, v3.16b, v1.16b >> .endm >> check_reg_neon 8, 9 >> check_reg_neon 10, 11 >> -- >> 2.13.1 >> > > Patch applied. > > Thanks. Do you know if this needs to be done for arm as well? Fate is reporting an error in fate-checkasm-float_dsp. I added a fate-checkasm-sbrdsp target earlier today, so if that one also fails on arm then it will mean it's highly likely the function with a float return value is what's at fault.
Le 6 juil. 2017 4:08 AM, "James Almer" <jamrial@gmail.com> a écrit : On 6/22/2017 5:35 AM, Matthieu Bouron wrote: > On Wed, Jun 21, 2017 at 10:19:33PM +0200, Matthieu Bouron wrote: >> On Wed, Jun 21, 2017 at 04:57:53PM -0300, James Almer wrote: >>> On 6/19/2017 6:08 AM, Matthieu Bouron wrote: >>>> Avoids overriding v0 (which containins the result of the tested >>>> function) in checkasm_call_checked. >>>> >>>> Also properly calls checkasm_call_checked. >>>> --- >>>> tests/checkasm/aarch64/checkasm.S | 8 ++++---- >>>> tests/checkasm/checkasm.h | 2 ++ >>>> 2 files changed, 6 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/tests/checkasm/aarch64/checkasm.S b/tests/checkasm/aarch64/checkasm.S >>>> index 53a2a478dc..75a9a56143 100644 >>>> --- a/tests/checkasm/aarch64/checkasm.S >>>> +++ b/tests/checkasm/aarch64/checkasm.S >>>> @@ -112,10 +112,10 @@ function checkasm_checked_call, export=1 >>>> movi v3.8h, #0 >>>> >>>> .macro check_reg_neon reg1, reg2 >>>> - ldr q0, [x9], #16 >>>> - uzp1 v1.2d, v\reg1\().2d, v\reg2\().2d >>>> - eor v0.16b, v0.16b, v1.16b >>>> - orr v3.16b, v3.16b, v0.16b >>>> + ldr q1, [x9], #16 >>>> + uzp1 v2.2d, v\reg1\().2d, v\reg2\().2d >>>> + eor v1.16b, v1.16b, v2.16b >>>> + orr v3.16b, v3.16b, v1.16b >>>> .endm >>>> check_reg_neon 8, 9 >>>> check_reg_neon 10, 11 >>>> diff --git a/tests/checkasm/checkasm.h b/tests/checkasm/checkasm.h >>>> index 695d871220..5249669fba 100644 >>>> --- a/tests/checkasm/checkasm.h >>>> +++ b/tests/checkasm/checkasm.h >>>> @@ -145,6 +145,8 @@ void checkasm_stack_clobber(uint64_t clobber, ...); >>>> void checkasm_checked_call(void *func, ...); >>>> #define declare_new(ret, ...) ret (*checked_call)(void *, int, int, int, int, int, int, int, __VA_ARGS__)\ >>>> = (void *)checkasm_checked_call; >>>> +#define declare_new_float(ret, ...) ret (*checked_call)(void *, int, int, int, int, int, int, int, __VA_ARGS__)\ >>>> + = (void *)checkasm_checked_call; >>> >>> Isn't this doing the same as the generic "#define declare_new_float" >>> about 15 lines below? If declare_new_float() is no different than >>> declare_new() for a given target, then just let the aforementioned check >>> handle it. >> >> You are right (I missed this define), so this chunk is removed from the >> patch. >> >>> >>>> #define CLOB (UINT64_C(0xdeadbeefdeadbeef)) >>>> #define call_new(...) (checkasm_stack_clobber(CLOB, CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,\ >>>> CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB),\ >>>> >>> >>> Should be ok either way if tested (The only aarch64 FATE client using >>> git head hasn't run in twenty days, so no way for me to check if this >>> fixes the problem anyway). >> >> I tested on my Odroid-C2 and fate passes with this patch (otherwise it >> fails on checkasm float_dsp.scalarproduct_float test). >> >> I've attached the updated patch. I'll push it in a few hours if there is >> no objection. >> >> -- >> Matthieu B. > >> From ee4bd4fdb9ac01bd331a43222e9029f820bec4e5 Mon Sep 17 00:00:00 2001 >> From: Matthieu Bouron <matthieu.bouron@gmail.com> >> Date: Mon, 19 Jun 2017 10:55:28 +0200 >> Subject: [PATCH] checkasm/aarch64: fix tests returning a float >> >> Avoids overriding v0 (which containins the result of the tested >> function) in checkasm_call_checked. >> --- >> tests/checkasm/aarch64/checkasm.S | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/tests/checkasm/aarch64/checkasm.S b/tests/checkasm/aarch64/ checkasm.S >> index 53a2a478dc..75a9a56143 100644 >> --- a/tests/checkasm/aarch64/checkasm.S >> +++ b/tests/checkasm/aarch64/checkasm.S >> @@ -112,10 +112,10 @@ function checkasm_checked_call, export=1 >> movi v3.8h, #0 >> >> .macro check_reg_neon reg1, reg2 >> - ldr q0, [x9], #16 >> - uzp1 v1.2d, v\reg1\().2d, v\reg2\().2d >> - eor v0.16b, v0.16b, v1.16b >> - orr v3.16b, v3.16b, v0.16b >> + ldr q1, [x9], #16 >> + uzp1 v2.2d, v\reg1\().2d, v\reg2\().2d >> + eor v1.16b, v1.16b, v2.16b >> + orr v3.16b, v3.16b, v1.16b >> .endm >> check_reg_neon 8, 9 >> check_reg_neon 10, 11 >> -- >> 2.13.1 >> > > Patch applied. > > Thanks. Do you know if this needs to be done for arm as well? Fate is reporting an error in fate-checkasm-float_dsp. I added a fate-checkasm-sbrdsp target earlier today, so if that one also fails on arm then it will mean it's highly likely the function with a float return value is what's at fault.
From ee4bd4fdb9ac01bd331a43222e9029f820bec4e5 Mon Sep 17 00:00:00 2001 From: Matthieu Bouron <matthieu.bouron@gmail.com> Date: Mon, 19 Jun 2017 10:55:28 +0200 Subject: [PATCH] checkasm/aarch64: fix tests returning a float Avoids overriding v0 (which containins the result of the tested function) in checkasm_call_checked. --- tests/checkasm/aarch64/checkasm.S | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/checkasm/aarch64/checkasm.S b/tests/checkasm/aarch64/checkasm.S index 53a2a478dc..75a9a56143 100644 --- a/tests/checkasm/aarch64/checkasm.S +++ b/tests/checkasm/aarch64/checkasm.S @@ -112,10 +112,10 @@ function checkasm_checked_call, export=1 movi v3.8h, #0 .macro check_reg_neon reg1, reg2 - ldr q0, [x9], #16 - uzp1 v1.2d, v\reg1\().2d, v\reg2\().2d - eor v0.16b, v0.16b, v1.16b - orr v3.16b, v3.16b, v0.16b + ldr q1, [x9], #16 + uzp1 v2.2d, v\reg1\().2d, v\reg2\().2d + eor v1.16b, v1.16b, v2.16b + orr v3.16b, v3.16b, v1.16b .endm check_reg_neon 8, 9 check_reg_neon 10, 11 -- 2.13.1