On Thu, Dec 6, 2018 at 7:59 AM Stephen Rothwell <sfr@xxxxxxxxxxxxxxxx> wrote:
>
> Hi Guy,
>
> On Wed, 5 Dec 2018 12:25:57 +0000 "Guy Levi(SW)" <guyle@xxxxxxxxxxxx> wrote:
> >
> > >
> > > Huh. So apparently every compiler that tested this patch (0-day, mine,
> > > the submitters) optimized this call away because is_atomic_response()
> > > always returns 0: meaning mlx5_get_atomic_laddr is never callable and
> > > can be deleted entirely, including the call to mlx5_get_send_wqe.
> > >
> > > Not sure what compiler setup will hit this, but it is clearly wrong
> > > code..
> >
> > Flag -o0 ?
>
> No, but the kbuild tree contains a change that allows turning off of
> gcc's autoinlining and the CONFIG option guarding that gets turned on
> for allmodconfig builds among others.
>
> Masahiro, should CONFIG_NO_AUTO_INLINE maybe need to be off unless
> explicitly enabled (like CONFIG_DEBUG_INFO and others)?
No.
If CONFIG_NO_AUTO_INLINE is turned off for compile-testing,
people will not even notice a breakage,
then the code will get broken here and there.
You will not be able to enable it
when you really want to use it.
In this case, the reason is obvious.
If you expect the compiler to optimize the code out,
you must use 'static inline' instead of 'static'.
static int is_atomic_response(struct mlx5_ib_qp *qp, uint16_t idx)
{
/* TBD: waiting decision
*/
return 0;
}
--
Best Regards
Masahiro Yamada
