> From: Bruce Richardson [mailto:bruce.richard...@intel.com]
> Sent: Monday, 4 November 2024 13.19
>
> On Mon, Nov 04, 2024 at 12:40:49PM +0100, Morten Brørup wrote:
> > Ping for review.
> >
> > > From: Morten Brørup [mailto:m...@smartsharesystems.com]
> > > Sent: Friday, 25 October 2024 13.52
> > >
> > > Added two new compiler/optimizer hints:
> > > * The __rte_unreachable hint for use in points in code known never
> to
> > > be
> > > reached.
> > > * The __rte_assume hint for providing information about
> preconditions
> > > the
> > > compiler/optimizer might be unable to figure out by itself.
> > >
> > > Signed-off-by: Morten Brørup <m...@smartsharesystems.com>
> > > ---
> > > lib/eal/include/rte_common.h | 27 +++++++++++++++++++++++++++
> > > 1 file changed, 27 insertions(+)
> > >
> > > diff --git a/lib/eal/include/rte_common.h
> > > b/lib/eal/include/rte_common.h
> > > index c79f9ed319..2f143c87e4 100644
> > > --- a/lib/eal/include/rte_common.h
> > > +++ b/lib/eal/include/rte_common.h
> > > @@ -366,6 +366,16 @@ static void
> > > __attribute__((destructor(RTE_PRIO(prio)), used)) func(void)
> > > #define __rte_noreturn __attribute__((noreturn))
> > > #endif
> > >
> > > +/**
> > > + * Hint point in program never reached
> > > + */
> > > +#if defined(RTE_TOOLCHAIN_GCC) || defined(RTE_TOOLCHAIN_CLANG)
> > > +#define __rte_unreachable() __extension__(__builtin_unreachable())
> > > +#else
> > > +/* MSVC or ICC */
> > > +#define __rte_unreachable() __assume(0)
> > > +#endif
> > > +
>
> Is there already somewhere in the code where we might want to use this?
Yes. It's used in the __rte_assume() macro below for GCC before version 13.
> I'm
> not sure about just adding macros just in case they might be needed in
> future.
IMHO, such hints might be useful for applications, and DPDK could use them in
the future.
But if it came to a vote about adding unused/dead code, it would probably not
be accepted.
Anyway, this macro is not unused, so no problem here.
> Having unreachable code seems a bit problematic to me generally
> anyway.
Agree.
The correct use of __rte_unreachable() is to provide information to the
compiler/optimizer/sanitizer it cannot figure out by itself, and/or as
information to the code reviewer.
Using it to prevent compiler warnings from unreachable code would be utterly
wrong. I don't know if that is even possible.
>
> > > /**
> > > * Issue a warning in case the function's return value is ignored.
> > > *
> > > @@ -423,6 +433,23 @@ static void
> > > __attribute__((destructor(RTE_PRIO(prio)), used)) func(void)
> > > #define __rte_cold __attribute__((cold))
> > > #endif
> > >
> > > +/**
> > > + * Hint precondition
> > > + *
> > > + * @warning Depending on the compiler, any code in ``condition``
> might
> > > be executed.
> > > + * This currently only occurs with GCC prior to version 13.
> > > + */
> > > +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 130000)
> > > +#define __rte_assume(condition) __attribute__((assume(condition)))
> > > +#elif defined(RTE_TOOLCHAIN_GCC)
> > > +#define __rte_assume(condition) do { if (!(condition))
> > > __rte_unreachable(); } while (0)
Note: I did not come up with this myself; this seems to be the "official" way
of doing it with older GCC versions.
> > > +#elif defined(RTE_TOOLCHAIN_CLANG)
> > > +#define __rte_assume(condition)
> > > __extension__(__builtin_assume(condition))
> > > +#else
> > > +/* MSVC or ICC */
> > > +#define __rte_assume(condition) __assume(condition)
> > > +#endif
> > > +
>
> This part seems ok, I see it used in the next patch, and also looks
> rather
> useful to have.
>
> /Bruce
