On 10/15/12 21:59:34, Joseph S. Myers wrote: > On Mon, 15 Oct 2012, Gary Funck wrote: > > > Some UPC-specific configuration options are added to > > the top-level configure.ac and gcc/configure.ac scripts. > > Any patch that includes new configure options should include the > documentation for those options in install.texi. Also please include > ChangeLog entries with each patch submission, and a more detailed > description of the changes involved and the rationale for them and > implementation choices made. Also please see the review comments I sent > previously in <http://gcc.gnu.org/ml/gcc-patches/2010-07/msg02136.html> > (at least, this patch has a license notice in obsolete form, referring to > GPLv2 and giving an FSF postal address instead of a URL).
Joseph, Back in 2010, we posted an initial RFC: http://gcc.gnu.org/ml/gcc-patches/2010-07/msg00628.html which received some review feedback. We attempted to incorporate the suggested changes and posted a follow up about a year later. It didn't take a year to make the changes; other development progressed and it just took us that long to get back to implementing the feedback. http://gcc.gnu.org/ml/gcc-patches/2011-07/msg00081.html We tried to make the recommended changes, but it is possible that some were missed, or that there are other issues such as the ones that you point out that (still) need to be fixed. Regarding ChangeLog entries, I can add them to each change set, if that is needed. However, I was hoping to wait on that until the patches are generally approved, in principle. I will break out the ABI-related changes in darwin.c, darwin.h, i386.c, and rs6000.c for separate review along with an explanation. However, for this go around, I would like to patch all the current change sets, in case there is quick feedback that can be provided which will help us get things right more quickly. Regarding additional configure options, I will add justification per your recommendation. We did try to avoid #if's to the degree possible, and are open to suggestions on how to make further changes along those lines. I mentioned some of our remaining questions/issues here in our previous patch discussion. http://gcc.gnu.org/ml/gcc-patches/2011-07/msg00138.html Per your recommendation, we will mention previous feedback and how we adjusted our patch, in future submittals. Regarding gcc/upc/Makefile.in, as you note it is likely un-used and un-needed and a left over that dates back to the time that we derived the initial gcc/upc directory structure from the gcc/objc structure at that time. We will remove it. The ACX_BUGURL is a left over from earlier GUPC packaging and is unique to the current GUPC branch. We will remove or adjust that change. My apologies for not catching those spurious changes. Regarding making adjustments that agree with current recommendations on the GCC patches list, we do try to make those changes as we merge in the trunk. However, we don't have the overview or experience in various areas that you and other GCC contributors have. We hope that those changes can be pointed out during the review process. Some other changes may be a matter of weighing development time versus priority. For example, I could see a situation where the current method of doing things in GUPC (with #if's) is accepted in this phase with a plan to upgrade that approach in future patch. This is similar in approach to the way that the GCC infrastructure has improved over time. Some of these choices will be more obvious based on review/discussion. Thanks, - Gary
