In C++17, an empty class deriving from an empty base is not an
aggregate, while in C++14 it is. In order to implement this, GCC adds
an artificial field to such classes.
This artificial field has no mapping to Fundamental Data Types in the
AArch64 PCS ABI and hence should not count towards determining whether an
object can be passed using the vector registers as per section
"6.4.2 Parameter Passing Rules" in the AArch64 PCS.
https://github.com/ARM-software/abi-aa/blob/master/aapcs64/aapcs64.rst#the-base-procedure-call-standard
This patch avoids counting this artificial field in
aapcs_vfp_sub_candidate, and hence calculates whether such objects
should be passed in vector registers in the same manner as C++14 (where
the artificial field does not exist).
Before this change, the test below would pass the arguments to `f` in
general registers. After this change, the test passes the arguments to
`f` using the vector registers.
The new behaviour matches the behaviour of `armclang`, and also matches
the behaviour when run with `-std=gnu++14`.
> gcc -std=gnu++17 test.cpp
``` test.cpp
struct base {};
struct pair : base
{
float first;
float second;
pair (float f, float s) : first(f), second(s) {}
};
void f (pair);
int main()
{
f({3.14, 666});
return 1;
}
```
We add a `-Wpsabi` warning to catch cases where this fix has changed the ABI for
some functions. Unfortunately this warning is currently emitted multiple times
for each problem, but I feel this is not much of a problem and can be fixed
later if needs be.
Testing:
Bootstrapped on aarch64-linux.
Jakub has provided a testsuite change that catches the original problem.
https://gcc.gnu.org/pipermail/gcc-patches/2020-April/544204.html
This patch makes that testcase pass.
gcc/ChangeLog:
2020-04-21 Matthew Malcomson <matthew.malcom...@arm.com>
Jakub Jelinek <ja...@redhat.com>
PR target/94383
* config/aarch64/aarch64.c (enum cpp17empty_state): New.
(aapcs_vfp_sub_candidate): Account for C++17 empty base class
artificial fields.
(aarch64_vfp_is_call_or_return_candidate): Warn when ABI PCS decision is
different after this fix.
############### Attachment also inlined for ease of reply ###############
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index
c90de65de127992cc0bd9603a32684ebb5bd4fdf..f9f40d52c846afae0d8d2cd9ed4737cdc4b88896
100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -15909,13 +15909,30 @@ aarch64_conditional_register_usage (void)
}
}
+enum cpp17empty_state {
+ DONT_AVOID = 0,
+ AVOID = 1,
+ AVOID_DONE = 3
+};
+
/* Walk down the type tree of TYPE counting consecutive base elements.
If *MODEP is VOIDmode, then set it to the first valid floating point
type. If a non-floating point type is found, or if a floating point
type that doesn't match a non-VOIDmode *MODEP is found, then return -1,
- otherwise return the count in the sub-tree. */
+ otherwise return the count in the sub-tree.
+
+ The AVOID_C17EMPTY_FIELD argument is to allow the caller to check whether
+ this function has changed its behaviour after the fix for PR94384 -- this
+ fix is to avoid artificial fields in empty base classes.
+ When called pointing at a value of DONT_AVOID then this function does not
+ avoid the artificial fields -- this is useful to check whether the function
+ returns something different after the fix.
+ When called pointing at a value which has the AVOID bit set, this function
+ avoids such artificial fields and sets the value to AVOID_DONE when one of
+ these fields has been set. */
static int
-aapcs_vfp_sub_candidate (const_tree type, machine_mode *modep)
+aapcs_vfp_sub_candidate (const_tree type, machine_mode *modep,
+ enum cpp17empty_state *avoid_c17empty_field)
{
machine_mode mode;
HOST_WIDE_INT size;
@@ -15992,7 +16009,8 @@ aapcs_vfp_sub_candidate (const_tree type, machine_mode
*modep)
|| TREE_CODE (TYPE_SIZE (type)) != INTEGER_CST)
return -1;
- count = aapcs_vfp_sub_candidate (TREE_TYPE (type), modep);
+ count = aapcs_vfp_sub_candidate (TREE_TYPE (type), modep,
+ avoid_c17empty_field);
if (count == -1
|| !index
|| !TYPE_MAX_VALUE (index)
@@ -16030,7 +16048,22 @@ aapcs_vfp_sub_candidate (const_tree type, machine_mode
*modep)
if (TREE_CODE (field) != FIELD_DECL)
continue;
- sub_count = aapcs_vfp_sub_candidate (TREE_TYPE (field), modep);
+ /* Ignore C++17 empty base fields, while their type indicates
+ they do contain padding, they have zero size and thus don't
+ contain any padding. */
+ if (DECL_ARTIFICIAL (field)
+ && DECL_NAME (field) == NULL_TREE
+ && RECORD_OR_UNION_TYPE_P (TREE_TYPE (field))
+ && DECL_SIZE (field)
+ && integer_zerop (DECL_SIZE (field))
+ && (*avoid_c17empty_field & AVOID))
+ {
+ *avoid_c17empty_field = AVOID_DONE;
+ continue;
+ }
+
+ sub_count = aapcs_vfp_sub_candidate (TREE_TYPE (field), modep,
+ avoid_c17empty_field);
if (sub_count < 0)
return -1;
count += sub_count;
@@ -16063,7 +16096,8 @@ aapcs_vfp_sub_candidate (const_tree type, machine_mode
*modep)
if (TREE_CODE (field) != FIELD_DECL)
continue;
- sub_count = aapcs_vfp_sub_candidate (TREE_TYPE (field), modep);
+ sub_count = aapcs_vfp_sub_candidate (TREE_TYPE (field), modep,
+ avoid_c17empty_field);
if (sub_count < 0)
return -1;
count = count > sub_count ? count : sub_count;
@@ -16180,10 +16214,22 @@ aarch64_vfp_is_call_or_return_candidate (machine_mode
mode,
}
else if (type && composite_p)
{
- int ag_count = aapcs_vfp_sub_candidate (type, &new_mode);
+ enum cpp17empty_state after_fix = AVOID;
+ int ag_count = aapcs_vfp_sub_candidate (type, &new_mode, &after_fix);
if (ag_count > 0 && ag_count <= HA_MAX_NUM_FLDS)
{
+ int alt;
+ enum cpp17empty_state before_fix = DONT_AVOID;
+ if (warn_psabi
+ && after_fix == AVOID_DONE
+ && ((alt = aapcs_vfp_sub_candidate (type, &new_mode,
+ &before_fix))
+ != ag_count)
+ && (alt <= 0 || alt > HA_MAX_NUM_FLDS))
+ inform (input_location, "parameter passing of type %qT changed for "
+ "std=c++17 in GCC 10", type);
+
if (is_ha != NULL) *is_ha = true;
*count = ag_count;
}
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index
c90de65de127992cc0bd9603a32684ebb5bd4fdf..f9f40d52c846afae0d8d2cd9ed4737cdc4b88896
100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -15909,13 +15909,30 @@ aarch64_conditional_register_usage (void)
}
}
+enum cpp17empty_state {
+ DONT_AVOID = 0,
+ AVOID = 1,
+ AVOID_DONE = 3
+};
+
/* Walk down the type tree of TYPE counting consecutive base elements.
If *MODEP is VOIDmode, then set it to the first valid floating point
type. If a non-floating point type is found, or if a floating point
type that doesn't match a non-VOIDmode *MODEP is found, then return -1,
- otherwise return the count in the sub-tree. */
+ otherwise return the count in the sub-tree.
+
+ The AVOID_C17EMPTY_FIELD argument is to allow the caller to check whether
+ this function has changed its behaviour after the fix for PR94384 -- this
+ fix is to avoid artificial fields in empty base classes.
+ When called pointing at a value of DONT_AVOID then this function does not
+ avoid the artificial fields -- this is useful to check whether the function
+ returns something different after the fix.
+ When called pointing at a value which has the AVOID bit set, this function
+ avoids such artificial fields and sets the value to AVOID_DONE when one of
+ these fields has been set. */
static int
-aapcs_vfp_sub_candidate (const_tree type, machine_mode *modep)
+aapcs_vfp_sub_candidate (const_tree type, machine_mode *modep,
+ enum cpp17empty_state *avoid_c17empty_field)
{
machine_mode mode;
HOST_WIDE_INT size;
@@ -15992,7 +16009,8 @@ aapcs_vfp_sub_candidate (const_tree type, machine_mode
*modep)
|| TREE_CODE (TYPE_SIZE (type)) != INTEGER_CST)
return -1;
- count = aapcs_vfp_sub_candidate (TREE_TYPE (type), modep);
+ count = aapcs_vfp_sub_candidate (TREE_TYPE (type), modep,
+ avoid_c17empty_field);
if (count == -1
|| !index
|| !TYPE_MAX_VALUE (index)
@@ -16030,7 +16048,22 @@ aapcs_vfp_sub_candidate (const_tree type, machine_mode
*modep)
if (TREE_CODE (field) != FIELD_DECL)
continue;
- sub_count = aapcs_vfp_sub_candidate (TREE_TYPE (field), modep);
+ /* Ignore C++17 empty base fields, while their type indicates
+ they do contain padding, they have zero size and thus don't
+ contain any padding. */
+ if (DECL_ARTIFICIAL (field)
+ && DECL_NAME (field) == NULL_TREE
+ && RECORD_OR_UNION_TYPE_P (TREE_TYPE (field))
+ && DECL_SIZE (field)
+ && integer_zerop (DECL_SIZE (field))
+ && (*avoid_c17empty_field & AVOID))
+ {
+ *avoid_c17empty_field = AVOID_DONE;
+ continue;
+ }
+
+ sub_count = aapcs_vfp_sub_candidate (TREE_TYPE (field), modep,
+ avoid_c17empty_field);
if (sub_count < 0)
return -1;
count += sub_count;
@@ -16063,7 +16096,8 @@ aapcs_vfp_sub_candidate (const_tree type, machine_mode
*modep)
if (TREE_CODE (field) != FIELD_DECL)
continue;
- sub_count = aapcs_vfp_sub_candidate (TREE_TYPE (field), modep);
+ sub_count = aapcs_vfp_sub_candidate (TREE_TYPE (field), modep,
+ avoid_c17empty_field);
if (sub_count < 0)
return -1;
count = count > sub_count ? count : sub_count;
@@ -16180,10 +16214,22 @@ aarch64_vfp_is_call_or_return_candidate (machine_mode
mode,
}
else if (type && composite_p)
{
- int ag_count = aapcs_vfp_sub_candidate (type, &new_mode);
+ enum cpp17empty_state after_fix = AVOID;
+ int ag_count = aapcs_vfp_sub_candidate (type, &new_mode, &after_fix);
if (ag_count > 0 && ag_count <= HA_MAX_NUM_FLDS)
{
+ int alt;
+ enum cpp17empty_state before_fix = DONT_AVOID;
+ if (warn_psabi
+ && after_fix == AVOID_DONE
+ && ((alt = aapcs_vfp_sub_candidate (type, &new_mode,
+ &before_fix))
+ != ag_count)
+ && (alt <= 0 || alt > HA_MAX_NUM_FLDS))
+ inform (input_location, "parameter passing of type %qT changed for "
+ "std=c++17 in GCC 10", type);
+
if (is_ha != NULL) *is_ha = true;
*count = ag_count;
}
