[PATCH v4 0/3] [AArch64, OpenMP] Support SVE types with various OpenMP clauses and constructs

[PATCH v4 0/3] [AArch64, OpenMP] Support SVE types with various OpenMP clauses and constructs

[Tejas Belagod]

This series is based on a previous thread and review comments from RichardS and
Jakub upstream:

https://gcc.gnu.org/pipermail/gcc-patches/2025-March/677072.html

The changes suggested are cosmetic in nature. As suggested in the previous
series v3, this series doesn't include tests - they're going to be part of a
separate patch series.

Patches have been rebased, retested and bootstrapped on aarch64-linux-gcc.
OK for trunk?

Richard Sandiford (1):
  gomp: Various fixes for SVE types [PR101018]

Tejas Belagod (2):
  Add function to strip pointer type and get down to the actual pointee
    type.
  AArch64: Diagnose OpenMP offloading when SVE types involved.

 gcc/config/aarch64/aarch64-sve-builtins.cc | 37 ++++++++++++-
 gcc/fold-const.cc                          |  7 +++
 gcc/gimplify.cc                            | 60 ++++++++++++++++++----
 gcc/omp-low.cc                             |  2 +-
 gcc/poly-int.h                             | 19 +++++++
 gcc/target.h                               | 37 ++++++++++++-
 gcc/tree.h                                 |  9 ++++
 7 files changed, 157 insertions(+), 14 deletions(-)

-- 
2.25.1

[PATCH v4 1/3] gomp: Various fixes for SVE types [PR101018]

[Tejas Belagod]

From: Richard Sandiford <richard.sandiford@arm.com>

Various parts of the omp code checked whether the size of a decl
was an INTEGER_CST in order to determine whether the decl was
variable-sized or not.  If it was variable-sized, it was expected
to have a DECL_VALUE_EXPR replacement, as for VLAs.

This patch uses poly_int_tree_p instead, so that variable-length
SVE vectors are treated like constant-length vectors.  This means
that some structures become poly_int-sized, with some fields at
poly_int offsets, but we already have code to handle that.

An alternative would have been to handle the data via indirection
instead.  However, that's likely to be more complicated, and it
would contradict is_variable_sized, which already uses a check
for TREE_CONSTANT rather than INTEGER_CST.

gimple_add_tmp_var should probably not add a safelen of 1
for SVE vectors, but that's really a separate thing and might
be hard to test.

Co-authored-by: Tejas Belagod <tejas.belagod@arm.com>

gcc/
	PR middle-end/101018
	* poly-int.h (can_and_p): New function.
	* fold-const.cc (poly_int_binop): Use it to optimize BIT_AND_EXPRs
	involving POLY_INT_CSTs.
	* gimplify.cc (omp_notice_variable): Use poly_int_tree_p instead
	of INTEGER_CST when checking for constant-sized omp data.
	(gimplify_adjust_omp_clauses_1): Likewise.
	(gimplify_adjust_omp_clauses): Likewise.
	* omp-low.cc (scan_sharing_clauses): Likewise.
---
 gcc/fold-const.cc |  7 +++++++
 gcc/gimplify.cc   | 19 +++++++++----------
 gcc/omp-low.cc    |  2 +-
 gcc/poly-int.h    | 19 +++++++++++++++++++
 4 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc
