On Tue, Dec 05, 2023 at 10:21:18AM -0500, Marek Polacek wrote:
> 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?
I'm too late. Never mind.
Marek
