On Tue, Dec 05, 2023 at 07:55:37AM +0100, Jakub Jelinek wrote:
> Hi!
>
> 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. Or should I create some helper function
> (how to call it) and call it from all of cp_parser_pragma_unroll,
> tsubst_stmt (here) and tsubst_expr (ANNOTATE_EXPR)?
> Another option is diagnose it instead when we create the ANNOTATE_EXPRs,
> but unfortunately that is in 3 different places. And at least for the
> non-template case we'd have worse location_t.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux.
>
> 2023-12-04 Jakub Jelinek <ja...@redhat.com>
>
> PR c++/112795
> * pt.cc (tsubst_stmt) <case RANGE_FOR_STMT>: Perform RANGE_FOR_UNROLL
> value checking here as well.
>
> * 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.
>
> --- gcc/cp/pt.cc.jj 2023-12-04 08:59:06.000000000 +0100
> +++ gcc/cp/pt.cc 2023-12-04 10:49:38.149254907 +0100
> @@ -18407,22 +18407,46 @@ tsubst_stmt (tree t, tree args, tsubst_f
> complain, in_decl, decomp);
> }
>
> + tree unroll = RECUR (RANGE_FOR_UNROLL (t));
> + if (unroll)
> + {
> + HOST_WIDE_INT lunroll;
> + if (type_dependent_expression_p (unroll))
> + ;
> + else if (!INTEGRAL_TYPE_P (TREE_TYPE (unroll))
> + || (!value_dependent_expression_p (unroll)
> + && (!tree_fits_shwi_p (unroll)
> + || (lunroll = tree_to_shwi (unroll)) < 0
> + || lunroll >= USHRT_MAX)))
> + {
> + error_at (EXPR_LOCATION (RANGE_FOR_UNROLL (t)),
> + "%<#pragma GCC unroll%> requires an "
> + "assignment-expression that evaluates to a "
> + "non-negative integral constant less than %u",
> + USHRT_MAX);
> + unroll = integer_one_node;
> + }
> + else if (TREE_CODE (unroll) == INTEGER_CST)
> + {
> + unroll = fold_convert (integer_type_node, unroll);
> + if (integer_zerop (unroll))
> + unroll = integer_one_node;
> + }
> + }
As you note, this is exactly the same code we already have in the
ANNOTATE_EXPR case. Maybe add check_and_convert_unroll value?
Marek