index fef7a6cc48e..4687412f01f 100644
--- a/gcc/fold-const.cc
+++ b/gcc/fold-const.cc
@@ -1284,6 +1284,13 @@ poly_int_binop (poly_wide_int &res, enum tree_code code,
 	return false;
       break;
 
+    case BIT_AND_EXPR:
+      if (TREE_CODE (arg2) != INTEGER_CST
+	  || !can_and_p (wi::to_poly_wide (arg1), wi::to_wide (arg2),
+			 &res))
+	return false;
+      break;
+
     case BIT_IOR_EXPR:
       if (TREE_CODE (arg2) != INTEGER_CST
 	  || !can_ior_p (wi::to_poly_wide (arg1), wi::to_wide (arg2),
diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc
index 5bdd970f570..534c8457280 100644
--- a/gcc/gimplify.cc
+++ b/gcc/gimplify.cc
@@ -9125,7 +9125,8 @@ omp_notice_variable (struct gimplify_omp_ctx *ctx, tree decl, bool in_code)
       && (flags & (GOVD_SEEN | GOVD_LOCAL)) == GOVD_SEEN
       && DECL_SIZE (decl))
     {
-      if (TREE_CODE (DECL_SIZE (decl)) != INTEGER_CST)
+      tree size;
+      if (!poly_int_tree_p (DECL_SIZE (decl)))
 	{
 	  splay_tree_node n2;
 	  tree t = DECL_VALUE_EXPR (decl);
@@ -9136,16 +9137,14 @@ omp_notice_variable (struct gimplify_omp_ctx *ctx, tree decl, bool in_code)
 	  n2->value |= GOVD_SEEN;
 	}
       else if (omp_privatize_by_reference (decl)
-	       && TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (decl)))
-	       && (TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (decl))))
-		   != INTEGER_CST))
+	       && (size = TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (decl))))
+	       && !poly_int_tree_p (size))
 	{
 	  splay_tree_node n2;
-	  tree t = TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (decl)));
-	  gcc_assert (DECL_P (t));
-	  n2 = splay_tree_lookup (ctx->variables, (splay_tree_key) t);
+	  gcc_assert (DECL_P (size));
+	  n2 = splay_tree_lookup (ctx->variables, (splay_tree_key) size);
 	  if (n2)
-	    omp_notice_variable (ctx, t, true);
+	    omp_notice_variable (ctx, size, true);
 	}
     }
 
@@ -14402,7 +14401,7 @@ gimplify_adjust_omp_clauses_1 (splay_tree_node n, void *data)
       if ((gimplify_omp_ctxp->region_type & ORT_ACC) == 0)
 	OMP_CLAUSE_MAP_RUNTIME_IMPLICIT_P (clause) = 1;
       if (DECL_SIZE (decl)
-	  && TREE_CODE (DECL_SIZE (decl)) != INTEGER_CST)
+	  && !poly_int_tree_p (DECL_SIZE (decl)))
 	{
 	  tree decl2 = DECL_VALUE_EXPR (decl);
 	  gcc_assert (INDIRECT_REF_P (decl2));
@@ -15143,7 +15142,7 @@ gimplify_adjust_omp_clauses (gimple_seq *pre_p, gimple_seq body, tree *list_p,
 	  if (!DECL_P (decl))
 	    break;
 	  if (DECL_SIZE (decl)
-	      && TREE_CODE (DECL_SIZE (decl)) != INTEGER_CST)
+	      && !poly_int_tree_p (DECL_SIZE (decl)))
 	    {
 	      tree decl2 = DECL_VALUE_EXPR (decl);
 	      gcc_assert (INDIRECT_REF_P (decl2));
diff --git a/gcc/omp-low.cc b/gcc/omp-low.cc
index c36ae38cf8e..00bcf27fa47 100644
--- a/gcc/omp-low.cc
+++ b/gcc/omp-low.cc
@@ -1461,7 +1461,7 @@ scan_sharing_clauses (tree clauses, omp_context *ctx)
 	  else
 	    install_var_field (decl, false, 11, ctx);
 	  if (DECL_SIZE (decl)
-	      && TREE_CODE (DECL_SIZE (decl)) != INTEGER_CST)
+	      && !poly_int_tree_p (DECL_SIZE (decl)))
 	    {
 	      tree decl2 = DECL_VALUE_EXPR (decl);
 	      gcc_assert (INDIRECT_REF_P (decl2));
diff --git a/gcc/poly-int.h b/gcc/poly-int.h
index 7c8901a971a..77f78752e74 100644
--- a/gcc/poly-int.h
+++ b/gcc/poly-int.h
@@ -1906,6 +1906,25 @@ known_alignment (const poly_int<N, Ca> &a)
   return r & -r;
 }
 
+/* Return true if we can compute A & B at compile time, storing the
+   result in RES if so.  */
+
+template<unsigned int N, typename Ca, typename Cb, typename Cr>
+inline typename if_nonpoly<Cb, bool>::type
+can_and_p (const poly_int<N, Ca> &a, Cb b, Cr *result)
+{
+  /* Coefficients 1 and above must be a multiple of something greater
+     than ~B.  */
+  typedef POLY_INT_TYPE (Ca) int_type;
+  if (N >= 2)
+    for (unsigned int i = 1; i < N; i++)
+      if ((-(a.coeffs[i] & -a.coeffs[i]) & ~b) != int_type (0))
+	return false;
+  *result = a;
+  result->coeffs[0] &= b;
+  return true;
+}
+
 /* Return true if we can compute A | B at compile time, storing the
    result in RES if so.  */
 
-- 
2.25.1

[PATCH v4 2/3] Add function to strip pointer type and get down to the actual pointee type.

[Tejas Belagod]

Add a function to traverse down the pointer layers to the pointee type.

gcc/ChangeLog:
	* tree.h (strip_pointer_types): New.
---
 gcc/tree.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/gcc/tree.h b/gcc/tree.h
index 6f45359f103..77eddc4515c 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -5047,6 +5047,15 @@ strip_array_types (tree type)
   return type;
 }
 
