Re: [v2][PATCH RFC] Add BPF AsmParser support in LLVM


Jiong Wang
 

On Tue, Sep 12, 2017 at 11:10 AM, Y Song <ys114321@...> wrote:
Thanks, Jiong,

The patch looks good. I have applied to llvm trunk.
https://reviews.llvm.org/rL313055
Thanks, Yonghong.


Hmm, I actually feel there is no need to offer an separate syntax for
64bit imm
encoding, it might be better to offer a unified single syntax "r =
signed_imm"
to the user and the encoder (BPFMCCodeEmitter::encodeInstruction)
guarantee to
choose optimal encoding.

Encoder could choose BPF_ALU64 | BPF_MOV | BPF_K for both "r1 = -1" and
"r1 = -1ll" while only resorting to BPF_LD | BPF_DW | BPF_IMM when the imm
really
doesn't fit into 32bit (!isInt<32>(imm_opnd)), for example, "r1 =
0x1ffffffffll".
Right now, this optimization actually
has been done in compiler side. So depending on the value, the compiler will
use
"move r1, <32_bit_value>" or "ld_imm64 r1, <greater_than_32_bit_value>".
In this case, it make sense to have different insn formats for two different
kinds of
insns.

What you described is to move the optimization down to MC Emit Code stage,
based on
the value, it can choose to use "move" or "ld_imm64" insn. So optimization
will be available
for .c => .o and for .s => .o. (The current scheme, optimization not
available from .s => .o).

Yes, it is possible. But typically, there is one-to-one mapping between asm
insn to insn encoding.
If later on, we found the complain about this, we can revisit this issue.
OK.

Regards,
Jiong

Join iovisor-dev@lists.iovisor.org to automatically receive all group messages.