[Bug rtl-optimization/95859] New: Statically true asserts not recognized as such with -O2, but with -O1, -Og, -O3

[Bug rtl-optimization/95859] New: Statically true asserts not recognized as such with -O2, but with -O1, -Og, -O3

[tobi at gcc dot gnu.org]

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

            Bug ID: 95859
           Summary: Statically true asserts not recognized as such with
                    -O2, but with -O1, -Og, -O3
           Product: gcc
           Version: 10.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: rtl-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: tobi at gcc dot gnu.org
  Target Milestone: ---

With -O2 compiling for x86-64, the compiler doesn't recognize that a number of
assertions in the code are true and they end up as function calls in the
assembly output.  They are removed with -O1, -Og, -O3 but not with -O2 or -O2 +
"all the additional compiler flags for -O3 given in the documentation" (that's
a documentation bug on top of this misoptimization).

The asserts look something like this in the assembled form:
   template <N>
   assertion(int n) {
      assert(n == N);
   }
and are only called with compile-time constant arguments, i.e. assertion<3>(3)
and the like.

The final output from the tree passes contains the assertion calls at all
optimization levels, so the rtl optimizers appear to perform worst in -O2 in
this particular case.  I don't see why the calls couldn't be removed before
lowering to RTL.

I'm not sure how to extract preprocessed output form the compiler explorer, but
will provide it once I can put my hands on a recent gcc.

Source code reproduced below, the compiler explorer link is here:
https://godbolt.org/z/REJDhy

#include <Eigen/Geometry>

struct m34 {
    float m[3][4];
};
typedef Eigen::Transform<double, 3, Eigen::AffineCompact, Eigen::DontAlign>
unalignedTrafo3d;
using Trafo3d = unalignedTrafo3d;


Trafo3d func34(m34 mat)
{
    using mapType = Eigen::Map<Eigen::Matrix<float, 3, 4, Eigen::RowMajor>>;

    // should compile to a conversion of 12 floats to double.
    auto mR = Trafo3d{ mapType{ (float*)mat.m }.cast<double>() };
    return mR;    
}

[Bug ipa/95859] Statically true asserts not recognized as such with -O2, but with -O1, -Og, -O3

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
                 CC|                            |marxin at gcc dot gnu.org
   Last reconfirmed|                            |2020-06-24
          Component|rtl-optimization            |ipa
             Status|UNCONFIRMED                 |WAITING
           Keywords|                            |missed-optimization

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Hmm, from the compiler explorer link it looks like the functions are called
with
runtime values so whether the assertion holds is not known.  Now the question
is why we don't inline them.

We need preprocessed source to investigate.

[Bug ipa/95859] Statically true asserts not recognized as such with -O2, but with -O1, -Og, -O3

[tobi at gcc dot gnu.org]

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

--- Comment #2 from Tobias Schlüter <tobi at gcc dot gnu.org> ---
Created attachment 48779
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48779&action=edit
Preprocessed source x86 (32bit)

Here's preprocessed source for x86 (32 bit).  This shows the same behavior as
verified with the compiler explorer.

[Bug ipa/95859] Statically true asserts not recognized as such with -O2, but with -O1, -Og, -O3

[tobi at gcc dot gnu.org]

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

--- Comment #3 from Tobias Schlüter <tobi at gcc dot gnu.org> ---
ps here's the compiler explorer link for the preprocessed test. 
https://godbolt.org/z/zDw5JQ

I tried manually reducing it, but couldn't easily get below the 1000kib limit.

[Bug ipa/95859] Statically true asserts not recognized as such with -O2, but with -O1, -Og, -O3

[tobi at gcc dot gnu.org]

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

--- Comment #4 from Tobias Schlüter <tobi at gcc dot gnu.org> ---
This is a regression. 9.3 works, producing this assembly with -O2 -m32:
func34(m34):
        fld     DWORD PTR [esp+8]
        mov     eax, DWORD PTR [esp+4]
        fstp    QWORD PTR [eax]
        fld     DWORD PTR [esp+24]
        fstp    QWORD PTR [eax+8]
        fld     DWORD PTR [esp+40]
        fstp    QWORD PTR [eax+16]
        fld     DWORD PTR [esp+12]
        fstp    QWORD PTR [eax+24]
        fld     DWORD PTR [esp+28]
        fstp    QWORD PTR [eax+32]
        fld     DWORD PTR [esp+44]
        fstp    QWORD PTR [eax+40]
        fld     DWORD PTR [esp+16]
        fstp    QWORD PTR [eax+48]
        fld     DWORD PTR [esp+32]
        fstp    QWORD PTR [eax+56]
        fld     DWORD PTR [esp+48]
        fstp    QWORD PTR [eax+64]
        fld     DWORD PTR [esp+20]
        fstp    QWORD PTR [eax+72]
        fld     DWORD PTR [esp+36]
        fstp    QWORD PTR [eax+80]
        fld     DWORD PTR [esp+52]
        fstp    QWORD PTR [eax+88]
        ret     4

