public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc(refs/vendors/ARM/heads/morello)] Fix few PCS bugs in aarch64_classify_capability_contents
@ 2021-09-21  9:15 Matthew Malcomson
  0 siblings, 0 replies; only message in thread
From: Matthew Malcomson @ 2021-09-21  9:15 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:8666b3dead23dd0258ed20ae743c0cd1f7b95f9f

commit 8666b3dead23dd0258ed20ae743c0cd1f7b95f9f
Author: Matthew Malcomson <matthew.malcomson@arm.com>
Date:   Thu Sep 16 11:34:00 2021 +0100

    Fix few PCS bugs in aarch64_classify_capability_contents
    
    Missed the possibility of a structure that is under the LARGE size and
    does not contain any capabilities.  This should behave as before
    introducing capabilities, but we ended up passing it in memory.
    
    The fundamental problem was that we did not iterate over all members in
    the structure to check there was indeed a capability in it if we'd
    already noticed a non-capability field overlapping bits 8-16 or 24-31.
    
    This fixes gcc.target/aarch64/vect_int32x2x4_1.c, since we no longer
    need to load any data in the function.
    
    A structure of the sort below would have been identified as
    CAPCOM_OVERLAP rather than CAPCOM_NONE since it has bytes overlapping
    capability metadata but despite the fact there is no capability in the
    structure.
    struct s { unsigned long long a; unsigned short b; }
    
    Second problem was that we checked each member with `capability_type_p`
    rather than recursing on `aarch64_classify_capability_contents`.  That
    missed structures which contained structures containing a capability (of
    the sort below).
    
    struct s { struct { void * a; } v; };
    
    Then, since we needed to recurse on members of a structure we needed
    `aarch64_classify_capability_contents` to be able to handle flexible
    array members.  Before now it would return CAPCOM_LARGE since they have
    no size, but we needed to return CAPCOM_NONE since they are not passed
    in the PCS and hence should not be taken into account for how their
    containing structure is handled.
    
    This is a problem largely because we now iterate over all elements
    (since these flexible array members must be last in the structure).
    
    struct s { long long a; long long b; int fa[]; };

Diff:
---
 gcc/config/aarch64/aarch64.c | 41 +++++++++++++++++++++++++++++++----------
 1 file changed, 31 insertions(+), 10 deletions(-)

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 8e25d7da8cf..2443629f0c9 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -1951,6 +1951,11 @@ aarch64_classify_capability_contents (const_tree type)
   if (TYPE_SIZE (type) && integer_zerop (TYPE_SIZE (type)))
     return CAPCOM_NONE;
 
+  /* Flexible array members are not passed in PCS, so do not contain
+     capabilities for PCS purposes.  */
+  if (TREE_CODE (type) == ARRAY_TYPE && !TYPE_SIZE (type))
+    return CAPCOM_NONE;
+
   if (int_size_in_bytes (type) == -1)
     return CAPCOM_LARGE;
 
@@ -1971,11 +1976,14 @@ aarch64_classify_capability_contents (const_tree type)
     return aarch64_classify_capability_contents (TREE_TYPE (type));
 
   enum capability_composite_type subfield_status = CAPCOM_NONE;
+  bool has_overlap = false;
+
   if (RECORD_OR_UNION_TYPE_P (type))
     /* CAPCOM_LARGE overrides CAPCOM_OVERLAP but has already been dealt with.
        CAPCOM_OVERLAP overrides CAPCOM_SOME (doesn't matter if there are
        capabilities if there are some fields that overlap their metadata).
-	 Hence as soon as we see CAPCOM_OVERLAP, we can return that.
+	 However if we're just seeing a non-capability structure member then
+	 this does not change state from CAPCOM_NONE.
        CAPCOM_SOME overrides CAPCOM_NONE.
 	 Have to search through all fields for any CAPCOM_SOME or
 	 CAPCOM_OVERLAP.
@@ -1983,15 +1991,28 @@ aarch64_classify_capability_contents (const_tree type)
     for (tree field = TYPE_FIELDS (type); field; field = TREE_CHAIN (field))
       if (TREE_CODE (field) == FIELD_DECL)
 	{
-	  if (!capability_type_p (TREE_TYPE (field))
-	      && aarch64_field_overlaps_capability_metadata (field))
-	    return CAPCOM_OVERLAP;
-	  else if (aarch64_classify_capability_contents (TREE_TYPE (field))
-		  == CAPCOM_OVERLAP)
-	    return CAPCOM_OVERLAP;
-	  else if (aarch64_classify_capability_contents (TREE_TYPE (field))
-		  == CAPCOM_SOME)
-	    subfield_status = CAPCOM_SOME;
+	  switch (aarch64_classify_capability_contents (TREE_TYPE (field)))
+	    {
+	    case CAPCOM_NONE:
+	      if (!aarch64_field_overlaps_capability_metadata (field))
+		break;
+	      if (subfield_status == CAPCOM_SOME)
+		return CAPCOM_OVERLAP;
+	      has_overlap = true;
+	      break;
+
+	    case CAPCOM_SOME:
+	      if (has_overlap)
+		return CAPCOM_OVERLAP;
+	      subfield_status = CAPCOM_SOME;
+	      break;
+
+	    case CAPCOM_OVERLAP:
+	      return CAPCOM_OVERLAP;
+
+	    default:
+	      gcc_unreachable ();
+	    }
 	}
 
   return subfield_status;


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2021-09-21  9:15 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-21  9:15 [gcc(refs/vendors/ARM/heads/morello)] Fix few PCS bugs in aarch64_classify_capability_contents Matthew Malcomson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).