[Bug d/114171] New: [13/14 Regression] gdc -O2 -mavx generates misaligned vmovdqa instruction

[Bug d/114171] New: [13/14 Regression] gdc -O2 -mavx generates misaligned vmovdqa instruction

[a.horodniceanu at proton dot me]

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

            Bug ID: 114171
           Summary: [13/14 Regression] gdc -O2 -mavx generates misaligned
                    vmovdqa instruction
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: d
          Assignee: ibuclaw at gdcproject dot org
          Reporter: a.horodniceanu at proton dot me
  Target Milestone: ---

I've hit this initially in the tests of D-Scanner but I've reduced it to:
---
struct Token {
        string label;
}

interface ASTNode {}

abstract class BaseNode : ASTNode {}

final class BreakStatement : BaseNode {
        Token label;
}

int main () {
        auto bn = new BreakStatement();
        return bn.label is Token.init;
}
---

Compiled with `gdc -O2 -mavx a.d -o a` it segfaults at runtime.

gdb shows the following assembly:
---
   0x25520 <_Dmain>:    sub    $0x8,%rsp
   0x25524 <_Dmain+4>:  call   0x254b0
<_D4core8lifetime__T12_d_newclassTTC1a14BreakStatementZQBjFNaNbNeZQBf>
   0x25529 <_Dmain+9>:  vmovdqa 0x18(%rax),%xmm0
---
The issue is with vmovdqa which expected the memory to be 16-byte aligned but
it is only 8-byte.

I've tested with gdc 13 and 14 with the exact versions:
---
Using built-in specs.
COLLECT_GCC=/usr/x86_64-pc-linux-gnu/gcc-bin/14/gdc
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-pc-linux-gnu/14/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with:
/var/tmp/portage/sys-devel/gcc-14.0.9999/work/gcc-14.0.9999/configure
--host=x86_64-pc-linux-gnu --build=x86_64-pc-linux-gnu --prefix=/usr
--bindir=/usr/x86_64-pc-linux-gnu/gcc-bin/14
--includedir=/usr/lib/gcc/x86_64-pc-linux-gnu/14/include
--datadir=/usr/share/gcc-data/x86_64-pc-linux-gnu/14
--mandir=/usr/share/gcc-data/x86_64-pc-linux-gnu/14/man
--infodir=/usr/share/gcc-data/x86_64-pc-linux-gnu/14/info
--with-gxx-include-dir=/usr/lib/gcc/x86_64-pc-linux-gnu/14/include/g++-v14
--disable-silent-rules --disable-dependency-tracking
--with-python-dir=/share/gcc-data/x86_64-pc-linux-gnu/14/python
--enable-languages=c,c++,d,objc,fortran --enable-obsolete --enable-secureplt
--disable-werror --with-system-zlib --enable-nls --without-included-gettext
--disable-libunwind-exceptions --enable-checking=yes,extra
--with-bugurl=https://bugs.gentoo.org/ --with-pkgversion='Gentoo Hardened
14.0.9999 p, commit e54a7fbca63053b5753fd9ba543c27ef392d3084'
--with-gcc-major-version-only --enable-libstdcxx-time --enable-lto
--disable-libstdcxx-pch --enable-shared --enable-threads=posix
--enable-__cxa_atexit --enable-clocale=gnu --enable-multilib
--with-multilib-list=m32,m64 --disable-fixed-point --enable-targets=all
--enable-libgomp --disable-libssp --disable-libada --enable-cet
--disable-systemtap --disable-valgrind-annotations --disable-vtable-verify
--disable-libvtv --without-zstd --without-isl --enable-default-pie
--enable-host-pie --enable-host-bind-now --enable-default-ssp
--disable-fixincludes --with-build-config=bootstrap-cet
Thread model: posix
Supported LTO compression algorithms: zlib
gcc version 14.0.1 20240219 (experimental)
3a6f3354eaaf38b5e6be41e4ebf521d299593a6e (Gentoo Hardened 14.0.9999 p, commit
e54a7fbca63053b5753fd9ba543c27ef392d3084)
---
and
---
Using built-in specs.
COLLECT_GCC=/usr/x86_64-pc-linux-gnu/gcc-bin/13/gdc
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-pc-linux-gnu/13/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with:
/var/tmp/portage/sys-devel/gcc-13.2.1_p20240113-r1/work/gcc-13-20240113/configure
--host=x86_64-pc-linux-gnu --build=x86_64-pc-linux-gnu --prefix=/usr
--bindir=/usr/x86_64-pc-linux-gnu/gcc-bin/13
--includedir=/usr/lib/gcc/x86_64-pc-linux-gnu/13/include
--datadir=/usr/share/gcc-data/x86_64-pc-linux-gnu/13
--mandir=/usr/share/gcc-data/x86_64-pc-linux-gnu/13/man
--infodir=/usr/share/gcc-data/x86_64-pc-linux-gnu/13/info
--with-gxx-include-dir=/usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13
--disable-silent-rules --disable-dependency-tracking
--with-python-dir=/share/gcc-data/x86_64-pc-linux-gnu/13/python
--enable-languages=c,c++,d,objc,fortran --enable-obsolete --enable-secureplt
--disable-werror --with-system-zlib --enable-nls --without-included-gettext
--disable-libunwind-exceptions --enable-checking=release
--with-bugurl=https://bugs.gentoo.org/ --with-pkgversion='Gentoo Hardened
13.2.1_p20240113-r1 p12' --with-gcc-major-version-only --enable-libstdcxx-time
--enable-lto --disable-libstdcxx-pch --enable-shared --enable-threads=posix
--enable-__cxa_atexit --enable-clocale=gnu --enable-multilib
--with-multilib-list=m32,m64 --disable-fixed-point --enable-targets=all
--enable-libgomp --disable-libssp --disable-libada --enable-cet
--disable-systemtap --disable-valgrind-annotations --disable-vtable-verify
--disable-libvtv --without-zstd --without-isl --enable-default-pie
--enable-default-ssp --with-build-config=bootstrap-cet
Thread model: posix
Supported LTO compression algorithms: zlib
gcc version 13.2.1 20240113 (Gentoo Hardened 13.2.1_p20240113-r1 p12)
---