I've also thrown some other simple Eigen codes at the compiler, and 10.1 and
trunk consistently keep the asserts in place.

[Bug ipa/95859] [10/11 regression] Statically true asserts not recognized as such with -O2, but with -O1, -Og, -O3

[tobi at gcc dot gnu.org]

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

Tobias Schlüter <tobi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Statically true asserts not |[10/11 regression]
                   |recognized as such with     |Statically true asserts not
                   |-O2, but with -O1, -Og, -O3 |recognized as such with
                   |                            |-O2, but with -O1, -Og, -O3

--- Comment #5 from Tobias Schlüter <tobi at gcc dot gnu.org> ---
Added the regression tag to .

[Bug ipa/95859] [10/11 regression] Statically true asserts not recognized as such with -O2, but with -O1, -Og, -O3

[tobi at gcc dot gnu.org]

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

--- Comment #6 from Tobias Schlüter <tobi at gcc dot gnu.org> ---
Added the regression tag to the title.

[Bug ipa/95859] [10/11 regression] Statically true asserts not recognized as such with -O2, but with -O1, -Og, -O3

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |NEW
      Known to work|                            |9.3.0
   Target Milestone|---                         |10.2

[Bug ipa/95859] [10/11 regression] Statically true asserts not recognized as such with -O2, but with -O1, -Og, -O3

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|10.2                        |10.3

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 10.2 is released, adjusting target milestone.

[Bug ipa/95859] [10/11 regression] Statically true asserts not recognized as such with -O2, but with -O1, -Og, -O3

[tobi at gcc dot gnu.org]

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

Tobias Schlüter <tobi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #48779|0                           |1
        is obsolete|                            |

--- Comment #8 from Tobias Schlüter <tobi at gcc dot gnu.org> ---
Created attachment 48927
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48927&action=edit
Reduced testcase (32bit, preprocessed).

I've further reduced the testcase down to 800Kb.  Again this is with -m32.  I
removed the definition of void Eigen::internal::check_for_aliasing, so this
will appear in the assembly, but it's a no-op.

New version attached, compiler explorer version here
https://godbolt.org/z/h6YMeq

I've also reproduced the bug with 10.2

[Bug ipa/95859] [10/11 regression] Statically true asserts not recognized as such with -O2, but with -O1, -Og, -O3

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2

[Bug ipa/95859] [10/11 regression] Statically true asserts not recognized as such with -O2, but with -O1, -Og, -O3

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hubicka at gcc dot gnu.org,
                   |                            |jakub at gcc dot gnu.org

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Bisection on #c8 shows r10-4944-g1e83bd7003e03160b7d71fb959111e89b53446ab

[Bug ipa/95859] [10/11 regression] Statically true asserts not recognized as such with -O2, but with -O1, -Og, -O3

[jakub at gcc dot gnu.org]

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

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Honza, could you please have a look?

[Bug ipa/95859] [10/11 regression] Statically true asserts not recognized as such with -O2, but with -O1, -Og, -O3

[tobi at gcc dot gnu.org]

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

--- Comment #11 from Tobias Schlüter <tobi at gcc dot gnu.org> ---
Works on trunk now but not 10.2.  Compiler explorer link:
https://godbolt.org/z/1zbh4YM4W

