Re: RFC: Merge the GUPC branch into the GCC 4.8 trunk (patch 01 of 16)

2012-10-18 Thread Joseph S. Myers 
On Thu, 18 Oct 2012, Gary Funck wrote:

> GUPC (then called GCC/UPC) dates back to the GCC 2.7 and GCC 2.95 days.
> The GCC 2.7 based implementation was a prototype, and the GCC 2.95 version
> was a first attempt to implement a UPC compiler that fits within the GCC
> build framework.  At the time, we had discussions with Mark Mitchell,
> Mike Stump and perhaps others regarding the best method for introducing
> a UPC compiler into GCC.  The consensus recommendation at the time was to
> implement GCC/UPC as a language dialect, similar to Objective-C.
> Thus, much of the initial work was patterned after ObjC; it used
> stubs and built a distinct front-end, so that is what we did.

GCC coding style has moved on a long way since then, and GCC generally has 
been moving away from conditional compilation, from #if to if conditions, 
from conditionally building something in to making the choice when the 
compiler is run.

I don't think ObjC as a separate front end is a good idea now, and don't 
think UPC should be added that way.  I also still don't think making UPC 
into a mode of the C front end should be more than a day or two's work.

> My main issue with entertaining large re-structuring of GUPC 
> at the moment is that I would like to see if we can introduce
> GUPC into GCC 4.8.  As you point out, there are a lot of

If the concern is 4.8, I'd think the priority should be the changes to the 
language and target independent code - changes to handle whatever 
additions UPC may make to GENERIC / GIMPLE - which will need middle-end 
maintainer review and which are probably the highest-risk part of the 
changes.

> Also, specifying the flavor of PTS representation on the UPC
> compiler command line (for example) is a departure from the
> current method of configuring and invoking GUPC.  There is a

As noted above, use of #if is no longer generally good practice in GCC.  
Even "if (macro)", which might turn into "if (0)", is better than #if - at 
least it means all the code does get checked by the compiler, whichever 
configuration is built - but command-line options are certainly better.

> JSM: Adding $(C_STUB_OBJS) uses to random language makefiles is very 
> JSM: suspicious.  stub-objc.o isn't needed by non-C-family front ends.  Nor 
> JSM: should stub-upc.o be - although I think the whole approach of using stub 
> JSM: functions like that is wrong, as discussed.
> 
> I'm not fond of stubs or adding extra files to other front-ends,
> but those changes seemed necessary at the time to get things to link.

