On Sun, Mar 26, 2023 at 8:57 PM Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> wrote: > > On Sun, Mar 26, 2023 at 8:16 PM Andrii Nakryiko > <andrii.nakryiko@xxxxxxxxx> wrote: > > > > On Sat, Mar 25, 2023 at 6:19 PM Alexei Starovoitov > > <alexei.starovoitov@xxxxxxxxx> wrote: > > > > > > On Sat, Mar 25, 2023 at 9:16 AM Stanislav Fomichev <sdf@xxxxxxxxxx> wrote: > > > > > > > > > > > > > > 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. > > > > > > I still prefer the clean look of the tests, so I've applied this set. > > > > > > But I'm not going to insist that this is the only style developers > > > should use moving forward. > > > Whoever prefers "" style can use it in the future tests. > > > > Great, because I found out in practice that inability to add comments > > to the manually written asm code is a pretty big limitation. > > What do you mean by "inability" ? > The comments can be added. See verifier_and.c > r0 &= 0xFFFF1234; \ > /* Upper bits are unknown but AND above masks out 1 zero'ing > lower bits */\ > if w0 < 1 goto l0_%=; \ My bad. I remembered that there were problems with comments in Eduards previous revision and concluded that they don't work in this "\-delimited mode". Especially that online documentation for GCC or Clang didn't explicitly say that they support /* */ comments in asm blocks (as far as I could google that). So now I know it's possible, thanks. I still find it very tedious to do manually, so I appreciate the flexibility in allowing to do ""-delimited style for new programs. Just to explain where I'm coming from. I took one asm program I have locally and converted it to a new style. It was tedious with all the tab alignment. Then I realized that one comment block uses too long lines and wanted to use vim to reformat them, and that doesn't work with those '\' delimiters at the end (I didn't have such a problem with the original style). So that turned into more tedious work. So for something that needs iteration and adjustments, ""-delimited style gives more flexibility. See below for reference. '#' comments are dangerous, btw, they silently ignore everything till the very end of asm block. No warning or error, just wrong and incomplete asm is generated, unfortunately. SEC("?raw_tp") __failure __log_level(2) __msg("XXX") __naked int subprog_result_precise(void) { asm volatile ( "r6 = 3;" /* pass r6 through r1 into subprog to get it back as r0; * this whole chain will have to be marked as precise later */ "r1 = r6;" "call identity_subprog;" /* now use subprog's returned value (which is a * r6 -> r1 -> r0 chain), as index into vals array, forcing * all of that to be known precisely */ "r0 *= 4;" "r1 = %[vals];" "r1 += r0;" /* here r0->r1->r6 chain is forced to be precise and has to be * propagated back to the beginning, including through the * subprog call */ "r0 = *(u32 *)(r1 + 0);" "exit;" : : __imm_ptr(vals) : __clobber_common, "r6" ); } SEC("?raw_tp") __failure __log_level(2) __msg("XXX") __naked int subprog_result_precise2(void) { asm volatile (" \ r6 = 3; \ /* pass r6 through r1 into subprog to get it back as r0; \ * this whole chain will have to be marked as precise later \ */ \ r1 = r6; \ call identity_subprog; \ /* now use subprog's returned value (which is a \ * r6 -> r1 -> r0 chain), as index into vals array, forcing \ * all of that to be known precisely \ */ \ r0 *= 4; \ r1 = %[vals]; \ r1 += r0; \ /* here r0->r1->r6 chain is forced to be precise and has to be \ * propagated back to the beginning, including through the \ * subprog call \ */ \ r0 = *(u32 *)(r1 + 0); \ exit; \ " : : __imm_ptr(vals) : __clobber_common, "r6" ); }