All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] nfp: bpf: Fix bitwise vs. logical OR warning
@ 2021-10-18 19:31 Nathan Chancellor
  2021-10-18 19:39 ` Nick Desaulniers
  2021-10-18 21:54 ` Jakub Kicinski
  0 siblings, 2 replies; 4+ messages in thread
From: Nathan Chancellor @ 2021-10-18 19:31 UTC (permalink / raw)
  To: Simon Horman, Jakub Kicinski, David S. Miller
  Cc: Nick Desaulniers, oss-drivers, netdev, linux-kernel, bpf, llvm,
	Nathan Chancellor

A new warning in clang points out two places in this driver where
boolean expressions are being used with a bitwise OR instead of a
logical one:

drivers/net/ethernet/netronome/nfp/nfp_asm.c:199:20: error: use of bitwise '|' with boolean operands [-Werror,-Wbitwise-instead-of-logical]
        reg->src_lmextn = swreg_lmextn(lreg) | swreg_lmextn(rreg);
                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                                             ||
drivers/net/ethernet/netronome/nfp/nfp_asm.c:199:20: note: cast one or both operands to int to silence this warning
drivers/net/ethernet/netronome/nfp/nfp_asm.c:280:20: error: use of bitwise '|' with boolean operands [-Werror,-Wbitwise-instead-of-logical]
        reg->src_lmextn = swreg_lmextn(lreg) | swreg_lmextn(rreg);
                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                                             ||
drivers/net/ethernet/netronome/nfp/nfp_asm.c:280:20: note: cast one or both operands to int to silence this warning
2 errors generated.

The motivation for the warning is that logical operations short circuit
while bitwise operations do not. In this case, it does not seem like
short circuiting is harmful so implement the suggested fix of changing
to a logical operation to fix the warning.

Link: https://github.com/ClangBuiltLinux/linux/issues/1479
Reported-by: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
 drivers/net/ethernet/netronome/nfp/nfp_asm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_asm.c b/drivers/net/ethernet/netronome/nfp/nfp_asm.c
index 2643ea5948f4..154399c5453f 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_asm.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_asm.c
@@ -196,7 +196,7 @@ int swreg_to_unrestricted(swreg dst, swreg lreg, swreg rreg,
 	}
 
 	reg->dst_lmextn = swreg_lmextn(dst);
-	reg->src_lmextn = swreg_lmextn(lreg) | swreg_lmextn(rreg);
+	reg->src_lmextn = swreg_lmextn(lreg) || swreg_lmextn(rreg);
 
 	return 0;
 }
@@ -277,7 +277,7 @@ int swreg_to_restricted(swreg dst, swreg lreg, swreg rreg,
 	}
 
 	reg->dst_lmextn = swreg_lmextn(dst);
-	reg->src_lmextn = swreg_lmextn(lreg) | swreg_lmextn(rreg);
+	reg->src_lmextn = swreg_lmextn(lreg) || swreg_lmextn(rreg);
 
 	return 0;
 }

base-commit: 041c61488236a5a84789083e3d9f0a51139b6edf
-- 
2.33.1.637.gf443b226ca


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] nfp: bpf: Fix bitwise vs. logical OR warning
  2021-10-18 19:31 [PATCH] nfp: bpf: Fix bitwise vs. logical OR warning Nathan Chancellor
