Re: [PATCH 09/11] test/ring: disable problematic tests for RISC-V

["Stanisław Kardach" <kda@semihalf.com>]

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
>