[Bug c++/61082] New: [x86-64 Itanium ABI] g++ uses wrong return location for class with head padding

[Bug c++/61082] New: [x86-64 Itanium ABI] g++ uses wrong return location for class with head padding

[greened at obbligato dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=61082

            Bug ID: 61082
           Summary: [x86-64 Itanium ABI] g++ uses wrong return location
                    for class with head padding
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: major
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: greened at obbligato dot org

Created attachment 32745
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=32745&action=edit
Testcase

gcc appears to use the wrong sequence for returning an object with head
padding.

For the attached testcase, gcc -O2 generates the following code for function
foo:

    .globl    _Z3fooR1Y
    .type    _Z3fooR1Y, @function
_Z3fooR1Y:
.LFB1019:
    .cfi_startproc
    movq    8(%rdi), %rax
    ret
    .cfi_endproc

The class Y should be classified INTEGER under the x86-64 ABI.  It has eight
bytes of head padding because it inherits from an empty base class and has a
member that also inherits from the same empty base class.  By my reading of the
ABI, foo should return the Y object in (RAX, RDX), with RDX containing the
meaningful bits (the long value).

The Intel compiler generates the code I would expect for this testcase.

[Bug c++/61082] [x86-64 Itanium ABI] g++ uses wrong return location for class with head padding

[greened at obbligato dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=61082

--- Comment #1 from David Greene <greened at obbligato dot org> ---
This is on Linux/SuSE/SLES 11.

[Bug c++/61082] [x86-64 Itanium ABI] g++ uses wrong return location for class with head padding

[pinskia at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=61082

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Y is a non-POD.

[Bug c++/61082] [x86-64 Itanium ABI] g++ uses wrong return location for class with head padding

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=61082

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
extern "C" int printf(const char*, ...);

struct B { };

struct X : public B { long p; };

struct Y : public B { X q; };

Y foo( Y& y )
{
    Y temp;
    temp = y;
    return temp;
}

int main( void )
{
    printf("%zd %zd\n", sizeof( X ), sizeof( Y ));

    Y y;
    y.q.p = 6L;

    Y yy = foo( y );
    printf("%ld\n", yy.q.p);
}

Clang uses the same registers as GCC:

    .globl    _Z3fooR1Y
    .align    16, 0x90
    .type    _Z3fooR1Y,@function
_Z3fooR1Y:                              # @_Z3fooR1Y
    .cfi_startproc
# BB#0:                                 # %entry
    movq    8(%rdi), %rax
    retq
.Ltmp0:
    .size    _Z3fooR1Y, .Ltmp0-_Z3fooR1Y
    .cfi_endproc

[Bug c++/61082] [x86-64 Itanium ABI] g++ uses wrong return location for class with head padding

[greened at obbligato dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=61082

--- Comment #5 from David Greene <greened at obbligato dot org> ---
(In reply to Jonathan Wakely from comment #4)

> Clang uses the same registers as GCC:

Ok, but that still doesn't explain why.  Can you point me to wording in either
the x86-64 ABI or Itanium ABI that describes this behavior?  I have looked for
days and can't find anything that suggests this is correct code.  Everything
I've read points to (RAX, RDX) being the correct return value location.  The
fact that the Intel compiler disagrees with gcc and clang at least means there
is some disagreement on this.

This is an interoperability issue and it would be good to get clarity on this
so people know what to expect.  If gcc and clang are correct that's fine but I
would like to know why.  Because either gcc and clang have to be fixed or the
Intel compiler has to be fixed.

[Bug c++/61082] [x86-64 Itanium ABI] g++ uses wrong return location for class with head padding

[greened at obbligato dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=61082

--- Comment #6 from David Greene <greened at obbligato dot org> ---
Bug filed against clang to see what they have to say.

http://llvm.org/bugs/show_bug.cgi?id=19675

[Bug c++/61082] [x86-64 Itanium ABI] g++ uses wrong return location for class with head padding

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=61082

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hjl.tools at gmail dot com

--- Comment #7 from H.J. Lu <hjl.tools at gmail dot com> ---
It is the matter of if the size of

---
struct B { };
struct X : public B { long p; };
---

is size of long or not.

[Bug c++/61082] [x86-64 Itanium ABI] g++ uses wrong return location for class with head padding

[greened at obbligato dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=61082

--- Comment #8 from David Greene <greened at obbligato dot org> ---
(In reply to H.J. Lu from comment #7)
> It is the matter of if the size of
> 
> ---
> struct B { };
> struct X : public B { long p; };
> ---
> 
> is size of long or not.

I don't understand.  The class of concern is Y, not X.  Y is (correctly) 16
bytes, as reported by sizeof().  X is 8 bytes due to EBO.  According to my
reading of the ABI, a 16-byte struct should be returned in (RAX, RDX).

[Bug c++/61082] [x86-64 Itanium ABI] g++ uses wrong return location for class with head padding

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=61082

--- Comment #9 from H.J. Lu <hjl.tools at gmail dot com> ---
Y is returned as {NO_CLASS, INTEGER} in register. psABI doesn't
explicitly say how NO_CLASS should be handled in this case.  GCC
simply skips NO_CLASS when assigning it to a register.

[Bug c++/61082] [x86-64 Itanium ABI] g++ uses wrong return location for class with head padding

[greened at obbligato dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=61082

--- Comment #10 from David Greene <greened at obbligato dot org> ---
(In reply to H.J. Lu from comment #9)
> Y is returned as {NO_CLASS, INTEGER} in register. psABI doesn't
> explicitly say how NO_CLASS should be handled in this case.  GCC
> simply skips NO_CLASS when assigning it to a register.

Except section 3.2.3 when talking about classifying aggregates says under point
4 (b):

If one of the classes is NO_CLASS, the resulting class is the other class.

So Y should be classified as INTEGER and returned in (RAX, RDX).

[Bug c++/61082] [x86-64 Itanium ABI] g++ uses wrong return location for class with head padding

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=61082

--- Comment #11 from H.J. Lu <hjl.tools at gmail dot com> ---
(In reply to David Greene from comment #10)
> 
> So Y should be classified as INTEGER and returned in (RAX, RDX).

That is correct and Y is classified as INTEGER with 2 fields:
NO_CLASS, INTEGER.  The question is how NO_CLASS should be handled.

[Bug c++/61082] [x86-64 Itanium ABI] g++ uses wrong return location for class with head padding

[greened at obbligato dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=61082

--- Comment #12 from David Greene <greened at obbligato dot org> ---
(In reply to H.J. Lu from comment #11)

> That is correct and Y is classified as INTEGER with 2 fields:
> NO_CLASS, INTEGER.  The question is how NO_CLASS should be handled.

Since the "Returning of Values" section of 3.2.3 doesn't talk about aggregate
members, I assume that the entire type classification (INTEGER in this case)
should be used.  That is the interpretation Intel seems to use.

The example given does talk about members so I can see how either
interpretation could be considered correct.

g++ appears to pass such an object in RDI only so it is at least consistent in
its pass/return processing.

Strangely, icc appears to pass Y in memory!

[Bug c++/61082] [x86-64 Itanium ABI] g++ uses wrong return location for class with head padding

[greened at obbligato dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=61082

--- Comment #13 from David Greene <greened at obbligato dot org> ---
I see that 3.2.3 4 (b) is talking about considering adjacent fields in an
eightbyte.  Is the intent to classify each eightbyte in an aggregate and then
consider each eightbyte separately for assigning argument and return registers?

The post-merger cleanup described in 5 appears to handle passing in memory in
that it considers all of the fields in the aggregate together.

Given the above, if a field crosses an eightbyte either it is larger than an
eightbyte either it is unaligned which forces the whole argument to memory or
its eightbytes are classified separately in a recursive manner.

Does this sound correct?  If so, I think gcc is correct in what it's doing.

[Bug c++/61082] [x86-64 Itanium ABI] g++ uses wrong return location for class with head padding

[greened at obbligato dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=61082

--- Comment #15 from David Greene <greened at obbligato dot org> ---
(In reply to H.J. Lu from comment #14)

> I think GCC is correct.

I agree.  Thanks for working through the explanation with me.

[Bug c++/61082] [x86-64 Itanium ABI] g++ uses wrong return location for class with head padding

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=61082

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |INVALID

--- Comment #16 from Richard Biener <rguenth at gcc dot gnu.org> ---
Not a bug then.