On Sat, Mar 25, 2023 at 5:20 AM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote: > > On Fri, 2023-03-24 at 20:23 -0700, Stanislav Fomichev wrote: > > On 03/25, Eduard Zingerman wrote: > > > This is a follow up for RFC [1]. It migrates a first batch of 38 > > > verifier/*.c tests to inline assembly and use of ./test_progs for > > > actual execution. The migration is done by a python script (see [2]). > > > > Jesus Christ, 43 patches on a Friday night (^_^) > > Unless I get bored on the weekend, will get to them Monday morning > > (or unless Alexei/Andrii beat me to it; I see they were commenting > > on the RFC). > > Alexei and Andrii wanted this to be organized like one patch-set with > patch per migrated file. I actually found the side-by-side comparison > process to be quite painful, took me ~1.5 days to go through all > migrated files. So, I can split this in smaller batches if that would > make others life easier. > > Also the last patch: > > selftests/bpf: verifier/xdp_direct_packet_access.c converted to inline assembly > > was blocked because it is too large. I'll split it in two in the v2 > (but will wait for some feedback first). Oh, sorry, I was joking and didn't mean this as a call to split it up. Was mostly trying to set the expectation that I'll be slacking off on the weekend :-) (plus it seems like Alexei/Andrii would like a take a look anyway) > [...] > > > Are those '\' at the end required? Can we do regular string coalescing > > like the following below? > > > > asm volatile( > > "r2 = *(u32*)(r1 + %[xdp_md_data]);" > > "r3 = *(u32*)(r1 + %[xdp_md_data_end]);" > > "r1 = r2;" > > ... > > ); > > > > Or is asm directive somehow special? > > Strings coalescing would work, I updated the script to support both > modes, here is an example of verifier/and.c converted this way > (caution, that test missuses BPF_ST_MEM and has a wrong jump, the > translation is fine): > > https://gist.github.com/eddyz87/8725b9140098e1311ca5186c6ee73855 > > It was my understanding from the RFC feedback that this "lighter" way > is preferable and we already have some tests written like that. > Don't have a strong opinion on this topic. Ack, I'm obviously losing a bunch of context here :-( I like coalescing better, but if the original suggestion was to use this lighter way, I'll keep that in mind while reviewing. > Pros for '\': > - it indeed looks lighter; > - labels could be "inline", like in the example above "l0_%=: r0 = 0;". > Cons for '\': > - harder to edit (although, at-least for emacs, I plan to solve this > using https://github.com/twlz0ne/separedit.el); > - no syntax highlighting for comments. > > Pros for string coalescing: > - easier to edit w/o special editor support; > - syntax highlighting for comments. > Cons for string coalescing: > - a bit harder to read; > - "inline" labels look super weird, so each labels takes a full line. > > [...]
