[iovisor-dev] [v2][PATCH RFC] Add BPF AsmParser support in LLVM

Jiong Wang jiong.wang at netronome.com
Mon Sep 11 14:28:52 UTC 2017


On 09/09/17 00:47, Y Song wrote:
> On Fri, Sep 8, 2017 at 1:00 PM, Jiong Wang <jiong.wang at netronome.com> wrote:
>>
>> Hi Y Song,
>>
>>   Thanks for the review and test.
>>
>> [snip]
>>>
>>> I guess, we may need to add something like LD_VAR_ADDR so that
>>> the proper assembly code can be issued. Also we could drop "ll" with
>>> "r1 = tx_port"?
>>
>>
>> Personally, I prefer drop "ll". The "ll" suffix was there to tell people it is 64bit
>> constant while the register "r1" is with 64-bit width so I feel it is enough. For
>> 32-bit situation, we need new register set, some thing like "w1 = tx_port".
>
> I just push a patch (similar to your suggestion below but without
> assertion) which still has
> "ll" suffix for the constant, but no "ll" suffix for symbols. The
> reason is that  we use "ll"
> in the asm printout to differentiate "r = 5"=>"mov r, 5" (32bit imm) and
> "r = 5ll" => "ld_imm64 r, 5ll" (64bit imm).
>
> Also, for long long constant, C standard does not allow space between
> value and "ll" and
> hence "567 ll" is not allowed to represent a number "567ll". Could you
> try to disallow
> 64bit immediate like "567 ll" as well in your patch?

Hi Y Song,

Thanks, patch updated, please review.

Changes since the initial version:

   * Addressed all comments on instruction unit tests.
     - Renamed test file to "insn-unit.s".
     - Removed unnecessary comments.
     - Added BPF_B tests for load and store.
     - Added BPF_K test for jmp and alu.
       (Noticed a seperate issue with unsigned compare then jump. Looks like BPF
        backend is alway printing the immediate as signed, I guess we need to
        change the operand types in instruction patterns.)
     - Adjusted constant load tests.
     - Interleaved the expected results with input insns.

   * Changed the disassembly output "ll" to "LL" in BPFInstPrinter.cpp.
     This is because I noticed MC parser only accept "LL" as long long suffix.
     It might be the reason documented here:

       https://www.securecoding.cert.org/confluence/pages/viewpage.action?pageId=19759250

-------------- next part --------------
A non-text attachment was scrubbed...
Name: bpf-asmparser-v2.patch
Type: text/x-patch
Size: 26633 bytes
Desc: not available
URL: <http://lists.iovisor.org/pipermail/iovisor-dev/attachments/20170911/a510b38d/attachment-0001.bin>


More information about the iovisor-dev mailing list