----- Ursprüngliche Mail -----
> On 07/30/2015 05:00 PM, Kai Tietz wrote:
> > 2015-07-30 18:52 GMT+02:00 Jason Merrill <ja...@redhat.com>:
> >> On 07/29/2015 06:56 PM, Kai Tietz wrote:
> >>>>>>>>>>> @@ -13078,6 +13042,8 @@ build_enumerator (tree name, tree value,
> >>>>>>>>>>> tree
> >>>>>>>>>>> enumtype, tree attributes,
> >>>>>>>>>>> if (value)
> >>>>>>>>>>> STRIP_TYPE_NOPS (value);
> >>>>>>>>>>>
> >>>>>>>>>>> + if (value)
> >>>>>>>>>>> + value = cp_try_fold_to_constant (value);
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> Again, this is unnecessary because we call cxx_constant_value
> >>>>>>>>>> below.
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> See nops, and other unary-operations we want to reduce here to real
> >>>>>>>>> constant value ...
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> The cxx_constant_value call below will deal with them.
> >>>>>>>
> >>>>>>>
> >>>>>>> Likewise for grokbitfield.
> >>>>>>
> >>>>>>
> >>>>>> Hmm, AFAIR we don't call cxx_constant_value in all code-paths. But I
> >>>>>> will look into it, and come back to you on it.
> >>>>>
> >>>>>
> >>>>> I am still on it ... first did the other points
> >>>>
> >>>>
> >>>> Looks like this hasn't changed.
> >>>
> >>>
> >>> Yes, for grokbitfield current version uses fold_simple for witdth. So
> >>> just expressions based on constants getting reduced to short form. In
> >>> grokbitfield I don't see invocation of cxx_constant_value. So how can
> >>> we be sure that width is reduced to integer-cst?
> >>
> >>
> >> We call cxx_constant_value on bit-field widths in check_bitfield_decl.
> >
> > Hmm, ok. But I don't see that this function gets called in context of
> > grokbitfield, after we set DECL_INITIAL.
>
> Nope, it's called later on as part of finish_struct.
>
Ok, adjusted.
> > By removing this folding here, we get new failures in
> > g++.dg/warn/overflow-warn-1.C testcase:
> > New errors are at lin 32 that 'bif-foeld 's::<anonymous>' width not an
> > integer constant'
> > and at same line ''(1 / 0) is not a constant expression'. Those
> > message don't look wrong.
> >
> > The testcase next to this 'overflow-warn-3.C and overflow-warn-4.C'
> > failing in the same manner for (1 / 0) case. But there are no other
> > regressions in g++.dg & libstdc++
> >
> > Shall I extend the testcases for this message?
>
> Please.
Done.
> >>>>>>>>>>> @@ -1947,6 +1947,8 @@ build_complex (tree type, tree real, tree
> >>>>>>>>>>> imag)
> >>>>>>>>>>> {
> >>>>>>>>>>> tree t = make_node (COMPLEX_CST);
> >>>>>>>>>>>
> >>>>>>>>>>> + real = fold (real);
> >>>>>>>>>>> + imag = fold (imag);
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> I still think this is wrong. The arguments should be sufficiently
> >>>>>>>>>> folded.
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> As we don't fold unary-operators on constants, we need to fold it
> >>>>>>>>> at
> >>>>>>>>> some place. AFAICS is the C++ FE not calling directly
> >>>>>>>>> build_complex.
> >>>>>>>>> So this place was the easiest way to avoid issues with things like
> >>>>>>>>> '-'
> >>>>>>>>> '1' etc.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Is this because of the
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> value = build_complex (NULL_TREE, convert (const_type,
> >>>>>>>>>
> >>>>>>>>> integer_zero_node),
> >>>>>>>>> value);
> >>>>>>
> >>>>>>
> >>>>>> Might be. This should be indeed a 'fold_convert', isn't it?
> >>>>
> >>>>
> >>>> Yes.
> >>>
> >>>
> >>> Applied modification to it.
> >>
> >>
> >> So can we remove the fold in build_complex now?
>
Yes. Done.
>
> >>>>>>>> in interpret_float? I think "convert" definitely needs to do some
> >>>>>>>> folding, since it's called from middle-end code that expects that.
> >>>>>>>
> >>>>>>>
> >>>>>>> I remember talking about "convert" doing some folding (and cp_convert
> >>>>>>> not) in our 1:1 last week.
> >>>>>>
> >>>>>>
> >>>>>> Can't remember that. I know that we were talking about the difference
> >>>>>> of convert and fold_convert. convert can be used on C++ specifics,
> >>>>>> but fold_convert is something shared with ME.
> >>>>
> >>>>
> >>>> convert is called from the ME, which sometimes expects folding.
> >>>>
> >>>>>> So first 'fold_convert'
> >>>>>> isn't the same as 'fold (convert ())'.
> >>>>>> I don't find places we invoke convert () in ME. We have some calls in
> >>>>>> convert.c (see convert_to_integer, convert_to_integer_nofold, and
> >>>>>> convert_to_real), which all used in AST only AFAICS.
> >>>>
> >>>>
> >>>> I was thinking of convert.c and fold-const.c to be part of the ME, since
> >>>> they are language-independent. But I guess other people think of the ME
> >>>> starting with gimple.
> >>>>
> >>>> And it looks like the only language-independent uses of convert are in
> >>>> c-family; I guess many of them should change to fold_convert.
> >>>
> >>>
> >>> Hmm, in context of this work? Or is this more a general point about
> >>> future
> >>> work?
> >>
> >>
> >> In the context of this work, if they are introducing problematic NOPs.
> >
> > Ok, I will take a closer look to convert () usage in c-family/. By
> > quick looking this seems to be the only place for now we needed to
> > change.
> >
> >>>>>>>>>>> @@ -5080,6 +5081,7 @@ output_constructor_bitfield (oc_local_state
> >>>>>>>>>>> *local,
> >>>>>>>>>>> unsigned int bit_offset)
> >>>>>>>>>>> while (TREE_CODE (local->val) == VIEW_CONVERT_EXPR
> >>>>>>>>>>> || TREE_CODE (local->val) == NON_LVALUE_EXPR)
> >>>>>>>>>>> local->val = TREE_OPERAND (local->val, 0);
> >>>>>>>>>>> + local->val = fold (local->val);
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> Likewise.
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> As soon as we can be sure that values getting fully_folded, or at
> >>>>>>>>> least folded for constants, we should be able to remove this.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Yep, they need to be folded before we get here.
> >>>
> >>>
> >>> I didn't come to remove this line for testing. As we fold now for
> >>> initializers more early, and cp_fold supports constructors, it could
> >>> be that we don't need this anymore. It is on my pile.
> >>
> >>
> >>> That fold is still required. By removing it, I saw boostrap issue due
> >>> 'invalid initializer'.
> >>
> >>
> >> That indicates a folding problem earlier on, that will cause some
> >> initialization that should be performed at compile time to happen at run
> >> time instead.
> >>
> >> Please investigate the bootstrap issue further.
> >
> > Yes, I do. I assume it is related to 'store_init_value'. For cases
> > decl_maybe_constant_var_p() is true, or decl is a static, we are
> > calling maybe_constant_init on the value, but for other cases we don't
> > simplify value. (Btw this might be related to the
> > STRIP_NOPS-requirement in 'reduced_constant_expression_p'). So by
> > adding the following hunk it seems to work (still need to verify)
> >
> > Index: typeck2.c
> > ===================================================================
> > --- typeck2.c (Revision 226401)
> > +++ typeck2.c (Arbeitskopie)
> > @@ -833,6 +833,8 @@ store_init_value (tree decl, tree init, vec<tree,
> > DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl) = const_init;
> > TREE_CONSTANT (decl) = const_init && decl_maybe_constant_var_p
> > (decl);
> > }
> > + else
> > + value = fold_simple (value);
> >
> > if (cxx_dialect >= cxx14 && CLASS_TYPE_P (strip_array_types (type)))
> > /* Handle aggregate NSDMI in non-constant initializers, too. */
>
> I guess we want to extend the code for handling statics and constants to
> also handle the case where the initializer is a CONSTRUCTOR. And also
> fold individual elements in split_nonconstant_init.
Yes, I added a general full folding to store_init_value, and to
split_nonconstant_init a folding to CONSTRUCTOR handling for RECORD_TYPES, etc.
By this I was able to remove the fold from varasm.c's output-function, too.
The invocation in split_nonconstant_init might not be necessary, but there are
other callers then store_init_value. So for them folding in those cases might
be still benifitial.
The "STRIP_NOPS-requirement in 'reduced_constant_expression_p'" I could remove,
but for one case in constexpr. Without folding we don't do
type-sinking/raising. So binary/unary operations might be containing cast,
which were in the past unexpected. On verify_constant we check by
reduced_constant_expression_p, if value is a constant. We don't handle here,
that NOP_EXPRs are something we want to look through here, as it doesn't change
anything if this is a constant, or not.
So I suggest following patch, so we are able to remove the STRIP_NOPS from
reduced_constant_expression_p.
--- constexpr.c (revision 226452)
+++ constexpr.c (working copy)
@@ -1441,8 +1441,6 @@ cxx_eval_call_expression (const constexpr_ctx *ctx
bool
reduced_constant_expression_p (tree t)
{
- /* Make sure we remove useless initial NOP_EXPRs. */
- STRIP_NOPS (t);
switch (TREE_CODE (t))
{
case PTRMEM_CST:
@@ -1476,7 +1474,10 @@ static bool
verify_constant (tree t, bool allow_non_constant, bool *non_constant_p,
bool *overflow_p)
{
- if (!*non_constant_p && !reduced_constant_expression_p (t))
+ tree rde = t;
+
+ STRIP_NOPS (rde);
+ if (!*non_constant_p && !reduced_constant_expression_p (rde))
{
if (!allow_non_constant)
error ("%q+E is not a constant expression", t);
Regards,
Kai
> Jason
>
>
