Re: [PATCH] mlx4: remove limitation on LSO header size

[Eli Cohen <eli-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>]

On Wed, Oct 07, 2009 at 03:45:16PM -0700, Roland Dreier wrote:
> 
>  > +	*blh = unlikely(halign > 64) ? 1 : 0;
> 
> This idiom of "(boolean condition) ? 1 : 0" looks odd to me... doesn't
> (halign > 64) already evaluate to 1 or 0 anyway?  Does the unlikely()
> actually affect code generation here?
True, (halign > 64) is the same and is cleaner. As for the unlikely()
-- well it's already been there and besides, we're never sure if it
will improve anything so the same question could be asked for other
places in the code.


> 
> With that said, see below...
> 
>  > +	int blh = 0;
> 
> I assume this initialization is to avoid a compiler warning.  But the
> code is actually correct without initializing blh -- so I think that we
> save a tiny bit of code by doing uninitialized_var() instead?
We must initialize blh since it is used for any send request and not
just LSO opcodes. 


> 
>  > +			(blh ? cpu_to_be32(1 << 6) : 0);
> 
> ...given that the only use of blh is as a flag to decide what constant
> to use here, does it generate better code to make blh be __be32 and set
> the value directly in build_lso_seg, ie do:
> 
> 	*blh = unlikely(halign > 64) ? cpu_to_be32(1 << 6) : 0;
> 
> and then use blh without ?: in mlx4_ib_post_send...
> 

So we can let build_lso_header() put the corrent value for blh and
unconditionally "or" it into owner_opcode.