On the trunk we get the following.  I'm guessing that one could enhance the
read pattern by using more registers, but without benchmarking I don't believe
that this can be beat: 
func34(m34):
        pxor    xmm0, xmm0
        mov     rax, rdi
        cvtss2sd        xmm0, DWORD PTR [rsp+8]
        movsd   QWORD PTR [rdi], xmm0
        pxor    xmm0, xmm0
        cvtss2sd        xmm0, DWORD PTR [rsp+24]
        movsd   QWORD PTR [rdi+8], xmm0
        pxor    xmm0, xmm0
        cvtss2sd        xmm0, DWORD PTR [rsp+40]
        movsd   QWORD PTR [rdi+16], xmm0
        pxor    xmm0, xmm0
        cvtss2sd        xmm0, DWORD PTR [rsp+12]
        movsd   QWORD PTR [rdi+24], xmm0
        pxor    xmm0, xmm0
        cvtss2sd        xmm0, DWORD PTR [rsp+28]
        movsd   QWORD PTR [rdi+32], xmm0
        pxor    xmm0, xmm0
        cvtss2sd        xmm0, DWORD PTR [rsp+44]
        movsd   QWORD PTR [rdi+40], xmm0
        pxor    xmm0, xmm0
        cvtss2sd        xmm0, DWORD PTR [rsp+16]
        movsd   QWORD PTR [rdi+48], xmm0
        pxor    xmm0, xmm0
        cvtss2sd        xmm0, DWORD PTR [rsp+32]
        movsd   QWORD PTR [rdi+56], xmm0
        pxor    xmm0, xmm0
        cvtss2sd        xmm0, DWORD PTR [rsp+48]
        movsd   QWORD PTR [rdi+64], xmm0
        pxor    xmm0, xmm0
        cvtss2sd        xmm0, DWORD PTR [rsp+20]
        movsd   QWORD PTR [rdi+72], xmm0
        pxor    xmm0, xmm0
        cvtss2sd        xmm0, DWORD PTR [rsp+36]
        movsd   QWORD PTR [rdi+80], xmm0
        pxor    xmm0, xmm0
        cvtss2sd        xmm0, DWORD PTR [rsp+52]
        movsd   QWORD PTR [rdi+88], xmm0
        ret
Thanks to whoever did that.

I see that a release candidate for 10.2.1 has been cut.  I would assume that
it's not fixed in 10.2.1 because there would be a bugfix mentioned here.  My
experience is clearly not representative and I can appreciate that there was no
deluge of performance regression PRs, but I would think that Eigen is an
important enough library that one should consider whether breaking it like this
is really something that should survive several (sub-)releases.

[Bug ipa/95859] [10/11 regression] Statically true asserts not recognized as such with -O2, but with -O1, -Og, -O3

[tobi at gcc dot gnu.org]

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

--- Comment #12 from Tobias Schlüter <tobi at gcc dot gnu.org> ---
Because I posted 32bit code before, here is what the trunk does with -m32
func34(m34):
        fld     DWORD PTR [esp+8]
        mov     eax, DWORD PTR [esp+4]
        fstp    QWORD PTR [eax]
        fld     DWORD PTR [esp+24]
        fstp    QWORD PTR [eax+8]
        fld     DWORD PTR [esp+40]
        fstp    QWORD PTR [eax+16]
        fld     DWORD PTR [esp+12]
        fstp    QWORD PTR [eax+24]
        fld     DWORD PTR [esp+28]
        fstp    QWORD PTR [eax+32]
        fld     DWORD PTR [esp+44]
        fstp    QWORD PTR [eax+40]
        fld     DWORD PTR [esp+16]
        fstp    QWORD PTR [eax+48]
        fld     DWORD PTR [esp+32]
        fstp    QWORD PTR [eax+56]
        fld     DWORD PTR [esp+48]
        fstp    QWORD PTR [eax+64]
        fld     DWORD PTR [esp+20]
        fstp    QWORD PTR [eax+72]
        fld     DWORD PTR [esp+36]
        fstp    QWORD PTR [eax+80]
        fld     DWORD PTR [esp+52]
        fstp    QWORD PTR [eax+88]
        ret     4
This is the same as the assembly from 9.3 in Comment 4.

[Bug ipa/95859] [10/11 regression] Statically true asserts not recognized as such with -O2, but with -O1, -Og, -O3

[tobi at gcc dot gnu.org]

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

--- Comment #13 from Tobias Schlüter <tobi at gcc dot gnu.org> ---
I'm sorry to say that the problem is NOT fixed on the trunk.  With "ARM64 gcc
trunk" on the compiler explorer, we get the below.  OTOH 9.3 produces perfect
code.  Compiler explorer link: https://godbolt.org/z/56h67rfc4

.LC0:
        .string "Eigen::internal::variable_if_dynamic<T,
Value>::variable_if_dynamic(T) [with T = long int; int Value = 0]"
.LC1:
        .string
"/opt/compiler-explorer/libs/eigen/v3.3.9/Eigen/src/Core/util/XprHelper.h"
.LC2:
        .string "v == T(Value)"
.LC3:
        .string "Eigen::internal::variable_if_dynamic<T,
Value>::variable_if_dynamic(T) [with T = long int; int Value = 3]"
.LC4:
        .string "Eigen::internal::variable_if_dynamic<T,
Value>::variable_if_dynamic(T) [with T = long int; int Value = 4]"
.LC5:
        .string "Eigen::internal::variable_if_dynamic<T,
