public inbox for gcc-cvs@sourceware.org help / color / mirror / Atom feed
From: Matthew Malcomson <matmal01@gcc.gnu.org> To: gcc-cvs@gcc.gnu.org Subject: [gcc(refs/vendors/ARM/heads/morello)] Fix few PCS bugs in aarch64_classify_capability_contents Date: Tue, 21 Sep 2021 09:15:11 +0000 (GMT) [thread overview] Message-ID: <20210921091511.4F2F43858412@sourceware.org> (raw) 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;
reply other threads:[~2021-09-21 9:15 UTC|newest] Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20210921091511.4F2F43858412@sourceware.org \ --to=matmal01@gcc.gnu.org \ --cc=gcc-cvs@gcc.gnu.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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).