RE: [PATCH bpf-next v4 01/11] bpf: x86: Factor out emission of ModR/M for *(reg + off)

[John Fastabend]

Brendan Jackman wrote:
> The case for JITing atomics is about to get more complicated. Let's
> factor out some common code to make the review and result more
> readable.
> 
> NB the atomics code doesn't yet use the new helper - a subsequent
> patch will add its use as a side-effect of other changes.
> 
> Signed-off-by: Brendan Jackman <jackmanb@xxxxxxxxxx>
> ---

Small nit on style preference below.

Acked-by: John Fastabend <john.fastabend@xxxxxxxxx>

[...]

>  
> @@ -1240,11 +1250,7 @@ st:			if (is_imm8(insn->off))
>  			goto xadd;
>  		case BPF_STX | BPF_XADD | BPF_DW:
>  			EMIT3(0xF0, add_2mod(0x48, dst_reg, src_reg), 0x01);
> -xadd:			if (is_imm8(insn->off))
> -				EMIT2(add_2reg(0x40, dst_reg, src_reg), insn->off);
> -			else
> -				EMIT1_off32(add_2reg(0x80, dst_reg, src_reg),
> -					    insn->off);
> +xadd:			emit_modrm_dstoff(&prog, dst_reg, src_reg, insn->off);

I at least prefer the xadd on its own line above the emit_*(). That seems
more consistent with the rest of the code in this file. The only other
example like this is st:.