[Bug c++/112795] New: [C++>=14] ICE pragma GCC unroll (n) cxx_eval_constant_expression

[Bug c++/112795] New: [C++>=14] ICE pragma GCC unroll (n) cxx_eval_constant_expression

[magnus.hegdahl at gmail dot com]

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

            Bug ID: 112795
           Summary: [C++>=14] ICE pragma GCC unroll (n)
                    cxx_eval_constant_expression
           Product: gcc
           Version: 8.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: magnus.hegdahl at gmail dot com
  Target Milestone: ---

Compiling the following code with GCC >= 8.1 (when #pragma GCC unroll was
added), and -std=c++14 or higher results in an internal compiler error:
```
constexpr int n = 1;
template <class = void>
void f() {
#pragma GCC unroll(n)
    for (int i = 0; i != n; ++i) {
    }
}
```

It does not fail if f is not a template, if n is not parenthesized, or if using
-std=c++11.

[Bug c++/112795] [C++>=14] ICE pragma GCC unroll (n) cxx_eval_constant_expression

[magnus.hegdahl at gmail dot com]

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

--- Comment #1 from Magnus Hokland Hegdahl <magnus.hegdahl at gmail dot com> ---
Full output:
// Target: x86_64-pc-linux-gnu
// Configured with: /build/gcc/src/gcc/configure
--enable-languages=ada,c,c++,d,fortran,go,lto,objc,obj-c++ --enable-bootstrap
--prefix=/usr --libdir=/usr/lib --libexecdir=/usr/lib --mandir=/usr/share/man
--infodir=/usr/share/info --with-bugurl=https://bugs.archlinux.org/
--with-build-config=bootstrap-lto --with-linker-hash-style=gnu
--with-system-zlib --enable-__cxa_atexit --enable-cet=auto
--enable-checking=release --enable-clocale=gnu --enable-default-pie
--enable-default-ssp --enable-gnu-indirect-function --enable-gnu-unique-object
--enable-libstdcxx-backtrace --enable-link-serialization=1
--enable-linker-build-id --enable-lto --enable-multilib --enable-plugin
--enable-shared --enable-threads=posix --disable-libssp --disable-libstdcxx-pch
--disable-werror
// Thread model: posix
// Supported LTO compression algorithms: zlib zstd
// gcc version 13.2.1 20230801 (GCC) 
// 
// mwe.cpp: In function ‘void f()’:
// mwe.cpp:5:21: internal compiler error: in cxx_eval_constant_expression, at
cp/constexpr.cc:7619
//     5 | #pragma GCC unroll(n)
//       |                     ^
// 0x1ad33c8 internal_error(char const*, ...)
//      ???:0
// 0x6b7b63 fancy_abort(char const*, int, char const*)
//      ???:0
// 0x711364 maybe_constant_value(tree_node*, tree_node*, mce_value)
//      ???:0
// 0x9443d0 c_common_parse_file()
//      ???:0
// Please submit a full bug report, with preprocessed source.
// Please include the complete backtrace with any bug report.
// See <https://bugs.archlinux.org/> for instructions.

// /usr/lib/gcc/x86_64-pc-linux-gnu/13.2.1/cc1plus -quiet -D_GNU_SOURCE mwe.cpp
-quiet -dumpbase mwe.cpp -dumpbase-ext .cpp -mtune=generic -march=x86-64 -Wall
-Wextra -Wpedantic -freport-bug -o - -frandom-seed=0 -fdump-noaddr

# 0 "mwe.cpp"
# 0 "<built-in>"
# 0 "<command-line>"
# 1 "/usr/include/stdc-predef.h" 1 3 4
# 0 "<command-line>" 2
# 1 "mwe.cpp"
constexpr int n = 1;

template <class = void>
void f() {
#pragma GCC unroll(n)
    for (int i = 0; i != n; ++i) {
    }
}

[Bug c++/112795] [C++>=14] ICE pragma GCC unroll (n) cxx_eval_constant_expression

[mpolacek at gcc dot gnu.org]

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

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mpolacek at gcc dot gnu.org
             Status|UNCONFIRMED                 |NEW
   Target Milestone|---                         |11.5
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2023-12-01

--- Comment #2 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Started with r8-5544-g170a8bd604a4c9:

