To: vim_dev@googlegroups.com Subject: Patch 8.2.2403 Fcc: outbox From: Bram Moolenaar Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ------------ Patch 8.2.2403 Problem: Vim9: profiling if/elseif/endif not correct. Solution: Add profile instructions. Fix that "elseif" was wrong. Files: src/vim9compile.c, src/testdir/test_profile.vim, src/testdir/test_vim9_script.vim, src/testdir/test_vim9_disassemble.vim *** ../vim-8.2.2402/src/vim9compile.c 2021-01-24 13:34:15.007739955 +0100 --- src/vim9compile.c 2021-01-24 17:37:40.652609577 +0100 *************** *** 44,49 **** --- 44,50 ---- */ typedef struct { int is_seen_else; + int is_seen_skip_not; // a block was unconditionally executed int is_had_return; // every block ends in :return int is_if_label; // instruction idx at IF or ELSEIF endlabel_T *is_end_label; // instructions to set end label *************** *** 2098,2110 **** may_generate_prof_end(cctx_T *cctx, int prof_lnum) { if (cctx->ctx_profiling && prof_lnum >= 0) - { - int save_lnum = cctx->ctx_lnum; - - cctx->ctx_lnum = prof_lnum; generate_instr(cctx, ISN_PROF_END); - cctx->ctx_lnum = save_lnum; - } } #endif --- 2099,2105 ---- *************** *** 6735,6740 **** --- 6730,6747 ---- else scope->se_u.se_if.is_if_label = -1; + #ifdef FEAT_PROFILE + if (cctx->ctx_profiling && cctx->ctx_skip == SKIP_YES + && skip_save != SKIP_YES) + { + // generated a profile start, need to generate a profile end, since it + // won't be done after returning + cctx->ctx_skip = SKIP_NOT; + generate_instr(cctx, ISN_PROF_END); + cctx->ctx_skip = SKIP_YES; + } + #endif + return p; } *************** *** 6758,6763 **** --- 6765,6789 ---- if (!cctx->ctx_had_return) scope->se_u.se_if.is_had_return = FALSE; + if (cctx->ctx_skip == SKIP_NOT) + { + // previous block was executed, this one and following will not + cctx->ctx_skip = SKIP_YES; + scope->se_u.se_if.is_seen_skip_not = TRUE; + } + if (scope->se_u.se_if.is_seen_skip_not) + { + // A previous block was executed, skip over expression and bail out. + // Do not count the "elseif" for profiling. + #ifdef FEAT_PROFILE + if (cctx->ctx_profiling && ((isn_T *)instr->ga_data)[instr->ga_len - 1] + .isn_type == ISN_PROF_START) + --instr->ga_len; + #endif + skip_expr_cctx(&p, cctx); + return p; + } + if (cctx->ctx_skip == SKIP_UNKNOWN) { if (compile_jump_to_end(&scope->se_u.se_if.is_end_label, *************** *** 6771,6777 **** --- 6797,6813 ---- // compile "expr"; if we know it evaluates to FALSE skip the block CLEAR_FIELD(ppconst); if (cctx->ctx_skip == SKIP_YES) + { cctx->ctx_skip = SKIP_UNKNOWN; + #ifdef FEAT_PROFILE + if (cctx->ctx_profiling) + { + // the previous block was skipped, need to profile this line + generate_instr(cctx, ISN_PROF_START); + instr_count = instr->ga_len; + } + #endif + } if (compile_expr1(&p, cctx, &ppconst) == FAIL) { clear_ppconst(&ppconst); *************** *** 6829,6835 **** scope->se_u.se_if.is_had_return = FALSE; scope->se_u.se_if.is_seen_else = TRUE; ! if (scope->se_skip_save != SKIP_YES) { // jump from previous block to the end, unless the else block is empty if (cctx->ctx_skip == SKIP_UNKNOWN) --- 6865,6891 ---- scope->se_u.se_if.is_had_return = FALSE; scope->se_u.se_if.is_seen_else = TRUE; ! #ifdef FEAT_PROFILE ! if (cctx->ctx_profiling) ! { ! if (cctx->ctx_skip == SKIP_NOT ! && ((isn_T *)instr->ga_data)[instr->ga_len - 1] ! .isn_type == ISN_PROF_START) ! // the previous block was executed, do not count "else" for profiling ! --instr->ga_len; ! if (cctx->ctx_skip == SKIP_YES && !scope->se_u.se_if.is_seen_skip_not) ! { ! // the previous block was not executed, this one will, do count the ! // "else" for profiling ! cctx->ctx_skip = SKIP_NOT; ! generate_instr(cctx, ISN_PROF_END); ! generate_instr(cctx, ISN_PROF_START); ! cctx->ctx_skip = SKIP_YES; ! } ! } ! #endif ! ! if (!scope->se_u.se_if.is_seen_skip_not && scope->se_skip_save != SKIP_YES) { // jump from previous block to the end, unless the else block is empty if (cctx->ctx_skip == SKIP_UNKNOWN) *************** *** 6884,6889 **** --- 6940,6956 ---- } // Fill in the "end" label in jumps at the end of the blocks. compile_fill_jump_to_end(&ifscope->is_end_label, cctx); + + #ifdef FEAT_PROFILE + // even when skipping we count the endif as executed, unless the block it's + // in is skipped + if (cctx->ctx_profiling && cctx->ctx_skip == SKIP_YES + && scope->se_skip_save != SKIP_YES) + { + cctx->ctx_skip = SKIP_NOT; + generate_instr(cctx, ISN_PROF_START); + } + #endif cctx->ctx_skip = scope->se_skip_save; // If all the blocks end in :return and there is an :else then the *************** *** 8005,8011 **** { // beyond the last line #ifdef FEAT_PROFILE ! may_generate_prof_end(&cctx, prof_lnum); #endif break; } --- 8072,8079 ---- { // beyond the last line #ifdef FEAT_PROFILE ! if (cctx.ctx_skip != SKIP_YES) ! may_generate_prof_end(&cctx, prof_lnum); #endif break; } *************** *** 8023,8029 **** } #ifdef FEAT_PROFILE ! if (cctx.ctx_profiling && cctx.ctx_lnum != prof_lnum) { may_generate_prof_end(&cctx, prof_lnum); --- 8091,8098 ---- } #ifdef FEAT_PROFILE ! if (cctx.ctx_profiling && cctx.ctx_lnum != prof_lnum && ! cctx.ctx_skip != SKIP_YES) { may_generate_prof_end(&cctx, prof_lnum); *** ../vim-8.2.2402/src/testdir/test_profile.vim 2021-01-24 12:53:30.784247042 +0100 --- src/testdir/test_profile.vim 2021-01-24 17:29:37.766083938 +0100 *************** *** 100,138 **** endfunc func Test_profile_func_with_ifelse() let lines =<< trim [CODE] ! func! Foo1() if 1 ! let x = 0 elseif 1 ! let x = 1 else ! let x = 2 endif ! endfunc ! func! Foo2() if 0 ! let x = 0 elseif 1 ! let x = 1 else ! let x = 2 endif ! endfunc ! func! Foo3() if 0 ! let x = 0 elseif 0 ! let x = 1 else ! let x = 2 endif ! endfunc call Foo1() call Foo2() call Foo3() [CODE] call writefile(lines, 'Xprofile_func.vim') call system(GetVimCommand() \ . ' -es -i NONE --noplugin' --- 100,147 ---- endfunc func Test_profile_func_with_ifelse() + call Run_profile_func_with_ifelse('func', 'let', 'let') + call Run_profile_func_with_ifelse('def', 'var', '') + endfunc + + func Run_profile_func_with_ifelse(command, declare, assign) let lines =<< trim [CODE] ! XXX Foo1() if 1 ! DDD x = 0 elseif 1 ! DDD x = 1 else ! DDD x = 2 endif ! endXXX ! XXX Foo2() if 0 ! DDD x = 0 elseif 1 ! DDD x = 1 else ! DDD x = 2 endif ! endXXX ! XXX Foo3() if 0 ! DDD x = 0 elseif 0 ! DDD x = 1 else ! DDD x = 2 endif ! endXXX call Foo1() call Foo2() call Foo3() [CODE] + call map(lines, {k, v -> substitute(v, 'XXX', a:command, '') }) + call map(lines, {k, v -> substitute(v, 'DDD', a:declare, '') }) + call map(lines, {k, v -> substitute(v, 'AAA', a:assign, '') }) + call writefile(lines, 'Xprofile_func.vim') call system(GetVimCommand() \ . ' -es -i NONE --noplugin' *************** *** 157,167 **** call assert_equal('', lines[5]) call assert_equal('count total (s) self (s)', lines[6]) call assert_match('^\s*1\s\+.*\sif 1$', lines[7]) ! call assert_match('^\s*1\s\+.*\s let x = 0$', lines[8]) call assert_match( '^\s\+elseif 1$', lines[9]) ! call assert_match( '^\s\+let x = 1$', lines[10]) call assert_match( '^\s\+else$', lines[11]) ! call assert_match( '^\s\+let x = 2$', lines[12]) call assert_match('^\s*1\s\+.*\sendif$', lines[13]) call assert_equal('', lines[14]) call assert_equal('FUNCTION Foo2()', lines[15]) --- 166,176 ---- call assert_equal('', lines[5]) call assert_equal('count total (s) self (s)', lines[6]) call assert_match('^\s*1\s\+.*\sif 1$', lines[7]) ! call assert_match('^\s*1\s\+.*\s \(let\|var\) x = 0$', lines[8]) call assert_match( '^\s\+elseif 1$', lines[9]) ! call assert_match( '^\s\+\(let\|var\) x = 1$', lines[10]) call assert_match( '^\s\+else$', lines[11]) ! call assert_match( '^\s\+\(let\|var\) x = 2$', lines[12]) call assert_match('^\s*1\s\+.*\sendif$', lines[13]) call assert_equal('', lines[14]) call assert_equal('FUNCTION Foo2()', lines[15]) *************** *** 171,181 **** call assert_equal('', lines[20]) call assert_equal('count total (s) self (s)', lines[21]) call assert_match('^\s*1\s\+.*\sif 0$', lines[22]) ! call assert_match( '^\s\+let x = 0$', lines[23]) call assert_match('^\s*1\s\+.*\selseif 1$', lines[24]) ! call assert_match('^\s*1\s\+.*\s let x = 1$', lines[25]) call assert_match( '^\s\+else$', lines[26]) ! call assert_match( '^\s\+let x = 2$', lines[27]) call assert_match('^\s*1\s\+.*\sendif$', lines[28]) call assert_equal('', lines[29]) call assert_equal('FUNCTION Foo3()', lines[30]) --- 180,190 ---- call assert_equal('', lines[20]) call assert_equal('count total (s) self (s)', lines[21]) call assert_match('^\s*1\s\+.*\sif 0$', lines[22]) ! call assert_match( '^\s\+\(let\|var\) x = 0$', lines[23]) call assert_match('^\s*1\s\+.*\selseif 1$', lines[24]) ! call assert_match('^\s*1\s\+.*\s \(let\|var\) x = 1$', lines[25]) call assert_match( '^\s\+else$', lines[26]) ! call assert_match( '^\s\+\(let\|var\) x = 2$', lines[27]) call assert_match('^\s*1\s\+.*\sendif$', lines[28]) call assert_equal('', lines[29]) call assert_equal('FUNCTION Foo3()', lines[30]) *************** *** 185,195 **** call assert_equal('', lines[35]) call assert_equal('count total (s) self (s)', lines[36]) call assert_match('^\s*1\s\+.*\sif 0$', lines[37]) ! call assert_match( '^\s\+let x = 0$', lines[38]) call assert_match('^\s*1\s\+.*\selseif 0$', lines[39]) ! call assert_match( '^\s\+let x = 1$', lines[40]) call assert_match('^\s*1\s\+.*\selse$', lines[41]) ! call assert_match('^\s*1\s\+.*\s let x = 2$', lines[42]) call assert_match('^\s*1\s\+.*\sendif$', lines[43]) call assert_equal('', lines[44]) call assert_equal('FUNCTIONS SORTED ON TOTAL TIME', lines[45]) --- 194,204 ---- call assert_equal('', lines[35]) call assert_equal('count total (s) self (s)', lines[36]) call assert_match('^\s*1\s\+.*\sif 0$', lines[37]) ! call assert_match( '^\s\+\(let\|var\) x = 0$', lines[38]) call assert_match('^\s*1\s\+.*\selseif 0$', lines[39]) ! call assert_match( '^\s\+\(let\|var\) x = 1$', lines[40]) call assert_match('^\s*1\s\+.*\selse$', lines[41]) ! call assert_match('^\s*1\s\+.*\s \(let\|var\) x = 2$', lines[42]) call assert_match('^\s*1\s\+.*\sendif$', lines[43]) call assert_equal('', lines[44]) call assert_equal('FUNCTIONS SORTED ON TOTAL TIME', lines[45]) *** ../vim-8.2.2402/src/testdir/test_vim9_script.vim 2021-01-22 22:06:51.636529831 +0100 --- src/testdir/test_vim9_script.vim 2021-01-24 17:40:07.836177328 +0100 *************** *** 1741,1747 **** CheckDefFailure(['elseif true'], 'E582:') CheckDefFailure(['else'], 'E581:') CheckDefFailure(['endif'], 'E580:') ! CheckDefFailure(['if true', 'elseif xxx'], 'E1001:') CheckDefFailure(['if true', 'echo 1'], 'E171:') enddef --- 1741,1747 ---- CheckDefFailure(['elseif true'], 'E582:') CheckDefFailure(['else'], 'E581:') CheckDefFailure(['endif'], 'E580:') ! CheckDefFailure(['if g:abool', 'elseif xxx'], 'E1001:') CheckDefFailure(['if true', 'echo 1'], 'E171:') enddef *** ../vim-8.2.2402/src/testdir/test_vim9_disassemble.vim 2021-01-24 13:34:15.007739955 +0100 --- src/testdir/test_vim9_disassemble.vim 2021-01-24 17:41:55.251864917 +0100 *************** *** 1857,1864 **** '\d PROFILE START line 1\_s*' .. '\d PUSHS "profiled"\_s*' .. '\d ECHO 1\_s*' .. - '\d PROFILE END\_s*' .. 'return "done"\_s*' .. '\d PROFILE START line 2\_s*' .. '\d PUSHS "done"\_s*' .. '\d RETURN\_s*' .. --- 1857,1864 ---- '\d PROFILE START line 1\_s*' .. '\d PUSHS "profiled"\_s*' .. '\d ECHO 1\_s*' .. 'return "done"\_s*' .. + '\d PROFILE END\_s*' .. '\d PROFILE START line 2\_s*' .. '\d PUSHS "done"\_s*' .. '\d RETURN\_s*' .. *** ../vim-8.2.2402/src/version.c 2021-01-24 15:25:50.749300869 +0100 --- src/version.c 2021-01-24 17:30:41.725881588 +0100 *************** *** 752,753 **** --- 752,755 ---- { /* Add new patch number below this line */ + /**/ + 2403, /**/ -- hundred-and-one symptoms of being an internet addict: 226. You sit down at the computer right after dinner and your spouse says "See you in the morning." /// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net \\\ /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\ \\\ an exciting new programming language -- http://www.Zimbu.org /// \\\ help me help AIDS victims -- http://ICCF-Holland.org ///