@ 2021-10-18 19:39 ` Nick Desaulniers
  2021-10-18 19:40   ` Nick Desaulniers
  2021-10-18 21:54 ` Jakub Kicinski
  1 sibling, 1 reply; 4+ messages in thread
From: Nick Desaulniers @ 2021-10-18 19:39 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Simon Horman, Jakub Kicinski, David S. Miller, oss-drivers,
	netdev, linux-kernel, bpf, llvm

On Mon, Oct 18, 2021 at 12:31 PM Nathan Chancellor <nathan@kernel.org> wrote:
>
> A new warning in clang points out two places in this driver where
> boolean expressions are being used with a bitwise OR instead of a
> logical one:
>
> drivers/net/ethernet/netronome/nfp/nfp_asm.c:199:20: error: use of bitwise '|' with boolean operands [-Werror,-Wbitwise-instead-of-logical]
>         reg->src_lmextn = swreg_lmextn(lreg) | swreg_lmextn(rreg);
>                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>                                              ||
> drivers/net/ethernet/netronome/nfp/nfp_asm.c:199:20: note: cast one or both operands to int to silence this warning
> drivers/net/ethernet/netronome/nfp/nfp_asm.c:280:20: error: use of bitwise '|' with boolean operands [-Werror,-Wbitwise-instead-of-logical]
>         reg->src_lmextn = swreg_lmextn(lreg) | swreg_lmextn(rreg);
>                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>                                              ||
> drivers/net/ethernet/netronome/nfp/nfp_asm.c:280:20: note: cast one or both operands to int to silence this warning
> 2 errors generated.
>
> The motivation for the warning is that logical operations short circuit
> while bitwise operations do not. In this case, it does not seem like
> short circuiting is harmful so implement the suggested fix of changing
> to a logical operation to fix the warning.

I agree. Thanks for the patch.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1479
> Reported-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
>  drivers/net/ethernet/netronome/nfp/nfp_asm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/netronome/nfp/nfp_asm.c b/drivers/net/ethernet/netronome/nfp/nfp_asm.c
> index 2643ea5948f4..154399c5453f 100644
> --- a/drivers/net/ethernet/netronome/nfp/nfp_asm.c
> +++ b/drivers/net/ethernet/netronome/nfp/nfp_asm.c
> @@ -196,7 +196,7 @@ int swreg_to_unrestricted(swreg dst, swreg lreg, swreg rreg,
>         }
>
>         reg->dst_lmextn = swreg_lmextn(dst);
> -       reg->src_lmextn = swreg_lmextn(lreg) | swreg_lmextn(rreg);
> +       reg->src_lmextn = swreg_lmextn(lreg) || swreg_lmextn(rreg);
>
>         return 0;
>  }
> @@ -277,7 +277,7 @@ int swreg_to_restricted(swreg dst, swreg lreg, swreg rreg,
>         }
>
>         reg->dst_lmextn = swreg_lmextn(dst);
> -       reg->src_lmextn = swreg_lmextn(lreg) | swreg_lmextn(rreg);
> +       reg->src_lmextn = swreg_lmextn(lreg) || swreg_lmextn(rreg);
>
>         return 0;
>  }
>
> base-commit: 041c61488236a5a84789083e3d9f0a51139b6edf
> --
> 2.33.1.637.gf443b226ca
>
>


-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] nfp: bpf: Fix bitwise vs. logical OR warning
  2021-10-18 19:39 ` Nick Desaulniers
