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. _______________________________________________ ewg mailing list ewg@lists.openfabrics.org http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ewg