"seemed necessary at the time to get things to link" isn't sufficient 
reason.  You'd need to state what symbols, used in what files, were 
undefined.  But then the solution won't be to link in a stub file - it 
will be to link in real code, unconditionally.  Whatever additions you 
make to GENERIC / GIMPLE for UPC should be handled entirely by 
language-independent code - that is, code that is always linked in, for 
all languages, to handle those GENERIC / GIMPLE features (even if most 
front ends don't generate those features), rather than code only linked in 
for particular front ends.

> JSM: The language-independent 
> JSM: compiler should be using language-independent interfaces to process 
> JSM: language-independent internal representations; if there are additions to 
> JSM: the language-independent IR to handle UPC, then the relevant code to 
> JSM: handle them should be built in unconditionally.
> 
> We followed the Objective C approach for better/worse.

ObjC does not require stubs to be linked into non-C-family front ends.  
Whatever code exists in the language-independent compiler to handle GIMPLE 
from the ObjC front end gets linked into all compilers without any need to 
link in any stubs.

> The few additional tree nodes needed for UPC are defined
> in a language dependent tree definition file.  The UPC-specific

Whatever those get gimplified to needs to be language-independent (that 
is, handled after gimplification entirely by language-independent parts of 
the compiler).

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: RFC: Merge the GUPC branch into the GCC 4.8 trunk (patch 01 of 16)

2012-10-18 Thread Gary Funck 
Joseph,

In this rather long reply, I have attempted to collect all your
recent feedback, and to provide a response where possible.

JSM: On Mon, 15 Oct 2012, Gary Funck wrote:
JSM: GF: Various UPC language related checks and operations
JSM: GF: are called in the "C" front-end and middle-end.
JSM: GF: To insure that these operations are defined,
JSM: GF: when linked with the other language front-ends
JSM: GF: and compilers, these functions are stub-ed,
JSM: GF: in a fashion similar to Objective C:
JSM: 
JSM: Is there a reason you chose this approach rather than the -fcilkplus 
JSM: approach of enabling an extension in the C front end given a command-line 
JSM: option?  (If you don't want to support e.g. the ObjC / UPC combination, 
JSM: you can always give an error in such cases.)  In general I think such 
JSM: conditionals are preferable to linking in stub variants of functions - and 
JSM: I'm sure people doing all-languages LTO bootstraps will appreciate not 
JSM: having to do link-time optimization of the language-independent parts of 
JSM: the compiler yet more times because of yet another binary like cc1, 
JSM: cc1plus, ... that links in much the same code.  The functions you stub out 
JSM: would then all start with assertions that they are only ever called in UPC 
JSM: mode - or if they are meant to be called in C mode but do nothing in that 
JSM: case, with appropriate checks that return early for C (if needed).
JSM:
JSM: On Mon, 15 Oct 2012, Gary Funck wrote:
JSM: GF: Back when we began to develop GUPC, it was recommended that we
JSM: GF: introduce the UPC capability as a language dialect, similar to
JSM: GF: Objective C.  That is the approach that we have taken.
JSM: 
JSM: Recommended where?  I think that approach has been a bad idea for a long 
JSM: time and the approach of building into cc1, as taken by the cilkplus 
JSM: patches, is better (and that really most objc/ code should be like 
JSM: c-family/, built once and linked into both cc1 and cc1plus, though in its 
JSM: present state that's much harder to achieve).
JSM:
JSM: GF: I agree that there is no de facto reason that cc1upc is built
JSM: GF: other than the fact we use a similar approach to Objective C.
JSM: GF: However, I think that re-working this aspect of how GUPC is
JSM: GF: implemented will require a fair amount of time/effort.  If we
JSM: 
JSM: I'd expect it to be a fairly straightforward rework (as would making ObjC 
JSM: a mode of cc1, if ObjC++ didn't exist).

GUPC (then called GCC/UPC) dates back to the GCC 2.7 and GCC 2.95 days.
The GCC 2.7 based implementation was a prototype, and the GCC 2.95 version
was a first attempt to implement a UPC compiler that fits within the GCC
build framework.  At the time, we had discussions with Mark Mitchell,
Mike Stump and perhaps others regarding the best method for introducing
a UPC compiler into GCC.  The consensus recommendation at the time was to
implement GCC/UPC as a language dialect, similar to Objective-C.
Thus, much of the initial work was patterned after ObjC; it used
stubs and built a distinct front-end, so that is what we did.

I will note that even back in those days that Mark indicated he
didn't particularly like the language dialect approach, but it
seemed the best way to go at the time.

For Clang UPC, we have built the UPC capability into the
Clang C/C++ compiler.  This just turned out to be an easier
way to go given Clang's build structure.

My main issue with entertaining large re-structuring of GUPC 
at the moment is that I would like to see if we can introduce
GUPC into GCC 4.8.  As you point out, there are a lot of
other things that we need to do to align the GUPC changes with
the GCC trunk.  Ideally, I'd like to see if we can phase those
changes; dealing with the critical changes now prior to the
merge and then implementing the rest of the changes over time.

Is that feasible or likely?

JSM: GF: Some UPC-specific configuration options are added to
JSM: GF: the top-level configure.ac and gcc/configure.ac scripts.
JSM: 
JSM: Any patch that includes new configure options should include the 
JSM: documentation for those options in install.texi.  Also please include 
JSM: ChangeLog entries with each patch submission, and a more detailed 
JSM: description of the changes involved and the rationale for them and 
JSM: implementation choices made.

OK, thanks.

JSM: The changes to darwin.c, darwin.h, i386.c, rs6000.c should definitely be 
JSM: given their own rationale and called to the attention of relevant target 
JSM: maintainers, possibly through extracting them into separate patches in the 
JSM: series with meaningful subject lines mentioning the relevant targets, not 
JSM: mixed into what's ostensibly a build-system patch.

OK.

JSM: The configure options need justification for their existence and (given 
JSM: that they exist) for working using #if, given that (as I said before, in 
JSM: the above referenced 2010 message, in 
JSM:

Re: RFC: Merge the GUPC branch into the GCC 4.8 trunk (patch 01 of 16)

2012-10-15 Thread Joseph S. Myers 
On Mon, 15 Oct 2012, Gary Funck wrote:

> 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.

As a patch submitter, it's your responsibility to make it easy for 
reviewers to review your patch.  The ChangeLogs provide a roadmap for the 
patch - showing what is changed where, at a glance.  They go alongside the 
plain text rationale, explaining at a higher level the structure and 
rationale for the changes and anything that might seem unclear as to why 
the patches work in a particular way.

Each patch submission's Subject: line should best include a brief 
description of that patch as well, not just a patch number.

Any changes that are not immediately and obviously inherently UPC-specific 
deserve specific explanation in the plain text rationale.  That certainly 
includes such things as the langhook change I mentioned, the target 
changes and the changes to non-C-family front-end Make-lang.in files.  In 
the case of the target changes, there should be a high-level explanation 
of how other target maintainers might determine whether changes to their 
targets might be appropriate for UPC, or how you determined that only 
those targets should be changed.

For changes developed over several years, reading through them in detail 
to prepare a ChangeLog is particularly valuable as it will show up places 
where there are spurious changes (such as those to whitespace) that may 
have resulted from code being changed and changed again in the course of 
development, but that are not needed for a clean patch submission.

I don't really think your division into 16 patches is a particularly good 
one - it separates things that should be together, and joins things that 
might better be separate.  Putting documentation in a separate patch from 
the things documented is always bad, for example - if you have new target 
hooks, put the .texi changes in the same patch that adds those hooks.  A 
better division might be:

* Target changes, split out per target.

* Changes to existing C front-end files (including c-family).

* Changes to any other front ends, split out by front end.

* New front-end files.

* Changes to the language and target independent compiler (including build 
system code).

* Library.

* Compiler testsuite.

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: RFC: Merge the GUPC branch into the GCC 4.8 trunk (patch 01 of 16)

2012-10-15 Thread Gary Funck 

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  
> (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

Re: RFC: Merge the GUPC branch into the GCC 4.8 trunk (patch 01 of 16)

2012-10-15 Thread Joseph S. Myers 
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  
(at least, this patch has a license notice in obsolete form, referring to 
GPLv2 and giving an FSF postal address instead of a URL).

The changes to darwin.c, darwin.h, i386.c, rs6000.c should definitely be 
given their own rationale and called to the attention of relevant target 
maintainers, possibly through extracting them into separate patches in the 
series with meaningful subject lines mentioning the relevant targets, not 
mixed into what's ostensibly a build-system patch.

The configure options need justification for their existence and (given 
that they exist) for working using #if, given that (as I said before, in 
the above referenced 2010 message, in 
 and 
), "if" 
conditionals are preferred to #if and it's better for users to be able to 
choose things when they run the compiler rather than only having it 
determined by the person building the compiler (so configure options 
determining defaults are better than configure options that force the 
compiler to behave in a particular way - even if the option affects 
runtime library builds, it's still better to have a command-line option 
and the configure option just affect the default, since then at least 
people can see how code generation changes with the option and 
potentially build multilibs with both variants).

In general, please make sure that you take account of previous feedback on 
previous GUPC patch submissions, and explain in your submissions how you 
have adjusted things for this feedback, or, if you are not making a 
previously requested change, why you consider the approach taken in your 
patch to be optimal.

The change to ACX_BUGURL in configure.ac is simply wrong.

Adding $(C_STUB_OBJS) uses to random language makefiles is very 
suspicious.  stub-objc.o isn't needed by non-C-family front ends.  Nor 
should stub-upc.o be - although I think the whole approach of using stub 
functions like that is wrong, as discussed.  The language-independent 
compiler should be using language-independent interfaces to process 
language-independent internal representations; if there are additions to 
the language-independent IR to handle UPC, then the relevant code to 
handle them should be built in unconditionally.

Front ends should not have their own Makefile.in if at all avoidable, only 
Make-lang.in - why do you have gcc/upc/Makefile.in?  Note that it says 
"Derived from objc/Makefile.in", which was removed in r37088 (2000-10-27).  
See what I said in my 2010 review about "careful review for whether it ... 
takes account of cleanups done in the past decade or so" - you really do 
need to read through the entirety of the code to be submitted, line by 
line, and check how it stands up to the conventions followed elsewhere in 
the compiler and to issues that get raised in reviews of other patches 
nowadays.  (But with rearrangement to be an option to the C front end, I 
expect you shouldn't need upc/Make-lang.in either.)

-- 
Joseph S. Myers
jos...@codesourcery.com

RFC: Merge the GUPC branch into the GCC 4.8 trunk (patch 01 of 16)

2012-10-15 Thread Gary Funck 
Attached, patch 01 of 16.

These are configure and 'make' related changes
that introduce UPC support into GCC.

As noted previously, these changes build the UPC front-end
as a 'cc1upc' binary in a fashion similar to Objective C.

There are also specific ABI adjustments made for passing
UPC pointers-to-shared (PTS) values for reasons of both efficiency
and because it simplifies the handling of PTS's if they
are targeted to a scalar mode (such as DImode or TImode).

Some UPC-specific configuration options are added to
the top-level configure.ac and gcc/configure.ac scripts.

- Gary
Configure and Make (patch 01 of 16)
---

configure.ac
contrib/gcc_update
gcc/c-family/stub-upc.c
gcc/c/Make-lang.in
gcc/config/darwin.c
gcc/config/darwin.h
gcc/config/i386/i386.c
gcc/config.in
gcc/config/rs6000/rs6000.c
gcc/configure.ac
gcc/cp/Make-lang.in
gcc/fortran/Make-lang.in
gcc/java/Make-lang.in
gcc/lto/Make-lang.in
gcc/Makefile.in
gcc/objc/Make-lang.in
gcc/upc/config-lang.in
gcc/upc/Makefile.in
gcc/upc/Make-lang.in
Makefile.def
Makefile.tpl

Index: configure.ac
===
--- configure.ac(.../trunk) (revision 192449)
+++ configure.ac(.../branches/gupc) (revision 192459)
@@ -166,6 +166,7 @@ target_libraries="target-libgcc \
target-boehm-gc \
${libgcj} \
target-libobjc \
+   target-libgupc \
target-libada \
target-libgo"
 
@@ -478,10 +479,8 @@ if test x$enable_libmudflap = x ; then
 esac
 fi
 
-# Disable libgomp on non POSIX hosted systems.
-if test x$enable_libgomp = x ; then
-# Enable libgomp by default on hosted POSIX systems.
-case "${target}" in
+posix_based_os="yes"
+case "${target}" in
 *-*-linux* | *-*-gnu* | *-*-k*bsd*-gnu | *-*-kopensolaris*-gnu)
;;
 *-*-netbsd* | *-*-freebsd* | *-*-openbsd* | *-*-dragonfly*)
@@ -491,9 +490,20 @@ if test x$enable_libgomp = x ; then
 *-*-darwin* | *-*-aix*)
