On Tue, Sep 12, 2017 at 11:10 AM, Y Song <ys114321@xxxxxxxxx> 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
|