On 02/12/15 10:45, Jakub Jelinek wrote:
On Fri, Nov 27, 2015 at 12:42:09PM +0100, Tom de Vries wrote:
--- a/gcc/omp-low.c
+++ b/gcc/omp-low.c
@@ -1366,10 +1366,12 @@ build_sender_ref (tree var, omp_context *ctx)
    return build_sender_ref ((splay_tree_key) var, ctx);
  }

-/* Add a new field for VAR inside the structure CTX->SENDER_DECL.  */
+/* Add a new field for VAR inside the structure CTX->SENDER_DECL.  If
+   BASE_POINTERS_RESTRICT, declare the field with restrict.  */

  static void
-install_var_field (tree var, bool by_ref, int mask, omp_context *ctx)
+install_var_field_1 (tree var, bool by_ref, int mask, omp_context *ctx,
+                    bool base_pointers_restrict)

Ugh, why the renaming?  Just use default argument:
                bool base_pointers_restrict = false

+/* As install_var_field_1, but with base_pointers_restrict == false.  */
+
+static void
+install_var_field (tree var, bool by_ref, int mask, omp_context *ctx)
+{
+  install_var_field_1 (var, by_ref, mask, ctx, false);
+}

And avoid the wrapper.

  /* Instantiate decls as necessary in CTX to satisfy the data sharing
-   specified by CLAUSES.  */
+   specified by CLAUSES.  If BASE_POINTERS_RESTRICT, install var field with
+   restrict.  */

  static void
-scan_sharing_clauses (tree clauses, omp_context *ctx)
+scan_sharing_clauses_1 (tree clauses, omp_context *ctx,
+                       bool base_pointers_restrict)

Likewise.

Otherwise LGTM,

Hi Jakub,

thanks for the review.

but I'm worried if this isn't related in any way to
PR68640 and might not make things worse.


AFAIU, they're sort of opposite cases:
- in the case of the PR, we add restrict in a function argument
  by accident
- in the case of this patch, we add restrict in a function argument
  by analysis

[ Btw, now that this patch (which exploits GOMP_MAP_FORCE_* mappings)
  is OK-ed, the patch "Fix oacc kernels default mapping for scalars" at
  https://gcc.gnu.org/ml/gcc-patches/2015-11/msg03334.html becomes more
  relevant, since that one ensures that scalars by default
  get the GOMP_MAP_FORCE_COPY mapping (rather than the incorrect
  GOMP_MAP_COPY) ]

Thanks,
- Tom

Reply via email to