[09/11] test/ring: disable problematic tests for RISC-V
Checks
Commit Message
When compiling for RISC-V in debug mode the large amount of inlining in
test_ring_basic_ex() and test_ring_with_exact_size() (in test_ring.c)
leads to large loop bodies. This causes 'goto' and 'for' loop
PC-relative jumps generated by the compiler to go beyond the architecture
limitation of +/-1MB offset (the 'j <offset>' instruction). This
instruction should not be generated by the compiler since C language does
not limit the maximum distance for 'goto' or 'for' loop jumps.
This only happens in the unit test for ring which tries to perform long
loops with ring enqueue/dequeue and it seems to be caused by excessive
__rte_always_inline usage. ring perf test compiles just fine under
debug.
To work around this, disable the offending tests in debug mode.
Signed-off-by: Stanislaw Kardach <kda@semihalf.com>
Sponsored-by: Frank Zhao <Frank.Zhao@starfivetech.com>
Sponsored-by: Sam Grove <sam.grove@sifive.com>
---
app/test/test_ring.c | 8 ++++++++
config/riscv/meson.build | 5 +++++
doc/guides/rel_notes/release_22_07.rst | 3 ++-
3 files changed, 15 insertions(+), 1 deletion(-)
Comments
On Thu, 5 May 2022 19:30:01 +0200
Stanislaw Kardach <kda@semihalf.com> wrote:
> When compiling for RISC-V in debug mode the large amount of inlining in
> test_ring_basic_ex() and test_ring_with_exact_size() (in test_ring.c)
> leads to large loop bodies. This causes 'goto' and 'for' loop
> PC-relative jumps generated by the compiler to go beyond the architecture
> limitation of +/-1MB offset (the 'j <offset>' instruction). This
> instruction should not be generated by the compiler since C language does
> not limit the maximum distance for 'goto' or 'for' loop jumps.
>
> This only happens in the unit test for ring which tries to perform long
> loops with ring enqueue/dequeue and it seems to be caused by excessive
> __rte_always_inline usage. ring perf test compiles just fine under
> debug.
>
> To work around this, disable the offending tests in debug mode.
>
> Signed-off-by: Stanislaw Kardach <kda@semihalf.com>
> Sponsored-by: Frank Zhao <Frank.Zhao@starfivetech.com>
> Sponsored-by: Sam Grove <sam.grove@sifive.com>
> ---
It seems to me that fixing the excessive inlining in the ring code
could benefit all architectures, rather than neutering the tests
on RISCV.
On Thu, May 5, 2022 at 7:35 PM Stephen Hemminger <stephen@networkplumber.org>
wrote:
> On Thu, 5 May 2022 19:30:01 +0200
> Stanislaw Kardach <kda@semihalf.com> wrote:
>
> > When compiling for RISC-V in debug mode the large amount of inlining in
> > test_ring_basic_ex() and test_ring_with_exact_size() (in test_ring.c)
> > leads to large loop bodies. This causes 'goto' and 'for' loop
> > PC-relative jumps generated by the compiler to go beyond the architecture
> > limitation of +/-1MB offset (the 'j <offset>' instruction). This
> > instruction should not be generated by the compiler since C language does
> > not limit the maximum distance for 'goto' or 'for' loop jumps.
> >
> > This only happens in the unit test for ring which tries to perform long
> > loops with ring enqueue/dequeue and it seems to be caused by excessive
> > __rte_always_inline usage. ring perf test compiles just fine under
> > debug.
> >
> > To work around this, disable the offending tests in debug mode.
> >
> > Signed-off-by: Stanislaw Kardach <kda@semihalf.com>
> > Sponsored-by: Frank Zhao <Frank.Zhao@starfivetech.com>
> > Sponsored-by: Sam Grove <sam.grove@sifive.com>
> > ---
>
> It seems to me that fixing the excessive inlining in the ring code
> could benefit all architectures, rather than neutering the tests
> on RISCV.
>
True. Since this only happened in the tests that I've mentioned, my other
approach was to introduce a "slow" wrapper over test_ring_dequeue|enqueue()
which did not force inlining via __rte_always_inline and use it in
functional test functions. However after talking with Thomas Monjalon we've
decided to guard the debug build of RISC-V only.
Another thing is that this is a clear bug in the compiler, the relaxation
of the jump should not be done since RISC-V has long jump construct for
arbitrary jumps (auipc+jalr).
On Thu, 5 May 2022 19:43:43 +0200
Stanisław Kardach <kda@semihalf.com> wrote:
> On Thu, May 5, 2022 at 7:35 PM Stephen Hemminger <stephen@networkplumber.org>
> wrote:
>
> > On Thu, 5 May 2022 19:30:01 +0200
> > Stanislaw Kardach <kda@semihalf.com> wrote:
> >
> > > When compiling for RISC-V in debug mode the large amount of inlining in
> > > test_ring_basic_ex() and test_ring_with_exact_size() (in test_ring.c)
> > > leads to large loop bodies. This causes 'goto' and 'for' loop
> > > PC-relative jumps generated by the compiler to go beyond the architecture
> > > limitation of +/-1MB offset (the 'j <offset>' instruction). This
> > > instruction should not be generated by the compiler since C language does
> > > not limit the maximum distance for 'goto' or 'for' loop jumps.
> > >
> > > This only happens in the unit test for ring which tries to perform long
> > > loops with ring enqueue/dequeue and it seems to be caused by excessive
> > > __rte_always_inline usage. ring perf test compiles just fine under
> > > debug.
> > >
> > > To work around this, disable the offending tests in debug mode.
> > >
> > > Signed-off-by: Stanislaw Kardach <kda@semihalf.com>
> > > Sponsored-by: Frank Zhao <Frank.Zhao@starfivetech.com>
> > > Sponsored-by: Sam Grove <sam.grove@sifive.com>
> > > ---
> >
> > It seems to me that fixing the excessive inlining in the ring code
> > could benefit all architectures, rather than neutering the tests
> > on RISCV.
> >
> True. Since this only happened in the tests that I've mentioned, my other
> approach was to introduce a "slow" wrapper over test_ring_dequeue|enqueue()
> which did not force inlining via __rte_always_inline and use it in
> functional test functions. However after talking with Thomas Monjalon we've
> decided to guard the debug build of RISC-V only.
> Another thing is that this is a clear bug in the compiler, the relaxation
> of the jump should not be done since RISC-V has long jump construct for
> arbitrary jumps (auipc+jalr).
There is no good reason the __rte_always_inline should be in the ring code.
The purpose of always inline should be only for code that would break if
not inlined.
<snip>
>
> When compiling for RISC-V in debug mode the large amount of inlining in
> test_ring_basic_ex() and test_ring_with_exact_size() (in test_ring.c) leads to
> large loop bodies. This causes 'goto' and 'for' loop PC-relative jumps generated
> by the compiler to go beyond the architecture limitation of +/-1MB offset (the
> 'j <offset>' instruction). This instruction should not be generated by the
> compiler since C language does not limit the maximum distance for 'goto' or
> 'for' loop jumps.
>
> This only happens in the unit test for ring which tries to perform long loops
> with ring enqueue/dequeue and it seems to be caused by excessive
> __rte_always_inline usage. ring perf test compiles just fine under debug.
>
> To work around this, disable the offending tests in debug mode.
Is this still required given you have submitted [1]
[1] http://patches.dpdk.org/project/dpdk/patch/20220510115758.457794-1-kda@semihalf.com/
>
> Signed-off-by: Stanislaw Kardach <kda@semihalf.com>
> Sponsored-by: Frank Zhao <Frank.Zhao@starfivetech.com>
> Sponsored-by: Sam Grove <sam.grove@sifive.com>
> ---
> app/test/test_ring.c | 8 ++++++++
> config/riscv/meson.build | 5 +++++
> doc/guides/rel_notes/release_22_07.rst | 3 ++-
> 3 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/app/test/test_ring.c b/app/test/test_ring.c index
> bde33ab4a1..7d809c147b 100644
> --- a/app/test/test_ring.c
> +++ b/app/test/test_ring.c
> @@ -955,6 +955,7 @@ test_ring_burst_bulk_tests4(unsigned int test_idx)
> return -1;
> }
>
> +#if !defined(RTE_RISCV_WO_DISABLE_RING_TESTS)
> /*
> * Test default, single element, bulk and burst APIs
> */
> @@ -1189,6 +1190,7 @@ test_ring_with_exact_size(void)
> rte_ring_free(exact_sz_r);
> return -1;
> }
> +#endif
>
> static int
> test_ring(void)
> @@ -1200,12 +1202,18 @@ test_ring(void)
> if (test_ring_negative_tests() < 0)
> goto test_fail;
>
> +/* Disable the following tests on RISC-V in debug mode. This is a
> +work-around
> + * GCC bug for RISC-V which fails to generate proper jumps for loops
> +with large
> + * bodies.
> + */
> +#if !defined(RTE_RISCV_WO_DISABLE_RING_TESTS)
> /* Some basic operations */
> if (test_ring_basic_ex() < 0)
> goto test_fail;
>
> if (test_ring_with_exact_size() < 0)
> goto test_fail;
> +#endif
>
> /* Burst and bulk operations with sp/sc, mp/mc and default.
> * The test cases are split into smaller test cases to diff --git
> a/config/riscv/meson.build b/config/riscv/meson.build index
> 0c16c31fc2..50d0b513bf 100644
> --- a/config/riscv/meson.build
> +++ b/config/riscv/meson.build
> @@ -141,3 +141,8 @@ foreach flag: dpdk_flags endforeach message('Using
> machine args: @0@'.format(machine_args))
>
> +# Enable work-around for ring unit tests in debug mode which fail to
> +link # properly due to bad code generation by GCC.
> +if get_option('optimization') == '0' or get_option('optimization') == 'g'
> + add_project_arguments('-DRTE_RISCV_WO_DISABLE_RING_TESTS',
> +language: 'c') endif
> diff --git a/doc/guides/rel_notes/release_22_07.rst
> b/doc/guides/rel_notes/release_22_07.rst
> index 453591e568..4d64b68dfd 100644
> --- a/doc/guides/rel_notes/release_22_07.rst
> +++ b/doc/guides/rel_notes/release_22_07.rst
> @@ -76,7 +76,8 @@ New Features
> * Debug build of ``app/test/dpdk-test`` fails currently on RISC-V due to
> seemingly invalid loop and goto jump code generation by GCC in
> ``test_ring.c`` where extensive inlining increases the code size beyond the
> - capability of the generated instruction (JAL: +/-1MB PC-relative).
> + capability of the generated instruction (JAL: +/-1MB PC-relative). The
> + workaround is to disable ``test_ring_basic_ex()`` and
> ``test_ring_with_exact_size()`` on RISC-V on ``-O0`` or ``-Og``.
>
> * **Updated Intel iavf driver.**
>
> --
> 2.30.2
On Wed, May 11, 2022 at 1:28 AM Honnappa Nagarahalli
<Honnappa.Nagarahalli@arm.com> wrote:
>
> <snip>
>
> >
> > When compiling for RISC-V in debug mode the large amount of inlining in
> > test_ring_basic_ex() and test_ring_with_exact_size() (in test_ring.c) leads to
> > large loop bodies. This causes 'goto' and 'for' loop PC-relative jumps generated
> > by the compiler to go beyond the architecture limitation of +/-1MB offset (the
> > 'j <offset>' instruction). This instruction should not be generated by the
> > compiler since C language does not limit the maximum distance for 'goto' or
> > 'for' loop jumps.
> >
> > This only happens in the unit test for ring which tries to perform long loops
> > with ring enqueue/dequeue and it seems to be caused by excessive
> > __rte_always_inline usage. ring perf test compiles just fine under debug.
> >
> > To work around this, disable the offending tests in debug mode.
> Is this still required given you have submitted [1]
>
> [1] http://patches.dpdk.org/project/dpdk/patch/20220510115758.457794-1-kda@semihalf.com/
Correct, this patch is no longer necessary and marked as superseded in
patchwork. I did send v2 and v3 series without it. Thank you for
taking a look.
>
> >
> > Signed-off-by: Stanislaw Kardach <kda@semihalf.com>
> > Sponsored-by: Frank Zhao <Frank.Zhao@starfivetech.com>
> > Sponsored-by: Sam Grove <sam.grove@sifive.com>
> > ---
> > app/test/test_ring.c | 8 ++++++++
> > config/riscv/meson.build | 5 +++++
> > doc/guides/rel_notes/release_22_07.rst | 3 ++-
> > 3 files changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/app/test/test_ring.c b/app/test/test_ring.c index
> > bde33ab4a1..7d809c147b 100644
> > --- a/app/test/test_ring.c
> > +++ b/app/test/test_ring.c
> > @@ -955,6 +955,7 @@ test_ring_burst_bulk_tests4(unsigned int test_idx)
> > return -1;
> > }
> >
> > +#if !defined(RTE_RISCV_WO_DISABLE_RING_TESTS)
> > /*
> > * Test default, single element, bulk and burst APIs
> > */
> > @@ -1189,6 +1190,7 @@ test_ring_with_exact_size(void)
> > rte_ring_free(exact_sz_r);
> > return -1;
> > }
> > +#endif
> >
> > static int
> > test_ring(void)
> > @@ -1200,12 +1202,18 @@ test_ring(void)
> > if (test_ring_negative_tests() < 0)
> > goto test_fail;
> >
> > +/* Disable the following tests on RISC-V in debug mode. This is a
> > +work-around
> > + * GCC bug for RISC-V which fails to generate proper jumps for loops
> > +with large
> > + * bodies.
> > + */
> > +#if !defined(RTE_RISCV_WO_DISABLE_RING_TESTS)
> > /* Some basic operations */
> > if (test_ring_basic_ex() < 0)
> > goto test_fail;
> >
> > if (test_ring_with_exact_size() < 0)
> > goto test_fail;
> > +#endif
> >
> > /* Burst and bulk operations with sp/sc, mp/mc and default.
> > * The test cases are split into smaller test cases to diff --git
> > a/config/riscv/meson.build b/config/riscv/meson.build index
> > 0c16c31fc2..50d0b513bf 100644
> > --- a/config/riscv/meson.build
> > +++ b/config/riscv/meson.build
> > @@ -141,3 +141,8 @@ foreach flag: dpdk_flags endforeach message('Using
> > machine args: @0@'.format(machine_args))
> >
> > +# Enable work-around for ring unit tests in debug mode which fail to
> > +link # properly due to bad code generation by GCC.
> > +if get_option('optimization') == '0' or get_option('optimization') == 'g'
> > + add_project_arguments('-DRTE_RISCV_WO_DISABLE_RING_TESTS',
> > +language: 'c') endif
> > diff --git a/doc/guides/rel_notes/release_22_07.rst
> > b/doc/guides/rel_notes/release_22_07.rst
> > index 453591e568..4d64b68dfd 100644
> > --- a/doc/guides/rel_notes/release_22_07.rst
> > +++ b/doc/guides/rel_notes/release_22_07.rst
> > @@ -76,7 +76,8 @@ New Features
> > * Debug build of ``app/test/dpdk-test`` fails currently on RISC-V due to
> > seemingly invalid loop and goto jump code generation by GCC in
> > ``test_ring.c`` where extensive inlining increases the code size beyond the
> > - capability of the generated instruction (JAL: +/-1MB PC-relative).
> > + capability of the generated instruction (JAL: +/-1MB PC-relative). The
> > + workaround is to disable ``test_ring_basic_ex()`` and
> > ``test_ring_with_exact_size()`` on RISC-V on ``-O0`` or ``-Og``.
> >
> > * **Updated Intel iavf driver.**
> >
> > --
> > 2.30.2
>
@@ -955,6 +955,7 @@ test_ring_burst_bulk_tests4(unsigned int test_idx)
return -1;
}
+#if !defined(RTE_RISCV_WO_DISABLE_RING_TESTS)
/*
* Test default, single element, bulk and burst APIs
*/
@@ -1189,6 +1190,7 @@ test_ring_with_exact_size(void)
rte_ring_free(exact_sz_r);
return -1;
}
+#endif
static int
test_ring(void)
@@ -1200,12 +1202,18 @@ test_ring(void)
if (test_ring_negative_tests() < 0)
goto test_fail;
+/* Disable the following tests on RISC-V in debug mode. This is a work-around
+ * GCC bug for RISC-V which fails to generate proper jumps for loops with large
+ * bodies.
+ */
+#if !defined(RTE_RISCV_WO_DISABLE_RING_TESTS)
/* Some basic operations */
if (test_ring_basic_ex() < 0)
goto test_fail;
if (test_ring_with_exact_size() < 0)
goto test_fail;
+#endif
/* Burst and bulk operations with sp/sc, mp/mc and default.
* The test cases are split into smaller test cases to
@@ -141,3 +141,8 @@ foreach flag: dpdk_flags
endforeach
message('Using machine args: @0@'.format(machine_args))
+# Enable work-around for ring unit tests in debug mode which fail to link
+# properly due to bad code generation by GCC.
+if get_option('optimization') == '0' or get_option('optimization') == 'g'
+ add_project_arguments('-DRTE_RISCV_WO_DISABLE_RING_TESTS', language: 'c')
+endif
@@ -76,7 +76,8 @@ New Features
* Debug build of ``app/test/dpdk-test`` fails currently on RISC-V due to
seemingly invalid loop and goto jump code generation by GCC in
``test_ring.c`` where extensive inlining increases the code size beyond the
- capability of the generated instruction (JAL: +/-1MB PC-relative).
+ capability of the generated instruction (JAL: +/-1MB PC-relative). The
+ workaround is to disable ``test_ring_basic_ex()`` and ``test_ring_with_exact_size()`` on RISC-V on ``-O0`` or ``-Og``.
* **Updated Intel iavf driver.**