On 12/01/15 12:12:29, Richard Biener wrote: > On Mon, 30 Nov 2015, Gary Funck wrote: > > At this time, we would like to re-submit the UPC patches for comment > > with the goal of introducing these changes into GCC 6.0. > > First of all let me say that it is IMNSHO now too late for GCC 6.
I realize that stage 1 recently closed, and that if UPC were accepted for inclusion, it would be an exception. To offset potential risk, we perform weekly merges and run a large suite of tests and apps. on differing hosts/cpu architectures. We have also tried to follow the sorts of re-factoring and C++ changes made over the course of the last year/so. I'd just ask that the changes be given some further consideration for 6.0. > You claim bits in tree_base - are those bits really used for > all tree kinds? The qualifiers look type specific where > eventually FE specific flags in type-lang-specific parts could > have been used (yeah, there are no spare bits in tree_type_*). > Similar the _factor stuff should not be on all tree kinds. When we first started building the gupc branch, it was suggested that UPC be implemented as a separate language ala ObjC. In that case, we used "language bits". Over time, this approach fell out of favor, and we were asked to move everything into the C front-end and middle-end, making compilation contingent upon -fupc, which is the way it is now. Also, over the past couple of years, there has been work to minimize the number of bits used by tree nodes, so some additional changes were needed. The main change recommended to reduce tree space was moving the "layout factor" (blocking factor) out of the tree node, and using only two bits there, one bit for a relatively common case of 0, and the other for > 1. It was suggested that we use a hash table to map tree nodes to layout qualifiers for the case they are > 1. This necessitated using a garbage collected tree map, which unfortunately meant that tree nodes needed special garbage collection logic. It is worth noting that the "layout qualifier" is an integral constant, currently represented as a tree node reference. It might be possible to represent it as a "wide int" instead. I did give that a go once, but it rippled through the code making some things awkward. Perhaps not as awkward as a custom tree node GC routine; this could be re-visited. > I find the names used a bit unspecific, please consider > prefixing them with upc_ (esp. shared_flag may be confused > with the similar private_flag). When we previously asked for a review, it was noted that if the UPC bits were moved into what amounts to common/generic tree node fields that we should drop UPC_ or upc_ from the related node names and functions. That's what we did. There is some middle ground, for example, where only TYPE_SHARED_P() is renamed to UPC_SHARED_TYPE_P() and the rest remain as is. Since renames are straight forward, we can make any recommended changes quickly. Originally, we were keeping the door open for UPC++, but there are complications with generalizing C++ into a multi-node environment, and that idea has been tabled for now. Therefore, the current structure/implementation is C only, with most of the new front-end/middle-end logic under the c/ directory. > Are these and the new tree codes below living beyond the time > the frontend is in control? That is, do they need to survive > throughout the middle-end? I'm not sure where the line is drawn for the front-end and middle-end. After upc_genericize() runs (just before c_genericize()) all operations on tree nodes that are UPC-specific are lowered into operations on the internal representation of a pointer-to-shared and/or runtime calls that operate on the internal representation. The pointer-to-shared values/types still show up in the tree, but only as containers (pointers-to-shared are typically 2x the size of a regular "C" pointer). The places where SHARED_TYPE_P() is referenced in 'c/' and 'c-family/' are: c/c-convert.c c/c-objc-common.c c/c-upc-pts-ops.c c/c-parser.c c/c-typeck.c c/c-upc-low.c c/c-upc-lang.c c/c-decl.c c/c-upc.c c-family/c-common.c The places in the gcc top-level where SHARED_TYPE_P() is referenced are: convert.c explow.c fold-const.c function.c gimple-expr.c match.pd tree.c tree.h tree-sra.c The target-specific references are here: config/rs6000/rs6000.c config/i386/i386.c All of the references outside of c/ and c-family/ and tree.[ch] are to differentiate operations on UPC pointers-to-shared from regular "C" pointers. (Some/all of those references might be mitigated by defining new language hooks. We haven't looked into that.) It may be the case that in the current design, that only the "shared" bit is needed in the common (base?) tree node, as long as there is some way to record the additional "strict" and "relaxed" qualifiers, the "layout qualifier" (a tree reference to an integral constant), and the "THREADS scaled" bit. The main thing that would need to be checked is when/where debugging (DWARF) info. is generated. thanks, - Gary