On Sun, Nov 19, 2017 at 7:34 PM, Kito Cheng <kito.ch...@gmail.com> wrote: > > I prefer write more comment in the code instead of ChangeLog, I add > more comment in __muluw3. > btw, long times ago, Vladimir was told me[1] it should contain what is > done but not why it is done?
Yes, the ChangeLog entry only states what was changed. But the email message or the comments should mention why, so we know why we are reviewing the patch. Something like "gives a 5% performance increase on our application" would be a good reason to accept a patch. Otherwise the patch is a little confusing, since it isn't clear why you are making the change. But I can see that there will be fewer register saves/restores in a function that needs umul_ppmm, and that will lead to better register allocation results, which is likely to result in better performance for some application. > You are right, I guess I just substitute all multiplication before, > thanks you point out this :) > Updated patch: > > 2017-11-24 Kito Cheng <kito.ch...@gmail.com> > > * longlong.h [__riscv] (__umulsidi3): Define. > [__riscv] (umul_ppmm) Likewise. > [__riscv] (__muluw3) Likewise. > Yes, this looks good to me. The placement of the backslashes to continue macro lines is different in the patch Palmer submitted, but that isn't important, just made it a little harder to compare the two patches. Jim