@ 2021-10-18 19:40   ` Nick Desaulniers
  0 siblings, 0 replies; 4+ messages in thread
From: Nick Desaulniers @ 2021-10-18 19:40 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Simon Horman, Jakub Kicinski, David S. Miller, oss-drivers,
	netdev, linux-kernel, bpf, llvm

On Mon, Oct 18, 2021 at 12:39 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Mon, Oct 18, 2021 at 12:31 PM Nathan Chancellor <nathan@kernel.org> wrote:
> >
> > A new warning in clang points out two places in this driver where
> > boolean expressions are being used with a bitwise OR instead of a
> > logical one:
> >
> > drivers/net/ethernet/netronome/nfp/nfp_asm.c:199:20: error: use of bitwise '|' with boolean operands [-Werror,-Wbitwise-instead-of-logical]
> >         reg->src_lmextn = swreg_lmextn(lreg) | swreg_lmextn(rreg);
> >                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >                                              ||
> > drivers/net/ethernet/netronome/nfp/nfp_asm.c:199:20: note: cast one or both operands to int to silence this warning
> > drivers/net/ethernet/netronome/nfp/nfp_asm.c:280:20: error: use of bitwise '|' with boolean operands [-Werror,-Wbitwise-instead-of-logical]
> >         reg->src_lmextn = swreg_lmextn(lreg) | swreg_lmextn(rreg);
> >                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >                                              ||
> > drivers/net/ethernet/netronome/nfp/nfp_asm.c:280:20: note: cast one or both operands to int to silence this warning
> > 2 errors generated.
> >
> > The motivation for the warning is that logical operations short circuit
> > while bitwise operations do not. In this case, it does not seem like
> > short circuiting is harmful so implement the suggested fix of changing
> > to a logical operation to fix the warning.
>
> I agree. Thanks for the patch.
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

Perhaps:

Fixes: 995e101ffa71 ("nfp: bpf: encode extended LM pointer operands")

>
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/1479
> > Reported-by: Nick Desaulniers <ndesaulniers@google.com>
> > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> > ---
> >  drivers/net/ethernet/netronome/nfp/nfp_asm.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/netronome/nfp/nfp_asm.c b/drivers/net/ethernet/netronome/nfp/nfp_asm.c
> > index 2643ea5948f4..154399c5453f 100644
> > --- a/drivers/net/ethernet/netronome/nfp/nfp_asm.c
> > +++ b/drivers/net/ethernet/netronome/nfp/nfp_asm.c
> > @@ -196,7 +196,7 @@ int swreg_to_unrestricted(swreg dst, swreg lreg, swreg rreg,
> >         }
> >
> >         reg->dst_lmextn = swreg_lmextn(dst);
> > -       reg->src_lmextn = swreg_lmextn(lreg) | swreg_lmextn(rreg);
> > +       reg->src_lmextn = swreg_lmextn(lreg) || swreg_lmextn(rreg);
> >
> >         return 0;
> >  }
> > @@ -277,7 +277,7 @@ int swreg_to_restricted(swreg dst, swreg lreg, swreg rreg,
> >         }
> >
> >         reg->dst_lmextn = swreg_lmextn(dst);
> > -       reg->src_lmextn = swreg_lmextn(lreg) | swreg_lmextn(rreg);
> > +       reg->src_lmextn = swreg_lmextn(lreg) || swreg_lmextn(rreg);
> >
> >         return 0;
> >  }
> >
> > base-commit: 041c61488236a5a84789083e3d9f0a51139b6edf
> > --
> > 2.33.1.637.gf443b226ca
> >
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers



-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] nfp: bpf: Fix bitwise vs. logical OR warning
  2021-10-18 19:31 [PATCH] nfp: bpf: Fix bitwise vs. logical OR warning Nathan Chancellor
  2021-10-18 19:39 ` Nick Desaulniers
@ 2021-10-18 21:54 ` Jakub Kicinski
  1 sibling, 0 replies; 4+ messages in thread
From: Jakub Kicinski @ 2021-10-18 21:54 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Simon Horman, David S. Miller, Nick Desaulniers, oss-drivers,
	netdev, linux-kernel, bpf, llvm

On Mon, 18 Oct 2021 12:31:01 -0700 Nathan Chancellor wrote:
> A new warning in clang points out two places in this driver where
> boolean expressions are being used with a bitwise OR instead of a
> logical one:
> 
> drivers/net/ethernet/netronome/nfp/nfp_asm.c:199:20: error: use of bitwise '|' with boolean operands [-Werror,-Wbitwise-instead-of-logical]
>         reg->src_lmextn = swreg_lmextn(lreg) | swreg_lmextn(rreg);
>                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>                                              ||
> drivers/net/ethernet/netronome/nfp/nfp_asm.c:199:20: note: cast one or both operands to int to silence this warning
> drivers/net/ethernet/netronome/nfp/nfp_asm.c:280:20: error: use of bitwise '|' with boolean operands [-Werror,-Wbitwise-instead-of-logical]
>         reg->src_lmextn = swreg_lmextn(lreg) | swreg_lmextn(rreg);
>                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>                                              ||
> drivers/net/ethernet/netronome/nfp/nfp_asm.c:280:20: note: cast one or both operands to int to silence this warning
> 2 errors generated.
> 
> The motivation for the warning is that logical operations short circuit
> while bitwise operations do not. In this case, it does not seem like
> short circuiting is harmful so implement the suggested fix of changing
> to a logical operation to fix the warning.

Warning seems generally useful, although in this case it is a little
out of place (swreg_lmextn() is a field extractor after all).

Applied to net, but without the Fixes tag, thanks!

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-10-18 21:54 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-18 19:31 [PATCH] nfp: bpf: Fix bitwise vs. logical OR warning Nathan Chancellor
2021-10-18 19:39 ` Nick Desaulniers
2021-10-18 19:40   ` Nick Desaulniers
2021-10-18 21:54 ` Jakub Kicinski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.