As a recent committer to GCC, I am always surprised to see the content of ChangeLog entries and commit messages.
I tend to find GCC ChangeLog entries useless. For example, the more recent ChangeLog entry in gcc/ChangeLog is: | 2007-11-30 Jan Hubicka <[EMAIL PROTECTED]> | | * ggc-common.c (dump_ggc_loc_statistics): Reset ggc_force_collect | flag. How could a newcomer guess why the gcc_force_collect flag needs to be reset? Jan posted a useful explanation on gcc-patches, but finding it by searching the mailing-list is not practical and it is not coupled with the checkin. Let's look at the corresponding svn log message, which can be found with "svn blame" if a particular line needs to be pinpointed: | r130560 | hubicka | 2007-12-01 22:06:31 +0100 (Sat, 01 Dec 2007) | 4 lines | | * ggc-common.c (dump_ggc_loc_statistics): Reset ggc_force_collect | flag. Ok, same information is mirrored here, not useful. Let's look at the change itself then: | Index: gcc/ChangeLog | =================================================================== | --- gcc/ChangeLog (revision 130559) | +++ gcc/ChangeLog (revision 130560) | @@ -1,3 +1,8 @@ | +2007-11-30 Jan Hubicka <[EMAIL PROTECTED]> | + | + * ggc-common.c (dump_ggc_loc_statistics): Reset ggc_force_collect | + flag. | + | 2007-11-30 Seongbae Park <[EMAIL PROTECTED]> | | PR rtl-optimization/34171 | Index: gcc/ggc-common.c | =================================================================== | --- gcc/ggc-common.c (revision 130559) | +++ gcc/ggc-common.c (revision 130560) | @@ -1018,5 +1018,6 @@ | fprintf (stderr, "%-48s %10s %10s %10s %10s %10s\n", | "source location", "Garbage", "Freed", "Leak", "Overhead", "Times"); | fprintf (stderr, "-------------------------------------------------------\n") | ; | + ggc_force_collect = false; | #endif | } Ok, still the same information because of the ChangeLog diff, and we can see that the change shows that... gcc_force_collect is reset. Wow! Now, compare that with Jan's message on the list: | pre-ipa-mem-reports force GGC to be done at each invokation in order to | collect data on live memory references, but forgets to reset the flag | after done. This means that compiler continues GGCcollecting and works | slowly. This *is* the information I would expect to be present somewhere in GCC history. A clear and detailed information on why the change was necessary. Sure, in some case the checkin references a PR, but the PR often contains information of what didn't work before the change and the same information which is already repeated three times (ChangeLog, svn log and svn diff). Compare this to a typical commit in the Linux kernel: | commit b1812582ba94b5f377d5d3cec7646cc17d84e733 | Author: Joachim Fenkes <[EMAIL PROTECTED]> | Date: Fri Nov 30 16:19:41 2007 -0800 | | IB/ehca: Fix static rate if path faster than link | | The formula would yield -1 if the path is faster than the link, which | is wrong in a bad way (max throttling). Clamp to 0, which is the | correct value. | | Signed-off-by: Joachim Fenkes <[EMAIL PROTECTED]> | Signed-off-by: Roland Dreier <[EMAIL PROTECTED]> | | diff --git a/drivers/infiniband/hw/ehca/ehca_av.c b/drivers/infiniband/hw/ehca/ehca_av.c | index 453eb99..f7782c8 100644 | --- a/drivers/infiniband/hw/ehca/ehca_av.c | +++ b/drivers/infiniband/hw/ehca/ehca_av.c | @@ -76,8 +76,12 @@ int ehca_calc_ipd(struct ehca_shca *shca, int port, | | link = ib_width_enum_to_int(pa.active_width) * pa.active_speed; | | - /* IPD = round((link / path) - 1) */ | - *ipd = ((link + (path >> 1)) / path) - 1; | + if (path >= link) | + /* no need to throttle if path faster than link */ | + *ipd = 0; | + else | + /* IPD = round((link / path) - 1) */ | + *ipd = ((link + (path >> 1)) / path) - 1; | | return 0; | } What do we have here? A one-line high-level description identifying what has been done, a synthetic analysis of the problem cause and its solution, and the audit trail with the Signed-off-by lines (which in the Linux case is more important than in GCC as copyrights are not assigned to one entity). Linux doesn't use ChangeLog, but its history is much more useful to developers and casual observers than GCC one. And it could be done for GCC (with SVN) as well. Maybe we should consider dropping ChangeLogs and using better checkin messages. Sam