;;
 *)
-   noconfigdirs="$noconfigdirs target-libgomp"
-   ;;
-esac
+posix_based_os="no"
+;;
+esac
+
+# Enable libgomp by default on POSIX hosted systems.
+if test x$enable_libgomp = x -a $posix_based_os = "no" ; then
+# Disable libgomp on non POSIX hosted systems.
+noconfigdirs="$noconfigdirs target-libgomp"
+fi
+
+# Enable libgupc by default on POSIX hosted systems.
+if test x$enable_libgupc = x -a $posix_based_os = "no" ; then
+# Disable libgupc on non POSIX hosted systems.
+noconfigdirs="$noconfigdirs target-libgupc"
 fi
 
 # Disable libatomic on unsupported systems.
@@ -1138,6 +1148,7 @@ if test "${build}" != "${host}" ; then
   GCJ_FOR_BUILD=${GCJ_FOR_BUILD-gcj}
   GFORTRAN_FOR_BUILD=${GFORTRAN_FOR_BUILD-gfortran}
   GOC_FOR_BUILD=${GOC_FOR_BUILD-gccgo}
+  GUPC_FOR_BUILD=${GUPC_FOR_BUILD-gupc}
   DLLTOOL_FOR_BUILD=${DLLTOOL_FOR_BUILD-dlltool}
   LD_FOR_BUILD=${LD_FOR_BUILD-ld}
   NM_FOR_BUILD=${NM_FOR_BUILD-nm}
