Bug 99091 - local array not prompted to static
Summary: local array not prompted to static
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 10.0
: P3 enhancement
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: missed-optimization
Depends on:
Blocks:
 
Reported: 2021-02-13 21:55 UTC by Barry Revzin
Modified: 2021-12-14 16:42 UTC (History)
7 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2021-08-28 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Barry Revzin 2021-02-13 21:55:26 UTC
Consider the following (link to compiler explorer: https://godbolt.org/z/jWsxsK)

#include <array>

static constexpr auto doy = []{
    std::array<int, 13> month_offset{};
    for (int m=1; m<=12;++m) {
        month_offset[m] = (153*(m > 2 ? m-3 : m+9) + 2)/5;
    }
    return month_offset;
}();

auto foo(int m) -> int {
    #ifdef LOCAL
    constexpr auto doy = []{
        std::array<int, 13> month_offset{};
        for (int m=1; m<=12;++m) {
            month_offset[m] = (153*(m > 2 ? m-3 : m+9) + 2)/5;
        }
        return month_offset;
    }();
    #endif

    return doy[m];
}

This is a fragment of code that does some calculation to figure out the date, which isn't super important. If LOCAL is not defined (i.e. we declare the array as a namespace-scope constexpr), the -O3 codegen of 'foo' is:

foo(int):
        movsx   rdi, edi
        mov     eax, DWORD PTR doy[0+rdi*4]
        ret

However, if we want to move the declaration of doy to be more local to the calculation and compile with -DLOCAL, the codegen instead is (on -O3):

foo(int):
        pxor    xmm0, xmm0
        mov     rax, QWORD PTR .LC0[rip]
        movsx   rdi, edi
        mov     DWORD PTR [rsp-24], 275
        movaps  XMMWORD PTR [rsp-72], xmm0
        movdqa  xmm0, XMMWORD PTR .LC1[rip]
        mov     QWORD PTR [rsp-68], rax
        movaps  XMMWORD PTR [rsp-56], xmm0
        movdqa  xmm0, XMMWORD PTR .LC2[rip]
        movaps  XMMWORD PTR [rsp-40], xmm0
        mov     eax, DWORD PTR [rsp-72+rdi*4]
        ret

This can be alleviated by marked the local variable doy as being static constexpr. Except that this prevents 'foo' from being a constexpr function (can't have static variables). 

This just seems like bad codegen, I'm not sure there's any reason that the compiler needs to do work here. It would be nice if the codegen with a local constexpr variable were the same as if it were a non-local constexpr variable.
Comment 1 Jakub Jelinek 2021-02-14 09:27:44 UTC
I don't see anything wrong on that, the constexpr var is not evaluated at runtime, it has a constant initializer.
So in the end it is
static const int doy1[] = { 306, 337, 31, 61, 92, 122, 153, 184, 214, 245, 275 };
int foo1(int m) { return doy1[m]; }
int foo2(int m) {
  static const int doy2[] = { 306, 337, 31, 61, 92, 122, 153, 184, 214, 245, 275 };
  return doy2[m];
}
int foo3(int m) {
  const int doy3[] = { 306, 337, 31, 61, 92, 122, 153, 184, 214, 245, 275 };
  return doy3[m];
}
and there is nothing C++ specific on it.
The compiler may promote the doy3 variable from automatic variable into static variable if it can prove
it is safe to do so (it would be unsafe e.g. if foo3 could be called recursively and compare the addresses of the var between
the recursive invocations).  GCC does that early (during gimplification) if the variable is not address taken, but
the array reference implies address taking.  And, especially for the original testcase where operator[] is overloaded,
it really can't do better early, so we'd need an optimization that would promote automatic vars to static when possible later (e.g. after inlining) if it can prove it is safe to do so.
Comment 2 Jakub Jelinek 2021-02-15 10:31:00 UTC
Wonder if SRA doesn't have the right infrastructure to optimize this.
In any case, GCC 12 material.
Comment 3 Andrew Pinski 2021-08-28 22:56:29 UTC
Confirmed, there is another bug which is very similar to this one but filed years ago.