+inline const_tree
+strip_pointer_types (const_tree type)
+{
+  while (POINTER_TYPE_P (type))
+    type = TREE_TYPE (type);
+
+  return type;
+}
+
 /* Desription of the reason why the argument of valid_constant_size_p
    is not a valid size.  */
 enum cst_size_error {
-- 
2.25.1

[PATCH v4 3/3] AArch64: Diagnose OpenMP offloading when SVE types involved.

[Tejas Belagod]

The target clause in OpenMP is used to offload loop kernels to accelarator
peripeherals.  target's 'map' clause is used to move data from and to the
accelarator.  When the data is SVE type, it may not be suitable because of
various reasons i.e. the two SVE targets may not agree on vector size or
some targets don't support variable vector size.  This makes SVE unsuitable
for use in OMP's 'map' clause.  This patch diagnoses all such cases and issues
an error where SVE types are not suitable.

Co-authored-by: Andrea Corallo <andrea.corallo@arm.com>

gcc/ChangeLog:

	* target.h (type_context_kind): Add new context kinds for target clauses.
	(omp_type_context): Query if the context is of OMP kind.
	* config/aarch64/aarch64-sve-builtins.cc (verify_type_context): Diagnose
	SVE types for a given OpenMP context.
	(omp_type_context): New.
	* gimplify.cc (omp_notice_variable): Diagnose implicitly-mapped SVE
	objects in OpenMP regions.
	(gimplify_scan_omp_clauses): Diagnose SVE types for various target
	clauses.
---
 gcc/config/aarch64/aarch64-sve-builtins.cc | 37 ++++++++++++++++++-
 gcc/gimplify.cc                            | 41 +++++++++++++++++++++-
 gcc/target.h                               | 37 ++++++++++++++++++-
 3 files changed, 112 insertions(+), 3 deletions(-)

diff --git a/gcc/config/aarch64/aarch64-sve-builtins.cc b/gcc/config/aarch64/aarch64-sve-builtins.cc
index 44e4807325a..dbf2dae4c61 100644
--- a/gcc/config/aarch64/aarch64-sve-builtins.cc
+++ b/gcc/config/aarch64/aarch64-sve-builtins.cc
@@ -5174,7 +5174,11 @@ bool
 verify_type_context (location_t loc, type_context_kind context,
 		     const_tree type, bool silent_p)
 {
-  if (!sizeless_type_p (type))
+  const_tree tmp = type;
+  if (omp_type_context (context) && POINTER_TYPE_P (type))
+    tmp = strip_pointer_types (tmp);
+
+  if (!sizeless_type_p (tmp))
     return true;
 
   switch (context)
@@ -5234,6 +5238,37 @@ verify_type_context (location_t loc, type_context_kind context,
       if (!silent_p)
 	error_at (loc, "capture by copy of SVE type %qT", type);
       return false;
+
+    case TCTX_OMP_MAP:
+      if (!silent_p)
+	error_at (loc, "SVE type %qT not allowed in map clause", type);
+      return false;
+
+    case TCTX_OMP_MAP_IMP_REF:
+      if (!silent_p)
+	error ("cannot reference %qT object types in target region", type);
+      return false;
+
+    case TCTX_OMP_PRIVATE:
+      if (!silent_p)
+	error_at (loc, "SVE type %qT not allowed in"
+		  " target private clause", type);
+      return false;
+
+    case TCTX_OMP_FIRSTPRIVATE:
+      if (!silent_p)
+	error_at (loc, "SVE type %qT not allowed in"
+		  " target firstprivate clause", type);
+      return false;
+
+    case TCTX_OMP_DEVICE_ADDR:
+      if (!silent_p)
+	error_at (loc, "SVE type %qT not allowed in"
+		  " target device clauses", type);
+      return false;
+
+    default:
+      break;
     }
   gcc_unreachable ();
 }
diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc
index 534c8457280..cbe85a5fbe1 100644
--- a/gcc/gimplify.cc
+++ b/gcc/gimplify.cc
@@ -9046,11 +9046,18 @@ omp_notice_variable (struct gimplify_omp_ctx *ctx, tree decl, bool in_code)
 			  | GOVD_MAP_ALLOC_ONLY)) == flags)
 	    {
 	      tree type = TREE_TYPE (decl);
+	      location_t loc = DECL_SOURCE_LOCATION (decl);
 
 	      if (gimplify_omp_ctxp->target_firstprivatize_array_bases
 		  && omp_privatize_by_reference (decl))
 		type = TREE_TYPE (type);
-	      if (!omp_mappable_type (type))
+
+	      if (!verify_type_context (loc, TCTX_OMP_MAP_IMP_REF, type))
+		/* Check if TYPE can appear in a target region.
+		   verify_type_context has already issued an error if it
+		   can't.  */
+		nflags |= GOVD_MAP | GOVD_EXPLICIT;
+	      else if (!omp_mappable_type (type))
 		{
 		  error ("%qD referenced in target region does not have "
 			 "a mappable type", decl);
@@ -12780,6 +12787,8 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq *pre_p,
       unsigned int flags;
       tree decl;
       auto_vec<omp_addr_token *, 10> addr_tokens;
+      tree op = NULL_TREE;
+      location_t loc = OMP_CLAUSE_LOCATION (c);
 
       if (grp_end && c == OMP_CLAUSE_CHAIN (grp_end))
 	{
@@ -12787,6 +12796,36 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq *pre_p,
 	  grp_end = NULL_TREE;
 	}
 
+      if (code == OMP_TARGET
+	  || code == OMP_TARGET_DATA
+	  || code == OMP_TARGET_ENTER_DATA
+	  || code == OMP_TARGET_EXIT_DATA)
+	/* Do some target-specific type checks for map operands.  */
+	switch (OMP_CLAUSE_CODE (c))
+	  {
+	  case OMP_CLAUSE_MAP:
+	    op = OMP_CLAUSE_OPERAND (c, 0);
+	    verify_type_context (loc, TCTX_OMP_MAP, TREE_TYPE (op));
+	    break;
+	  case OMP_CLAUSE_PRIVATE:
+	    op = OMP_CLAUSE_OPERAND (c, 0);
+	    verify_type_context (loc, TCTX_OMP_PRIVATE, TREE_TYPE (op));
+	    break;
+	  case OMP_CLAUSE_FIRSTPRIVATE:
+	    op = OMP_CLAUSE_OPERAND (c, 0);
+	    verify_type_context (loc, TCTX_OMP_FIRSTPRIVATE, TREE_TYPE (op));
+	    break;
+	  case OMP_CLAUSE_IS_DEVICE_PTR:
+	  case OMP_CLAUSE_USE_DEVICE_ADDR:
+	  case OMP_CLAUSE_USE_DEVICE_PTR:
+	  case OMP_CLAUSE_HAS_DEVICE_ADDR:
+	    op = OMP_CLAUSE_OPERAND (c, 0);
+	    verify_type_context (loc, TCTX_OMP_DEVICE_ADDR, TREE_TYPE (op));
+	    break;
+	  default:
+	    break;
+	  }
+
       switch (OMP_CLAUSE_CODE (c))
 	{
 	case OMP_CLAUSE_PRIVATE:
diff --git a/gcc/target.h b/gcc/target.h
index 2bf35e2d0ee..c9c7b52b40f 100644
--- a/gcc/target.h
+++ b/gcc/target.h
@@ -274,7 +274,24 @@ enum type_context_kind {
   TCTX_EXCEPTIONS,
 
   /* Capturing objects of type T by value in a closure.  */
-  TCTX_CAPTURE_BY_COPY
+  TCTX_CAPTURE_BY_COPY,
+
+  /* Objects of type T appearing in OpenMP map clause.  */
+  TCTX_OMP_MAP,
+
+  /* Objects of type T appearing in OpenMP target region
+     without explicit map.  */
+  TCTX_OMP_MAP_IMP_REF,
+
+  /* Objects of type T appearing in OpenMP private clause.  */
+  TCTX_OMP_PRIVATE,
+
+  /* Objects of type T appearing in OpenMP firstprivate clause.  */
+  TCTX_OMP_FIRSTPRIVATE,
+
+  /* Objects of type T appearing in OpenMP device clauses.  */
+  TCTX_OMP_DEVICE_ADDR
+
 };
 
 enum poly_value_estimate_kind
@@ -343,6 +360,24 @@ mode_can_transfer_bits (machine_mode mode)
   return true;
 }
 
+/* Return true if OpenMP context types.  */
+
+inline bool
+omp_type_context (type_context_kind context)
+{
+  switch (context)
+    {
+    case TCTX_OMP_MAP:
+    case TCTX_OMP_MAP_IMP_REF:
+    case TCTX_OMP_PRIVATE:
+    case TCTX_OMP_FIRSTPRIVATE:
+    case TCTX_OMP_DEVICE_ADDR:
+      return true;
+    default:
+      return false;
+    }
+}
+
 #ifdef GCC_TM_H
 
 #ifndef CUMULATIVE_ARGS_MAGIC
-- 
2.25.1

Re: [PATCH v4 0/3] [AArch64, OpenMP] Support SVE types with various OpenMP clauses and constructs

[Tejas Belagod]

Ping.

Thanks,
Tejas.

On 3/18/25 11:27 AM, Tejas Belagod wrote:
> This series is based on a previous thread and review comments from RichardS and
> Jakub upstream:
> 
> https://gcc.gnu.org/pipermail/gcc-patches/2025-March/677072.html
> 
> The changes suggested are cosmetic in nature. As suggested in the previous
> series v3, this series doesn't include tests - they're going to be part of a
> separate patch series.
> 
> Patches have been rebased, retested and bootstrapped on aarch64-linux-gcc.
> OK for trunk?
> 
> Richard Sandiford (1):
>    gomp: Various fixes for SVE types [PR101018]
> 
> Tejas Belagod (2):
>    Add function to strip pointer type and get down to the actual pointee
>      type.
>    AArch64: Diagnose OpenMP offloading when SVE types involved.
> 
>   gcc/config/aarch64/aarch64-sve-builtins.cc | 37 ++++++++++++-
>   gcc/fold-const.cc                          |  7 +++
>   gcc/gimplify.cc                            | 60 ++++++++++++++++++----
>   gcc/omp-low.cc                             |  2 +-
>   gcc/poly-int.h                             | 19 +++++++
>   gcc/target.h                               | 37 ++++++++++++-
>   gcc/tree.h                                 |  9 ++++
>   7 files changed, 157 insertions(+), 14 deletions(-)
>