Bug 94711 - [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 on arm
Summary: [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 ...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 10.0
: P1 blocker
Target Milestone: 10.0
Assignee: Not yet assigned to anyone
URL:
Keywords: ABI, wrong-code
Depends on: 94383
Blocks:
  Show dependency treegraph
 
Reported: 2020-04-22 12:12 UTC by Jakub Jelinek
Modified: 2020-04-28 14:45 UTC (History)
9 users (show)

See Also:
Host:
Target: arm-*-linux
Build:
Known to work:
Known to fail:
Last reconfirmed: 2020-04-23 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jakub Jelinek 2020-04-22 12:12:14 UTC
+++ This bug was initially created as a clone of Bug #94383 +++

The same issue applies to 32-bit ARM too.
Comment 1 Jakub Jelinek 2020-04-27 08:04:17 UTC
So, what exactly needs changing on ARM?
From quick skimming, maybe arm_return_in_memory, very likely aapcs_vfp_sub_candidate, and maybe arm_needs_doubleword_align.
What about comp_not_to_clear_mask_str_un ?

Note, we'd like to do 10.1-rc1 as early possible and this PR is one of the last two remaining blockers.
Comment 2 CVS Commits 2020-04-28 14:38:58 UTC
The master branch has been updated by Matthew Malcomson <matmal01@gcc.gnu.org>:

https://gcc.gnu.org/g:a5bff8af0a68d039e1586087639c86d6931c9b81

commit r10-8013-ga5bff8af0a68d039e1586087639c86d6931c9b81
Author: Matthew Malcomson <matthew.malcomson@arm.com>
Date:   Tue Apr 28 15:38:43 2020 +0100

    [Arm] Account for C++17 artificial field determining Homogeneous Aggregates
    
    In C++14, an empty class deriving from an empty base is not an
    aggregate, while in C++17 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
    Arm PCS ABI and hence should not count towards determining whether an
    object can be passed using the vector registers as per section
    "7.1.2 Procedure Calling" in the arm PCS
    https://developer.arm.com/docs/ihi0042/latest?_ga=2.60211309.1506853196.1533541889-405231439.1528186050
    
    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 GCC behaviour when run with `-std=gnu++14`.
    
    > gcc -std=gnu++17 -march=armv8-a+simd -mfloat-abi=hard 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 not emitted twice for multiple
    calls to the same function, but I feel this is not much of a problem and can be
    fixed later if needs be.
    
    (i.e. if `main` called `f` twice in a row we only emit a diagnostic for the
    first).
    
    Testing:
        Bootstrapped and regression tested on arm-linux.
        This change fixes the struct-layout-1 tests Jakub added
        https://gcc.gnu.org/pipermail/gcc-patches/2020-April/544204.html
        Regression tested on arm-none-eabi.
    
    gcc/ChangeLog:
    
    2020-04-28  Matthew Malcomson  <matthew.malcomson@arm.com>
                Jakub Jelinek  <jakub@redhat.com>
    
            PR target/94711
            * config/arm/arm.c (aapcs_vfp_sub_candidate): Account for C++17 empty
            base class artificial fields.
            (aapcs_vfp_is_call_or_return_candidate): Warn when PCS ABI
            decision is different after this fix.
Comment 3 Matthew Malcomson 2020-04-28 14:44:04 UTC
This has been fixed by.
https://gcc.gnu.org/pipermail/gcc-patches/2020-April/544625.html

In the email linked above I mentioned another problem using `-mabi=apcs-gnu`.
Since that ABI is obsolete (Kyrylo pointed that out to me) I don't think that problem should hold up GCC10.
Comment 4 Matthew Malcomson 2020-04-28 14:45:07 UTC
Resolving since the issue with the obsolete ABI is different to this ticket.