From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14004 invoked by alias); 2 Jul 2011 05:47:47 -0000 Received: (qmail 13994 invoked by uid 22791); 2 Jul 2011 05:47:46 -0000 X-SWARE-Spam-Status: No, hits=-0.3 required=5.0 tests=AWL,BAYES_50,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from intrepid.intrepid.com (HELO mail.intrepid.com) (74.95.8.113) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 02 Jul 2011 05:47:31 +0000 Received: from screamer.local (screamer.local [10.10.1.2]) by mail.intrepid.com (8.13.8/8.13.8) with ESMTP id p625lSAb022218 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 1 Jul 2011 22:47:28 -0700 Received: from screamer.local (screamer.local [127.0.0.1]) by screamer.local (8.14.4/8.14.4) with ESMTP id p625lRL7003298; Fri, 1 Jul 2011 22:47:27 -0700 Received: (from gary@localhost) by screamer.local (8.14.4/8.14.4/Submit) id p625lRxW003296; Fri, 1 Jul 2011 22:47:27 -0700 Date: Sat, 02 Jul 2011 05:47:00 -0000 From: Gary Funck To: "Joseph S. Myers" Cc: Gcc Patches , Richard Henderson , Tom Tromey Subject: Re: RFC: [GUPC] UPC-related front-end changes Message-ID: <20110702054727.GA1172@intrepid.com> References: <20100708061704.GA8748@intrepid.com> <20110701183145.GD6262@intrepid.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2011-07/txt/msg00123.txt.bz2 On 07/01/11 19:28:34, Joseph S. Myers wrote: > On Fri, 1 Jul 2011, Gary Funck wrote: > GF: * Most of the #ifdef conditionals have been removed. Some target macros > GF: have been defined and documented in tm.texi. We still have some questions > > [...] > I looked at the first of the documented macros, and it doesn't seem to be > a target macro at all, being defined by configure rather than in tm.h. > Configure-defined macros don't go in tm.texi. But any that are really > target macros need such a justification, and it's still preferable to > define configure-defined macros to 1 or 0 (rather than defined or > undefined) and test them in "if" statements not #if. I was uncertain where/how to document the macros used to configure UPC. (Your explanation in the [...] above was helpful in answering the technical questions that I had in that regard, thanks.) The patch submitted for review did document (as target macros) switches that are set by the 'configure' script. Whether they should be #defines/not, I'm not sure, and whether they are target specific I wasn't sure. Hopefully, the following description will provide enough background to figure out the best way to handle these configuration items. The recent changes were motivated by your original suggestion: * It appears you have a large number of #ifdef conditionals with no obvious reason for the macros being conditionally defined or not defined. The use of conditional compilation like this is deprecated for new code. If something may vary from target to target, use target hooks, not macros, and document them in tm.texi (if they are in fact documented there in something outside this patch, perhaps you need to post that other patch). Conditions using "if" are strongly preferred to those using "#if" whenever possible. In the version of GUPC that your comments above relate to, the #define's were primarily in config/upc_conf.h: http://gcc.gnu.org/viewcvs/branches/gupc/gcc/config/upc-conf.h?revision=173527&view=markup&pathrev=173545 Some of those definitions, which list the names of UPC runtime functions now in upc/rts-names.h: http://gcc.gnu.org/viewcvs/branches/gupc/gcc/upc/upc-rts-names.h?revision=173837&view=markup The definitions that are specific to the representation of the UPC pointer-to-shared (PTS) type that has been configured by the user were swept under this corner of the rug in upc/upc-pts.h: http://gcc.gnu.org/viewcvs/branches/gupc/gcc/upc/upc-pts.h?revision=173837&view=markup The reason that so many #ifdef's and #defines remain is that I wasn't sure about the best method of parameterizing those values to avoid the use of conditional compilation. For example, should a small struct be defined that has fields for the configuration-specific values? The switching between the files that implement the packed representation of a pointer-to-shared and those that implemented the struct representation used to be done in the UPC Makefile fragment Make-lang.in (lines 61 and 62): http://gcc.gnu.org/viewcvs/branches/gupc/gcc/upc/Make-lang.in?revision=169939&view=markup&pathrev=173545 In that version of GUPC only one of two files upc-pts-packed.c or upc-pts-struct.c was compiled, depending upon the configuration switch that selected between the two representations. Both files defined the same exported functions. In an effort to make sure that both implementations are compiled, a table of PTS operations is defined in the current version of GUPC in upc-pts.h: http://gcc.gnu.org/viewcvs/branches/gupc/gcc/upc/upc-pts.h?revision=173837&view=markup The table of representation-specific functions is filled in with either the packed PTS routines or the struct representation routines when the UPC compiler is initialized. Where the need for this PTS representation specific logic begins is controlled by the following UPC configure options: --with-upc-pts={struct,packed} choose the representation of a UPC pointer-to-shared --with-upc-pts-vaddr-order={last,first} choose position of the address field in UPC pointer-to-shared representation --with-packed-upc-pts-bits=phase,thread,vaddr choose bit distribution in packed UPC pointer-to-shared representation The set of configuration choices looks like this: PTS Representation Packed Vaddr: {first, last} field sizes: {phase,thread,vaddr} Struct Vaddr: {first, last} Although the PTS representation is not target-specific, it has some aspects of being target-specific in that the representation and layout of the PTS changes the binary representation of the PTS in the compiled program (and runtime), and some representations will perform better/worse on a specific target (for example, on some targets some UPC programs may run faster if vaddr is first. Generally, the 'struct PTS' is chosen because it provides the maximum range of the various fields (at the expense of using more space to represent the PTS). Based upon your suggestions/comments, it sounds like: 1. The PTS-related #define's set by 'configure' are not target macros, and should not be documented as such. 2. The PTS-related operations that are specific either to the packed or the struct PTS representation are not target hooks and should not be documented as such. 3. UPC_PTS_VADDR_FIRST currently defined in upc-pts.h: 83 #ifdef HAVE_UPC_PTS_VADDR_FIRST 84 #define UPC_PTS_VADDR_FIRST 1 85 #else 86 #define UPC_PTS_VADDR_FIRST 0 87 #endif /* HAVE_UPC_PTS_VADDR_FIRST */ should be set directly to 1 or 0 by 'configure', avoiding the need to test HAVE_UPC_PTS_VADDR_FIRST with an #ifdef. 4. The #define's in upc-pts.h that are derived based upon PTS-specific configuration settings: #ifdef HAVE_UPC_PTS_PACKED_REP 51 /* 'packed' UPC pointer-to-shared representation */ 52 #define UPC_PTS_SIZE 64 53 #else 54 /* 'struct' UPC pointer-to-shared representation */ 55 #define UPC_PTS_SIZE (LONG_TYPE_SIZE + POINTER_SIZE) 56 #define UPC_PTS_PHASE_SIZE (LONG_TYPE_SIZE/2) 57 #define UPC_PTS_THREAD_SIZE (LONG_TYPE_SIZE/2) 58 #define UPC_PTS_VADDR_SIZE POINTER_SIZE 59 #define UPC_PTS_PHASE_TYPE ((LONG_TYPE_SIZE == 64) ? "uint32_t" : "uint16_t") 60 #define UPC_PTS_THREAD_TYPE ((LONG_TYPE_SIZE == 64) ? "uint32_t" : "uint16_t") 61 #define UPC_PTS_VADDR_TYPE "char *" 62 #endif /* HAVE_UPC_PTS_PACKED_REP */ 63 64 #ifdef HAVE_UPC_PTS_VADDR_FIRST 65 #define UPC_PTS_PHASE_SHIFT 0 66 #define UPC_PTS_THREAD_SHIFT UPC_PTS_PHASE_SIZE 67 #define UPC_PTS_VADDR_SHIFT (UPC_PTS_PHASE_SIZE+UPC_PTS_THREAD_SIZE) 68 #else 69 #define UPC_PTS_PHASE_SHIFT (UPC_PTS_VADDR_SIZE+UPC_PTS_THREAD_SIZE) 70 #define UPC_PTS_THREAD_SHIFT UPC_PTS_VADDR_SIZE 71 #define UPC_PTS_VADDR_SHIFT 0 72 #endif 73 #if HOST_BITS_PER_WIDE_INT == HOST_BITS_PER_LONG 74 #define UPC_PTS_VADDR_MASK ((1L << UPC_PTS_VADDR_SIZE) - 1L) 75 #elif HOST_BITS_PER_WIDE_INT == HOST_BITS_PER_LONGLONG 76 #define UPC_PTS_VADDR_MASK ((1LL << UPC_PTS_VADDR_SIZE) - 1LL) 77 #else 78 #error Unexpected "HOST_BITS_PER_WIDE_INT" value. 79 #endif /* HOST_BITS_PER_WIDE_INT */ 80 #define UPC_PTS_THREAD_MASK ((1 << UPC_PTS_THREAD_SIZE) - 1) 81 #define UPC_PTS_PHASE_MASK ((1 << UPC_PTS_PHASE_SIZE) - 1) Should be moved into a 'struct' that is filled in with the UPC compiler is initialized. This can be done by the representation specific 'init' routine. Does that make sense? - Gary