[vim/vim] Unnecessary multiplications in backspace code (PR #14308)
zeertzjq
Problem: Unnecessary multiplications in backspace code, as
"col / ts * ts" is the same as "col - col % ts".
Solution: Change "col / ts * ts" to "col - col % ts". Adjust the loop
and the comments ins_bs() to be easier to understand. Update
tests to reset 'smarttab' properly.
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/14308
Commit Summary
- 50ec541 Unnecessary multiplications in backspace code
File Changes
(3 files)
- M src/edit.c (28)
- M src/indent.c (4)
- M src/testdir/test_edit.vim (37)
Patch Links:
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Marvin Renich
> Solution: Change "col / ts * ts" to "col - col % ts". Adjust the loop
> You can view, comment on, or merge this pull request online at:
>
> https://github.com/vim/vim/pull/14308
expected that any reasonable optimizing compiler would produce identical
code, and in fact, gcc -O2 on amd64 (linux) does for a very simple
source program comparing them:
-------- multmod.c --------
/* multmod compares int division and modulus
*
* To compile to assembler with some optimization: cc -S -O2 multmod.c
*/
#include <stdio.h>
int mult(int a, int b)
{
return a / b * b;
}
int mod(int a, int b)
{
return a - a % b;
}
int main(int argc, char* argv[])
{
int a = 76;
int b = 7;
int c;
c = mult(a, b);
printf("c = %d\n", c);
c = mod(a, b);
printf("c = %d\n", c);
}
--------
Examining the produced assembler, mult and mod are identical (only one
idivl, with the expected subl).
It is certainly possible that for the specific source you are changing,
the compiler does not recognize the opportunity for optimization, but
until you inspect the assembler for the original code, you will not
know. I did not actually compile to assembler and examine the code you
are changing, so I cannot say definitively.
Unless there is a reason (such a particular architecture that is giving
problems with not-good-enough optimizations), I would avoid source churn
for zero benefit.
...Marvin
Christian Brabandt
That seems like unneccessary optimizations at the risk of breaking something. Does this really gain us something?
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
zeertzjq
Yes, this also deduplicates code between +vartabs and -vartabs.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
zeertzjq
@zeertzjq pushed 1 commit.
- 384445d Unnecessary multiplications in backspace code
—
View it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
okay thanks!
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.