[Bug d/114171] [13/14 Regression] gdc -O2 -mavx generates misaligned vmovdqa instruction

[pinskia at gcc dot gnu.org]

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
[apinski@xeond2 libstdc++-v3]$ c++filt --format dlang
_D4core8lifetime__T12_d_newclassTTC1a14BreakStatementZQBjFNaNbNeZQBf
core.lifetime._d_newclassT!(a.BreakStatement)._d_newclassT()

[Bug d/114171] [13/14 Regression] gdc -O2 -mavx generates misaligned vmovdqa instruction

[liuhongt at gcc dot gnu.org]

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

Hongtao Liu <liuhongt at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |liuhongt at gcc dot gnu.org
   Last reconfirmed|                            |2024-3-1

--- Comment #2 from Hongtao Liu <liuhongt at gcc dot gnu.org> ---
on rtl level,we get

(insn 7 6 8 2 (set (reg:CCZ 17 flags)
        (compare:CCZ (mem:TI (plus:DI (reg/f:DI 100 [ _5 ])
                    (const_int 24 [0x18])) [0 MEM[(ucent *)_5 + 24B]+0 S16
A128])
            (const_int 0 [0]))) "test.d":15:16 30 {*cmpti_doubleword}
     (nil))

It's 16-byte aligned.

[Bug d/114171] [13/14 Regression] gdc -O2 -mavx generates misaligned vmovdqa instruction

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |13.3

[Bug d/114171] [13/14 Regression] gdc -O2 -mavx generates misaligned vmovdqa instruction

[a.horodniceanu at proton dot me]

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

