[Bug c++/95384] New: Poor codegen cause by using base class instead of member for Optional construction

[Bug c++/95384] New: Poor codegen cause by using base class instead of member for Optional construction

[barry.revzin at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95384

            Bug ID: 95384
           Summary: Poor codegen cause by using base class instead of
                    member for Optional construction
           Product: gcc
           Version: 10.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: barry.revzin at gmail dot com
  Target Milestone: ---

Following up on #95383:

struct nullopt_t {} inline constexpr nullopt{};

template <typename T>
struct OptionalStorage {
    struct Empty { };
    union {
        Empty _;
        T value;
    };
    bool engaged;

    OptionalStorage(nullopt_t) : _(), engaged(false) { }
    OptionalStorage(T v) : value(v), engaged(true) { }
};

template <typename T>
struct OptionalWithBase : OptionalStorage<T> {
    using OptionalStorage<T>::OptionalStorage;
};

template <typename T>
struct OptionalWithMember {
    OptionalStorage<T> o;
    OptionalWithMember(nullopt_t) : o(nullopt) { }
    OptionalWithMember(T v) : o(v) { }
};

OptionalWithBase<int> foo_with_base(bool b) {
    if (b) {
        return 42;
    }
    return nullopt;
}

OptionalWithMember<int> foo_with_member(bool b) {
    if (b) {
        return 42;
    }
    return nullopt;
}

OptionalWithBase<T> and OptionalWithMember<T> should be the same thing. It's
just that one inherits from its storage and the other has it as a member. But
the codegen is very different (https://godbolt.org/z/j8m68Y), probably
something to do with tail padding?

gcc 10.1 -O2:

foo_with_base(bool):
        test    dil, dil
        je      .L2
        mov     DWORD PTR [rsp-8], 42
        mov     BYTE PTR [rsp-4], 1
        mov     rax, QWORD PTR [rsp-8]
        ret
.L2:
        mov     BYTE PTR [rsp-4], 0
        mov     rax, QWORD PTR [rsp-8]
        ret
foo_with_member(bool):
        test    dil, dil
        mov     eax, 0
        movabs  rdx, 4294967338
        cmovne  rax, rdx
        ret

gcc 10.2 -O3 or gcc trunk -O2:

foo_with_base(bool):
        xor     eax, eax
        test    dil, dil
        je      .L2
        mov     DWORD PTR [rsp-8], 42
        mov     eax, 1
.L2:
        mov     BYTE PTR [rsp-4], al
        mov     rax, QWORD PTR [rsp-8]
        ret
foo_with_member(bool):
        xor     edx, edx
        mov     ecx, 42
        test    dil, dil
        cmovne  rax, rcx
        mov     ecx, 1
        cmovne  rdx, rcx
        movabs  rcx, -1095216660481
        and     rax, rcx
        sal     rdx, 32
        or      rax, rdx
        ret

clang 10, -O2 or -O3:

foo_with_base(bool):                     # @foo_with_base(bool)
        shl     rdi, 32
        lea     rax, [rdi + 42]
        ret
foo_with_member(bool):                   # @foo_with_member(bool)
        shl     rdi, 32
        lea     rax, [rdi + 42]
        ret

[Bug c++/95384] Poor codegen cause by using base class instead of member for Optional construction

[barry.revzin at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95384

--- Comment #1 from Barry Revzin <barry.revzin at gmail dot com> ---
Here's a simpler example: https://godbolt.org/z/FD7uEQ

If the engaged member is an int instead of a bool (to remove the tail padding),
gcc generates better code.

This follows up on "PR 95383"

[Bug c++/95384] Poor codegen cause by using base class instead of member for Optional construction

[glisse at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95384

--- Comment #2 from Marc Glisse <glisse at gcc dot gnu.org> ---
Or with less templates:

struct A {
  A() = default;
  union S {
    constexpr S() noexcept : e() { }
    struct {} e;
    int i;
  } s;
  bool b = false;
};
struct B : A {
  B() = default;
  using A::A;
};
B f() { return {}; }


        movl    $0, -12(%rsp)
        movq    -16(%rsp), %rax
        ret

instead of a plain

        xorl    %eax, %eax

optimized dump is

  MEM <char[4]> [(struct B *)&D.2265 + 4B] = {};
  MEM[(union S *)&D.2265] ={v} {CLOBBER};
  D.2275 = D.2265;
  D.2265 ={v} {CLOBBER};
  return D.2275;

expand

(insn 5 2 6 2 (set (mem/c:SI (plus:DI (reg/f:DI 77 virtual-stack-vars)
                (const_int -12 [0xfffffffffffffff4])) [7 MEM <char[4]> [(struct
B *)&D.2265 + 4B]+0 S4 A32])
        (const_int 0 [0])) "o.cc":14:17 -1
     (nil))
(insn 6 5 7 2 (set (reg:DI 83)
        (mem/c:DI (plus:DI (reg/f:DI 77 virtual-stack-vars)
                (const_int -16 [0xfffffffffffffff0])) [7 D.2265+0 S8 A64]))
"o.cc":14:17 -1
     (nil))
(insn 7 6 8 2 (set (mem/c:DI (plus:DI (reg/f:DI 77 virtual-stack-vars)
                (const_int -8 [0xfffffffffffffff8])) [7 D.2275+0 S8 A64])
        (reg:DI 83)) "o.cc":14:17 -1
     (nil))
(insn 8 7 9 2 (set (reg:DI 84)
        (const_int 0 [0])) "o.cc":14:17 -1
     (nil))
(insn 9 8 10 2 (set (reg:DI 84)
        (mem/c:DI (plus:DI (reg/f:DI 77 virtual-stack-vars)
                (const_int -8 [0xfffffffffffffff8])) [7 D.2275+0 S8 A64]))
"o.cc":14:17 -1
     (nil))
(insn 10 9 11 2 (set (reg:DI 85)
        (reg:DI 84)) "o.cc":14:17 -1
     (nil))
(insn 11 10 15 2 (set (reg:DI 82 [ <retval> ])
        (reg:DI 85)) "o.cc":14:17 -1
     (nil))
(insn 15 11 16 2 (set (reg/i:DI 0 ax)
        (reg:DI 82 [ <retval> ])) "o.cc":14:20 -1
     (nil))
(insn 16 15 0 2 (use (reg/i:DI 0 ax)) "o.cc":14:20 -1
     (nil))

and dfinish

(insn:TI 5 2 15 2 (set (mem/c:SI (plus:DI (reg/f:DI 7 sp)
                (const_int -12 [0xfffffffffffffff4])) [7 MEM <char[4]> [(struct
B *)&D.2265 + 4B]+0 S4 A32])
        (const_int 0 [0])) "o.cc":14:17 67 {*movsi_internal}
     (nil))
(insn 15 5 16 2 (set (reg/i:DI 0 ax)
        (mem/c:DI (plus:DI (reg/f:DI 7 sp)
                (const_int -16 [0xfffffffffffffff0])) [7 D.2265+0 S8 A64]))
"o.cc":14:20 66 {*movdi_internal}
     (nil))
(insn 16 15 25 2 (use (reg/i:DI 0 ax)) "o.cc":14:20 -1
     (nil))

[Bug tree-optimization/95384] Poor codegen cause by using base class instead of member for Optional construction

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95384

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|c++                         |tree-optimization
           Severity|normal                      |enhancement

[Bug tree-optimization/95384] Poor codegen cause by using base class instead of member for Optional construction

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95384

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2021-08-04

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Confirmed.
All bad around.  I wonder if we could "expose" some of the details of the ABI
late in gimple to simplify/solve this.

[Bug tree-optimization/95384] Poor codegen cause by using base class instead of member for Optional construction

[barry.revzin at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95384

--- Comment #4 from Barry Revzin <barry.revzin at gmail dot com> ---
Here's another example of the same kind of issue
(https://godbolt.org/z/KWr9rMssj):

template <class T, class U>
struct tagged_union {
    tagged_union(T t) : index(0), a(t) { }
    tagged_union(U u) : index(1), b(u) { }


    union {
        T a;
        U b;
    };
    char index;
};

struct X { int i; };
struct Y { int j; };

tagged_union<X, Y> as_tagged_union(X x) {
    return x;
}

template <typename T, typename U>
struct tagged_union_wrapped : tagged_union<T, U> {
    using tagged_union<T, U>::tagged_union;
};

auto as_tagged_union2(X x) {
    return tagged_union_wrapped<X, Y>(x);
}

this on -O3 emits:

as_tagged_union(X):
        mov     eax, edi
        ret
as_tagged_union2(X):
        mov     DWORD PTR [rsp-8], edi
        mov     BYTE PTR [rsp-4], 0
        mov     rax, QWORD PTR [rsp-8]
        ret

If you change the index member from 'char' to 'int', causing the tail padding
to disappear, as_tagged_union2 improves to the same code gen as
as_tagged_union.

This is relevant for std::variant performance. std::variant<X, Y> behaves like
tagged_union_wrapped<X, Y>, whereas if you drop down to the implementation
details and directly use _Variant_storage_alias<X, Y>, that behaves like
tagged_union<X, Y> for these purposes.