On Tue, Sep 13, 2022 at 02:04:30PM -0700, Julian Brown wrote: > This patch implements OpenMP 5.0 "declare mapper" support for C++. > This hasn't been fully revised yet following previous review comments, > but I am including it in this series to demonstrate the new approach to > gimplifying map clauses after "declare mapper" instantiation. > > The "gimplify_scan_omp_clauses" function is still called twice: firstly > (before scanning the offload region's body) with SUPPRESS_GIMPLIFICATION > set to true. This sets up variables in the splay tree, etc. but does > not gimplify anything. > > Then, implicit "declare mappers" are instantiated after scanning the > region's body, then "gimplify_scan_omp_clauses" is called again, and > does the rest of its previous tasks -- builds struct sibling lists, > and gimplifies clauses. Then gimplify_adjust_omp_clauses is called, > and compilation continues. > > Does this approach seem OK?
As I wrote in https://gcc.gnu.org/pipermail/gcc-patches/2022-June/596444.html I don't see a reason for this 3 passes approach and it will be a maintainance problem. The reason why we have 2 passes approach is that we need to populate the splay trees based on explicit clauses before we gimplify the body, at which point we can look up for variables seen in the body those splay trees, either mark the explicit ones as seen or create new ones for implicit etc. And finally we need to adjust some explicit clauses based on that (that is what the main loop in gimplify_adjust_omp_clauses does) and add new clauses for implicit data sharing or mapping (that is done through splay tree traversal through gimplify_adjust_omp_clauses_1). We also need to gimplify expressions from the clauses somewhere, but due to the way the gimplifier works we don't care that much when exactly it is done, it can be done before the body is gimplified (for most clauses we do it there in gimplify_scan_omp_clauses), it can be done after the body is gimplified, the expressions from the clauses will be in either case gimplified into some gimple sequence that we'll place before the construct with its body. The only reason to have the gimplification done before and not after would be if the temporaries from the gimplification are then needed in the splay trees for the gimplification of the body. I'd strongly prefer if the gimplification APIs for all the constructs were just gimplify_scan_omp_clauses before processing the body and gimplify_adjust_omp_clauses after doing so, not some extra APIs. If there is a strong reason for 3 or more passes on say a subset of clauses, either gimplify_scan_omp_clauses or gimplify_adjust_omp_clauses can do more than one loop over the clauses, but those secondary loops ideally should be enabled only when needed (e.g. see the gimplify_adjust_omp_clauses has_inscan_reductions guarded loop at the end) and should only process clauses they strictly have to. Conceptually, there is no reason why e.g. the gimplification of the explicit map clauses can't be done in gimplify_adjust_omp_clauses rather than in gimplify_scan_omp_clauses. What needs to happen in gimplify_scan_omp_clauses is just what affects the gimplification of the body. Does sorting of the map clause affect it? I don't think so. Does declare mapper processing of the explicit map clauses affect it? I very much hope it doesn't, but I'm afraid I don't remember all the declare mapper restrictions and discussions. Can declare mapper e.g. try to map an unrelated variable in addition to say parts of the structure? If yes, then it could affect the gimplification of the body, say struct S { int s, t; }; extern int y; #pragma omp declare mapper (struct S s) map (to: s.s) map (to: y) #pragma omp target defaultmap(none:scalar) map(tofrom: x) { int x = s.s + y; } because if we do process declare mapper before the gimplification of the body, then y would be explicitly mapped, but if we don't, it wouldn't and it should be rejected. But in that case we'd be in big trouble with implicit mappings using that same declare mapper. Because it would then be significant in which order we process the vars during gimplification of the body and whether we process declare mapper right away at that point or only at the end etc. We have the "List items in map clauses on the declare mapper directive may only refer to the declared variable var and entities that could be referenced by a procedure defined at the same location." restriction but not sure that says the above isn't valid. So probably it needs to be discussed in omp-lang. If the agreement is that declare mapper for explicit map clauses needs to be done before processing the body and declare mapper for implicit map clauses can be deferred to the end, then yes, we need to handle declare mapper twice, but it can be done say in a secondary loop of gimplify_scan_omp_clauses guarded on at least one of the map clauses needs declare mapper processing or something similar that can be quickly determined on the first loop and can use a helper function that gimplify_scan_omp_clauses also uses. Jakub