--- Comment #3 from Andrei Horodniceanu <a.horodniceanu at proton dot me> ---
(In reply to Hongtao Liu from comment #2)
> on rtl level,we get
> 
> (insn 7 6 8 2 (set (reg:CCZ 17 flags)
>         (compare:CCZ (mem:TI (plus:DI (reg/f:DI 100 [ _5 ])
>                     (const_int 24 [0x18])) [0 MEM[(ucent *)_5 + 24B]+0 S16
> A128])
>             (const_int 0 [0]))) "test.d":15:16 30 {*cmpti_doubleword}
>      (nil))
> 
> It's 16-byte aligned.

I'm not sure I understand this notation but I think I can pick the mem function
which returns a memory reference at the offset given by the _5 register? plus
24 of 16 byte size and 128 bit alignment. This does match up as the size of the
BreakStatement class is 40 (24 + 16). I don't see how it can say that that
memory is 16-byte aligned as in D code the alignment of the class is 8 but it
may be just that, internally, gcc has a more accurate record.

Could you check the rtl for the same snippet of code but instead of `interface
ASTNode` do `abstract class ASTNode`? This would make BreakStatement be of size
32 instead of 40 and change the 24 offset to 16 which happens to produce a
16-byte aligned memory location. This is the only other interesting thing I hit
when trying to reduce the failing code.

[Bug d/114171] [13/14 Regression] gdc -O2 -mavx generates misaligned vmovdqa instruction

[ibuclaw at gdcproject dot org]

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

--- Comment #4 from Iain Buclaw <ibuclaw at gdcproject dot org> ---
Removed druntime dependency.
---
import gcc.builtins;
struct Token {
    string label;
}
struct BreakStatement {
    ulong pad;
    Token label;
}

pragma(inline, false)
auto newclass()
{
    void *p = __builtin_malloc(BreakStatement.sizeof);
    __builtin_memset(p, 0, BreakStatement.sizeof);
    return cast(BreakStatement*) p;
}

int main ()
{
    auto bn = newclass();
    return bn.label is Token.init;
}
---



Roughly the equivalent C++
---
struct Token {
    struct {
    __SIZE_TYPE__ length;
    const char *ptr;
    } label;
};
struct BreakStatement {
    __UINT64_TYPE__ pad;
    Token label;
};

__attribute__((noinline))
BreakStatement *newclass()
{
    void *p = __builtin_malloc(sizeof(BreakStatement));
    __builtin_memset(p, 0, sizeof(BreakStatement));
    return (BreakStatement*) p;
}

int main ()
{
    auto bn = newclass();
    auto init = Token();
    return *(__uint128_t*)&bn->label == *(__uint128_t*)&init;
}
---

[Bug d/114171] [13/14 Regression] gdc -O2 -mavx generates misaligned vmovdqa instruction

[rguenth at gcc dot gnu.org]

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

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Iain Buclaw from comment #4)
> Removed druntime dependency.
> ---
> import gcc.builtins;
> struct Token {
>     string label;
> }
> struct BreakStatement {
>     ulong pad;
>     Token label;
> }
> 
> pragma(inline, false)
> auto newclass()
> {
>     void *p = __builtin_malloc(BreakStatement.sizeof);
>     __builtin_memset(p, 0, BreakStatement.sizeof);
>     return cast(BreakStatement*) p;
> }
> 
> int main ()
> {
>     auto bn = newclass();
>     return bn.label is Token.init;
> }
> ---
> 
> 
> 
> Roughly the equivalent C++
> ---
> struct Token {
>     struct {
>     __SIZE_TYPE__ length;
>     const char *ptr;
>     } label;
> };
> struct BreakStatement {
>     __UINT64_TYPE__ pad;
>     Token label;
> };
> 
> __attribute__((noinline))
> BreakStatement *newclass()
> {
>     void *p = __builtin_malloc(sizeof(BreakStatement));
>     __builtin_memset(p, 0, sizeof(BreakStatement));
>     return (BreakStatement*) p;
> }
> 
> int main ()
> {
>     auto bn = newclass();
>     auto init = Token();
>     return *(__uint128_t*)&bn->label == *(__uint128_t*)&init;
> }
> ---

Unless gdc somehow guarantees bn->label and init are 128bit aligned
then "casting" this way is broken.  You can of course use
build_aligned_type to build a properly (mis-)aligned type to use
to dereference to.

[Bug d/114171] [13/14 Regression] gdc -O2 -mavx generates misaligned vmovdqa instruction

[ibuclaw at gdcproject dot org]

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

--- Comment #6 from Iain Buclaw <ibuclaw at gdcproject dot org> ---
(In reply to Richard Biener from comment #5)
> Unless gdc somehow guarantees bn->label and init are 128bit aligned
> then "casting" this way is broken.  You can of course use
> build_aligned_type to build a properly (mis-)aligned type to use
> to dereference to.
Right, it looks like the lowering for struct comparisons wasn't taking the
original alignment into account when doing identity comparisons of struct-like
fields.

---
--- a/gcc/d/d-codegen.cc
+++ b/gcc/d/d-codegen.cc
@@ -1006,6 +1006,7 @@ lower_struct_comparison (tree_code code,
StructDeclaration *sd,
              if (tmode == NULL_TREE)
                tmode = make_unsigned_type (GET_MODE_BITSIZE (mode.require
()));

+             tmode = build_aligned_type (tmode, TYPE_ALIGN (stype));
              t1ref = build_vconvert (tmode, t1ref);
              t2ref = build_vconvert (tmode, t2ref);

---


The above change gets reflected in the generated assembly.
---
@@ -326,7 +326,7 @@ _Dmain:
        subq    $8, %rsp
        .cfi_def_cfa_offset 16
        call    _D8pr1141718newclassFNbNiZPSQBa14BreakStatement
-       vmovdqa 8(%rax), %xmm0
+       vmovdqu 8(%rax), %xmm0
        xorl    %eax, %eax
        vptest  %xmm0, %xmm0
        sete    %al
---

[Bug d/114171] [13/14 Regression] gdc -O2 -mavx generates misaligned vmovdqa instruction

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Iain Buclaw <ibuclaw@gcc.gnu.org>:

https://gcc.gnu.org/g:623f52775e677bb3d6e9e7ef97196741dd904b1e

commit r14-9277-g623f52775e677bb3d6e9e7ef97196741dd904b1e
Author: Iain Buclaw <ibuclaw@gdcproject.org>
Date:   Sun Mar 3 02:26:37 2024 +0100

    d: Fix gdc -O2 -mavx generates misaligned vmovdqa instruction [PR114171]

            PR d/114171

    gcc/d/ChangeLog:

            * d-codegen.cc (lower_struct_comparison): Keep alignment of
original
            type in reinterpret cast for comparison.

    gcc/testsuite/ChangeLog:

            * gdc.dg/torture/pr114171.d: New test.

[Bug d/114171] [13/14 Regression] gdc -O2 -mavx generates misaligned vmovdqa instruction

[cvs-commit at gcc dot gnu.org]

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

--- Comment #8 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by Iain Buclaw
<ibuclaw@gcc.gnu.org>:

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

commit r13-8401-gcdcbc56c3f5a04e4e7cccdc70a420bc069a0941f
Author: Iain Buclaw <ibuclaw@gdcproject.org>
Date:   Sun Mar 3 02:26:37 2024 +0100

    d: Fix gdc -O2 -mavx generates misaligned vmovdqa instruction [PR114171]

            PR d/114171

    gcc/d/ChangeLog:

            * d-codegen.cc (lower_struct_comparison): Keep alignment of
original
            type in reinterpret cast for comparison.

    gcc/testsuite/ChangeLog:

            * gdc.dg/torture/pr114171.d: New test.

    (cherry picked from commit 623f52775e677bb3d6e9e7ef97196741dd904b1e)

[Bug d/114171] [13/14 Regression] gdc -O2 -mavx generates misaligned vmovdqa instruction

[cvs-commit at gcc dot gnu.org]

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

--- Comment #9 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by Iain Buclaw
<ibuclaw@gcc.gnu.org>:

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

commit r12-10191-gff9d13e0110b46b39cacb431926515cf4be3aa8d
Author: Iain Buclaw <ibuclaw@gdcproject.org>
Date:   Sun Mar 3 02:26:37 2024 +0100

    d: Fix gdc -O2 -mavx generates misaligned vmovdqa instruction [PR114171]

            PR d/114171

    gcc/d/ChangeLog:

            * d-codegen.cc (lower_struct_comparison): Keep alignment of
original
            type in reinterpret cast for comparison.

    gcc/testsuite/ChangeLog:

            * gdc.dg/torture/pr114171.d: New test.

    (cherry picked from commit 623f52775e677bb3d6e9e7ef97196741dd904b1e)

[Bug d/114171] [13/14 Regression] gdc -O2 -mavx generates misaligned vmovdqa instruction

[cvs-commit at gcc dot gnu.org]

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

--- Comment #10 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Iain Buclaw
<ibuclaw@gcc.gnu.org>:

https://gcc.gnu.org/g:3e60064a03a1a6d38ceb5ca4eb7e1f4d30a8aed1

commit r11-11266-g3e60064a03a1a6d38ceb5ca4eb7e1f4d30a8aed1
Author: Iain Buclaw <ibuclaw@gdcproject.org>
Date:   Sun Mar 3 02:26:37 2024 +0100

    d: Fix gdc -O2 -mavx generates misaligned vmovdqa instruction [PR114171]

            PR d/114171

    gcc/d/ChangeLog:

            * d-codegen.cc (lower_struct_comparison): Keep alignment of
original
            type in reinterpret cast for comparison.

    gcc/testsuite/ChangeLog:

            * gdc.dg/torture/pr114171.d: New test.

    (cherry picked from commit 623f52775e677bb3d6e9e7ef97196741dd904b1e)

[Bug d/114171] [13/14 Regression] gdc -O2 -mavx generates misaligned vmovdqa instruction

[ibuclaw at gdcproject dot org]

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

Iain Buclaw <ibuclaw at gdcproject dot org> changed:

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

--- Comment #11 from Iain Buclaw <ibuclaw at gdcproject dot org> ---
Fix committed and backported.