Value>::variable_if_dynamic(T) [with T = long int; int Value = 1]"
func34(m34):
        stp     x29, x30, [sp, -128]!
        mov     x1, 0
        mov     x29, sp
        stp     x19, x20, [sp, 16]
        mov     x20, x8
        mov     x19, x0
        add     x0, sp, 40
        bl      Eigen::internal::variable_if_dynamic<long,
0>::variable_if_dynamic(long) [complete object constructor]
        add     x0, sp, 41
        mov     x1, 0
        bl      Eigen::internal::variable_if_dynamic<long,
0>::variable_if_dynamic(long) [complete object constructor]
        str     x19, [sp, 48]
        add     x0, sp, 56
        mov     x1, 3
        bl      Eigen::internal::variable_if_dynamic<long,
3>::variable_if_dynamic(long) [complete object constructor]
        add     x0, sp, 57
        mov     x1, 4
        bl      Eigen::internal::variable_if_dynamic<long,
4>::variable_if_dynamic(long) [complete object constructor]
        add     x0, sp, 58
        mov     x1, 0
        bl      Eigen::internal::variable_if_dynamic<long,
0>::variable_if_dynamic(long) [complete object constructor]
        add     x0, sp, 59
        mov     x1, 0
        bl      Eigen::internal::variable_if_dynamic<long,
0>::variable_if_dynamic(long) [complete object constructor]
        str     x19, [sp, 80]
        add     x0, sp, 90
        mov     x1, 0
        bl      Eigen::internal::variable_if_dynamic<long,
0>::variable_if_dynamic(long) [complete object constructor]
        add     x0, sp, 91
        mov     x1, 0
        bl      Eigen::internal::variable_if_dynamic<long,
0>::variable_if_dynamic(long) [complete object constructor]
        str     x19, [sp, 112]
        add     x0, sp, 120
        mov     x1, 1
        bl      Eigen::internal::variable_if_dynamic<long,
1>::variable_if_dynamic(long) [complete object constructor]
        add     x0, sp, 121
        mov     x1, 4
        bl      Eigen::internal::variable_if_dynamic<long,
4>::variable_if_dynamic(long) [complete object constructor]
        str     x20, [sp, 64]
        add     x0, sp, 72
        mov     x1, 3
        bl      Eigen::internal::variable_if_dynamic<long,
3>::variable_if_dynamic(long) [complete object constructor]
        ldp     s18, s16, [x19]
        mov     x0, x20
        ldp     s4, s1, [x19, 8]
        ldp     s17, s6, [x19, 16]
        fcvt    d18, s18
        ldp     s3, s0, [x19, 24]
        fcvt    d16, s16
        ldp     s7, s5, [x19, 32]
        fcvt    d4, s4
        ldr     s2, [x19, 40]
        fcvt    d1, s1
        fcvt    d17, s17
        fcvt    d6, s6
        fcvt    d3, s3
        fcvt    d7, s7
        fcvt    d5, s5
        fcvt    d2, s2
        fcvt    d0, s0
        stp     d18, d17, [x20]
        stp     d7, d16, [x20, 16]
        stp     d6, d5, [x20, 32]
        stp     d4, d3, [x20, 48]
        stp     d2, d1, [x20, 64]
        str     d0, [x20, 80]
        ldr     s0, [x19, 44]
        fcvt    d0, s0
        str     d0, [x20, 88]
        ldp     x19, x20, [sp, 16]
        ldp     x29, x30, [sp], 128
        ret
_GLOBAL__sub_I_func34(m34):
        ret

[Bug ipa/95859] [10/11 regression] Statically true asserts not recognized as such with -O2, but with -O1, -Og, -O3

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|10.3                        |10.4

--- Comment #14 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 10.3 is being released, retargeting bugs to GCC 10.4.

[Bug ipa/95859] [10/11/12 regression] Statically true asserts not recognized as such with -O2, but with -O1, -Og, -O3

[tobi at gcc dot gnu.org]

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

Tobias Schlüter <tobi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |10.3.0, 11.1.0

--- Comment #15 from Tobias Schlüter <tobi at gcc dot gnu.org> ---
Seems to be fixed also on ARM64 in 10.3, 11.1

[Bug ipa/95859] [10/11/12 regression] Statically true asserts not recognized as such with -O2, but with -O1, -Og, -O3

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
   Target Milestone|10.4                        |10.3
         Resolution|---                         |DUPLICATE

--- Comment #16 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
This is an exact dup of bug 98265.

*** This bug has been marked as a duplicate of bug 98265 ***