commit 170a8bd604a4c960b8a378d50f7cc6c66cbdaf5c
Author: Eric Botcazou <ebotcazou@gcc.gnu.org>
Date:   Fri Dec 22 10:22:15 2017 +0000

    extend.texi (Loop-Specific Pragmas): Document pragma GCC unroll.

[Bug c++/112795] [C++>=14] ICE pragma GCC unroll (n) cxx_eval_constant_expression

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |ebotcazou at gcc dot gnu.org,
                   |                            |jakub at gcc dot gnu.org
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The bug is in calling
  expr = maybe_constant_value (expr);
That is something that shouldn't be done in code possibly parsed in templates,
unless !processing_template_decl.
A quick fix would be to use
  expr = fold_non_dependent_expr (expr);
and I guess I'll just submit a patch with that.
That is e.g. what we do in OpenMP collapse clause parsing where we really want
a constant already during parsing because how we parse the following code
depends on that.
If that is not the case in #pragma GCC unroll, the normal C++ way would be to
only
check if the result is INTEGER_CST etc. if it is not value dependent expr,
otherwise
pass down a tree and during instantiation instantiate that tree.
That way
template <int N>
void
foo ()
{
#pragma GCC unroll(N)
  for (int i = 0; i != N; ++i)
    ;
}
would be possible.

[Bug c++/112795] [C++>=14] ICE pragma GCC unroll (n) cxx_eval_constant_expression

[jakub at gcc dot gnu.org]

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 56754
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=56754&action=edit
gcc14-pr112795.patch

Untested full fix.  If we backport at all, I think we could just change the
maybe_const_value to fold_non_dependent_expr.

[Bug c++/112795] [C++>=14] ICE pragma GCC unroll (n) cxx_eval_constant_expression

[ebotcazou at gcc dot gnu.org]

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

--- Comment #5 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
Thanks for looking into this, Jakub.  IIRC I didn't write this C++ support, it
was already there in Mike's original implementation.

[Bug c++/112795] [C++>=14] ICE pragma GCC unroll (n) cxx_eval_constant_expression

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

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

commit r14-6107-gb6c78feea08c36e5754818c6a3d7536b3f8913dc
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Mon Dec 4 08:59:15 2023 +0100

    c++: #pragma GCC unroll C++ fixes [PR112795]

    foo in the unroll-5.C testcase ICEs because cp_parser_pragma_unroll
    during parsing calls maybe_constant_value unconditionally, which is
    fine if !processing_template_decl, but can ICE otherwise.

    While just calling fold_non_dependent_expr there instead could be enough
    to fix the ICE (and I guess the right thing to do for backports if any),
    I don't see a reason why we couldn't handle a dependent #pragma GCC unroll
    argument as well, the unrolling isn't done in the FE and all the middle-end
    cares about is that ANNOTATE_EXPR has a 1..65534 last operand when it is
    annot_expr_unroll_kind.

    So, the following patch changes all the unsigned short unroll arguments
    to tree unroll (and thus avoids the tree -> unsigned short -> tree
    conversions), does the type and value checking during parsing only if
    the argument isn't dependent and repeats it during instantiation.

    2023-12-04  Jakub Jelinek  <jakub@redhat.com>

            PR c++/112795
    gcc/cp/
            * cp-tree.h (cp_convert_range_for): Change UNROLL type from
            unsigned short to tree.
            (finish_while_stmt_cond, finish_do_stmt, finish_for_cond):
Likewise.
            * parser.cc (cp_parser_statement): Pass NULL_TREE rather than 0 to
            cp_parser_iteration_statement UNROLL argument.
            (cp_parser_for, cp_parser_c_for): Change UNROLL type from
            unsigned short to tree.
            (cp_parser_range_for): Likewise.  Set RANGE_FOR_UNROLL to just
UNROLL
            rather than build_int_cst from it.
            (cp_convert_range_for, cp_parser_iteration_statement): Change
UNROLL
            type from unsigned short to tree.
            (cp_parser_omp_loop_nest): Pass NULL_TREE rather than 0 to
            cp_parser_range_for UNROLL argument.
            (cp_parser_pragma_unroll): Return tree rather than unsigned short.
            If parsed expression is type dependent, just return it, don't