@@ -1152,6 +1163,7 @@ else
   GCJ_FOR_BUILD="\$(GCJ)"
   GFORTRAN_FOR_BUILD="\$(GFORTRAN)"
   GOC_FOR_BUILD="\$(GOC)"
+  GUPC_FOR_BUILD="\$(GUPC)"
   DLLTOOL_FOR_BUILD="\$(DLLTOOL)"
   LD_FOR_BUILD="\$(LD)"
   NM_FOR_BUILD="\$(NM)"
@@ -1957,6 +1969,28 @@ case ,${enable_languages},:${enable_objc
 ;;
 esac
 
+AC_ARG_WITH([upc-pts],
+AS_HELP_STRING(
+[[--with-upc-pts=[{struct,packed}]]],
+ [choose the representation of a UPC pointer-to-shared]),
+[
+ case ,${enable_languages}, in
+   *,upc,) 
+ case "$withval" in
+   struct|packed)
+true
+ ;;
+   *)
+ AC_MSG_ERROR([$withval is an invalid option to --with-upc-pts])
+ ;;
+ esac
+ ;;
+   *)
+ AC_MSG_ERROR([--with-upc-pts supplied, but UPC language not enabled])
+ ;;
+ esac
+],[])
+
 # Disable libitm if we're not building C++
 case ,${enable_languages}, in
   *,c++,*) ;;
@@ -2901,6 +2935,48 @@ case "${target}" in
 ;;
 esac
 
+# UPC linker script check
+AC_MSG_CHECKING([for UPC link script support])
+AC_ARG_ENABLE(upc-link-script,
+AS_HELP_STRING(
+  [--enable-upc-link-script],
+  [enable UPC's use of a custom linker script;
+  this will define the UPC shared section as a no load section on
+  targets where this feature is supported (requires GNU LD)]),
+[
+  case $enableval in
+  yes | no) ;;
+  *)
+AC_MSG_ERROR([--enable-upc-link-script accepts only yes or no.])
+;;
+  esac
+],
+[
+  if test x${use_gnu_ld} != xno ; then
+case "$target" in
+  # disable linker script for Apple Mac OS X
+  *-*-darwin*)
+enable_upc_link_script=no
+;;
+  *)
+enable_upc_link_script=yes
+;;
+esac
+  else
+enable_upc_link_script=no
+  fi
+  if test "$enable_upc_link_script" = yes; then
+target_configargs="${target_configargs} --enable-upc-link-script"
+host_configargs="${host_configargs} --enable-u