Re: [PATCH rdma-core 4/8] mlx5: Avoid sparse complaints about !!

Bart Van Assche
2017-07-13T17:52:06Z

On Thu, 2017-07-13 at 11:20 -0600, Jason Gunthorpe wrote:
> On Thu, Jul 13, 2017 at 12:51:16AM -0700, Christoph Hellwig wrote:
> > > if (qp->qp_cap_cache & MLX5_RX_CSUM_VALID)
> > > - wc->wc_flags |= (!!(cqe->hds_ip_ext & MLX5_CQE_L4_OK) &
> > > - !!(cqe->hds_ip_ext & MLX5_CQE_L3_OK) &
> > > - (get_cqe_l3_hdr_type(cqe) ==
> > > - MLX5_CQE_L3_HDR_TYPE_IPV4)) <<
> > > - IBV_WC_IP_CSUM_OK_SHIFT;
> > > + wc->wc_flags |=
> > > + ((bool)(cqe->hds_ip_ext & MLX5_CQE_L4_OK) &
> > > + (bool)(cqe->hds_ip_ext & MLX5_CQE_L3_OK) &
> > > + (get_cqe_l3_hdr_type(cqe) ==
> > > + MLX5_CQE_L3_HDR_TYPE_IPV4))
> > > + << IBV_WC_IP_CSUM_OK_SHIFT;
> >
> > Meh. This code is complete crap. Please factor it out into a little
> > helper that mere humans can read first. And then replace the odd ^ used
> > as && with proper if constructs and all should make much more sense.
>
> As far as I could make out, this ugly thing is designed like this for
> performance.

Hello Jason,

How about using an expression like the below to avoid that branches get inserted
for testing the MLX5_CQE_L4_OK and MLX5_CQE_L3_OK flags?

(cqe->hds_ip_ext & (MLX5_CQE_L4_OK | MLX5_CQE_L3_OK)) == MLX5_CQE_L4_OK | MLX5_CQE_L3_OK

Bart.--