diagnose
            issues with value if it is value dependent.
            (cp_parser_pragma): Change UNROLL type from unsigned short to tree.
            * semantics.cc (finish_while_stmt_cond): Change UNROLL type from
            unsigned short to tree.  Build ANNOTATE_EXPR with UNROLL as its
last
            operand rather than build_int_cst from it.
            (finish_do_stmt, finish_for_cond): Likewise.
            * pt.cc (tsubst_stmt) <case RANGE_FOR_STMT>: Change UNROLL type
from
            unsigned short to tree and set it to RECUR on RANGE_FOR_UNROLL (t).
            (tsubst_expr) <case ANNOTATE_EXPR>: For annot_expr_unroll_kind
repeat
            checks on UNROLL value from cp_parser_pragma_unroll.
    gcc/testsuite/
            * g++.dg/ext/unroll-5.C: New test.
            * g++.dg/ext/unroll-6.C: New test.

[Bug c++/112795] [C++>=14] ICE pragma GCC unroll (n) cxx_eval_constant_expression

[jakub at gcc dot gnu.org]

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

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed for GCC 14+, backports if any will be more limited.

[Bug c++/112795] [C++>=14] ICE pragma GCC unroll (n) cxx_eval_constant_expression

[cvs-commit at gcc dot gnu.org]

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

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

https://gcc.gnu.org/g:5b651cf6bf1903dd3a4a5ce8c2a95d0e76ee0089

commit r13-8124-g5b651cf6bf1903dd3a4a5ce8c2a95d0e76ee0089
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Mon Dec 4 08:59:15 2023 +0100

    c++: #pragma GCC unroll C++ fixes [PR112795]

    foo in the unroll-5.C testcase ICEs because cp_parser_pragma_unroll
    during parsing calls maybe_constant_value unconditionally, which is
    fine if !processing_template_decl, but can ICE otherwise.

    While just calling fold_non_dependent_expr there instead could be enough
    to fix the ICE (and I guess the right thing to do for backports if any),
    I don't see a reason why we couldn't handle a dependent #pragma GCC unroll
    argument as well, the unrolling isn't done in the FE and all the middle-end
    cares about is that ANNOTATE_EXPR has a 1..65534 last operand when it is
    annot_expr_unroll_kind.

    So, the following patch changes all the unsigned short unroll arguments
    to tree unroll (and thus avoids the tree -> unsigned short -> tree
    conversions), does the type and value checking during parsing only if
    the argument isn't dependent and repeats it during instantiation.

    2023-12-04  Jakub Jelinek  <jakub@redhat.com>

            PR c++/112795
    gcc/cp/
            * parser.cc (cp_parser_pragma_unroll): Use fold_non_dependent_expr
            instead of maybe_constant_value.
    gcc/testsuite/
            * g++.dg/ext/unroll-5.C: New test.

    (cherry picked from commit b6c78feea08c36e5754818c6a3d7536b3f8913dc)

[Bug c++/112795] [C++>=14] ICE pragma GCC unroll (n) cxx_eval_constant_expression

[cvs-commit at gcc dot gnu.org]

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

--- Comment #9 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:59be79fd596ec8f8fbdf1479bc99e6aba9c52778

commit r14-6193-g59be79fd596ec8f8fbdf1479bc99e6aba9c52778
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Dec 5 22:54:08 2023 +0100

    c++: Further #pragma GCC unroll C++ fix [PR112795]

    When committing the #pragma GCC unroll patch, I found I forgot one spot
    for diagnosting the invalid unrolls - if #pragma GCC unroll argument is
    dependent and the pragma is before a range for loop, the unroll tree (now,
    before one converted form ushort) is saved into RANGE_FOR_UNROLL and
    tsubst_stmt was RECURing on it, but didn't diagnose if it was invalid and
    so we ICEd later in the middle-end when  ANNOTATE_EXPR had unexpected
    argument.

    The following patch fixes that.  So that the diagnostics isn't done in 3
    different places, the patch introduces a new function that both
    cp_parser_pragma_unroll and instantiation of ANNOTATE_EXPR and
RANGE_FOR_STMT
    can use.

    2023-12-05  Jakub Jelinek  <jakub@redhat.com>

            PR c++/112795
            * cp-tree.h (cp_check_pragma_unroll): Declare.
            * semantics.cc (cp_check_pragma_unroll): New function.
            * parser.cc (cp_parser_pragma_unroll): Use cp_check_pragma_unroll.
            * pt.cc (tsubst_expr) <case ANNOTATE_EXPR>: Likewise.
            (tsubst_stmt) <case RANGE_FOR_STMT>: Likwsie.

            * g++.dg/ext/unroll-2.C: Use { target c++11 } instead of dg-skip-if
for
            -std=gnu++98.
            * g++.dg/ext/unroll-3.C: Likewise.
            * g++.dg/ext/unroll-7.C: New test.
            * g++.dg/ext/unroll-8.C: New test.

[Bug c++/112795] [C++>=14] ICE pragma GCC unroll (n) cxx_eval_constant_expression

[jakub at gcc dot gnu.org]

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

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed for GCC 14.1+ (better way) and 13.3+ (just the ICE) for now.

[Bug c++/112795] [C++>=14] ICE pragma GCC unroll (n) cxx_eval_constant_expression

[cvs-commit at gcc dot gnu.org]

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

--- Comment #11 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

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

commit r12-10051-gf1761303c77219281c22ab50b4a9ce65920c4023
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Mon Dec 4 08:59:15 2023 +0100

    c++: #pragma GCC unroll C++ fixes [PR112795]

    foo in the unroll-5.C testcase ICEs because cp_parser_pragma_unroll
    during parsing calls maybe_constant_value unconditionally, which is
    fine if !processing_template_decl, but can ICE otherwise.

    While just calling fold_non_dependent_expr there instead could be enough
    to fix the ICE (and I guess the right thing to do for backports if any),
    I don't see a reason why we couldn't handle a dependent #pragma GCC unroll
    argument as well, the unrolling isn't done in the FE and all the middle-end
    cares about is that ANNOTATE_EXPR has a 1..65534 last operand when it is
    annot_expr_unroll_kind.

    So, the following patch changes all the unsigned short unroll arguments
    to tree unroll (and thus avoids the tree -> unsigned short -> tree
    conversions), does the type and value checking during parsing only if
    the argument isn't dependent and repeats it during instantiation.

    2023-12-04  Jakub Jelinek  <jakub@redhat.com>

            PR c++/112795
    gcc/cp/
            * parser.cc (cp_parser_pragma_unroll): Use fold_non_dependent_expr
            instead of maybe_constant_value.
    gcc/testsuite/
            * g++.dg/ext/unroll-5.C: New test.

    (cherry picked from commit b6c78feea08c36e5754818c6a3d7536b3f8913dc)

[Bug c++/112795] [C++>=14] ICE pragma GCC unroll (n) cxx_eval_constant_expression

[cvs-commit at gcc dot gnu.org]

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

--- Comment #12 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:67762f3d76d42b2d9a436b6279fd111082f47c2b

commit r11-11151-g67762f3d76d42b2d9a436b6279fd111082f47c2b
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Mon Dec 4 08:59:15 2023 +0100

    c++: #pragma GCC unroll C++ fixes [PR112795]

    foo in the unroll-5.C testcase ICEs because cp_parser_pragma_unroll
    during parsing calls maybe_constant_value unconditionally, which is
    fine if !processing_template_decl, but can ICE otherwise.

    While just calling fold_non_dependent_expr there instead could be enough
    to fix the ICE (and I guess the right thing to do for backports if any),
    I don't see a reason why we couldn't handle a dependent #pragma GCC unroll
    argument as well, the unrolling isn't done in the FE and all the middle-end
    cares about is that ANNOTATE_EXPR has a 1..65534 last operand when it is
    annot_expr_unroll_kind.

    So, the following patch changes all the unsigned short unroll arguments
    to tree unroll (and thus avoids the tree -> unsigned short -> tree
    conversions), does the type and value checking during parsing only if
    the argument isn't dependent and repeats it during instantiation.

    2023-12-04  Jakub Jelinek  <jakub@redhat.com>

            PR c++/112795
    gcc/cp/
            * parser.c (cp_parser_pragma_unroll): Use fold_non_dependent_expr
            instead of maybe_constant_value.
    gcc/testsuite/
            * g++.dg/ext/unroll-5.C: New test.

    (cherry picked from commit b6c78feea08c36e5754818c6a3d7536b3f8913dc)