Re: C++ delayed folding branch review

[Kai Tietz <ktietz70 at googlemail dot com>]

2015-04-24 6:22 GMT+02:00 Jason Merrill <jason@redhat.com>:
>> +  expr = fold (expr);
>>    /* This may happen, because for LHS op= RHS we preevaluate
>>       RHS and create C_MAYBE_CONST_EXPR <SAVE_EXPR <RHS>>, which
>>       means we could no longer see the code of the EXPR.  */
>>    if (TREE_CODE (expr) == C_MAYBE_CONST_EXPR)
>>      expr = C_MAYBE_CONST_EXPR_EXPR (expr);
>>    if (TREE_CODE (expr) == SAVE_EXPR)
>> -    expr = TREE_OPERAND (expr, 0);
>> +    expr = fold (TREE_OPERAND (expr, 0));
>
>
> How about moving the first fold after the SAVE_EXPR block, so that we only
> need to call fold once?

I will try.  It might be required to fold in front to strip of useless
type-conversions ...

>> +    case NEGATE_EXPR:
>> +    case BIT_NOT_EXPR:
>> +    case CONVERT_EXPR:
>> +    case VIEW_CONVERT_EXPR:
>> +    case NOP_EXPR:
>> +    case FIX_TRUNC_EXPR:
>> +      {
>> +       tree op1 = TREE_OPERAND (expr, 0);
>> +       tree fop1 = fold (op1);
>> +       if (fop1 && op1 != fop1)
>> +         fop1 = fold_build1_loc (loc, TREE_CODE (expr), TREE_TYPE (expr),
>> +                                 fop1);

well, AFAIR was the idea here to make sure we do constant-value folding ...

>
> Isn't this redundant with the call to fold above?  If not, it seems that the
> above call should be to *_fully_fold.  I suppose we want an entry point
> defined by both front ends that c-common code can call which does full
> folding of an expression.

Sure, we can use here instead *_fully_fold, but for what costs?  In
general we need to deal here a simple one-level fold for simplifying
constant-values, and/or removing useless type-conversions.  As convert
doesn't fold them away anymore, it can stay with such NOP_EXPR before
constant-values, which cause us to fail on later value-analyzis.  So
sure, we can use *_fully_fold, but actually we don't need a full-fold.
(this applies to other places below too, where you mentioned the same
issue, too).

>> @@ -597,9 +597,9 @@ null_member_pointer_value_p (tree t)
>>      return false;
>>    else if (TYPE_PTRMEMFUNC_P (type))
>>      return (TREE_CODE (t) == CONSTRUCTOR
>> -           && integer_zerop (CONSTRUCTOR_ELT (t, 0)->value));
>> +           && integer_zerop (fold (CONSTRUCTOR_ELT (t, 0)->value)));
>>    else if (TYPE_PTRDATAMEM_P (type))
>> -    return integer_all_onesp (t);
>> +    return integer_all_onesp (fold (t));
>
>
> Calling fold here is wrong; it doesn't handle constexpr, and we should have
> folded before we got here.

s.o. we need to make sure constant-values get rid of useless
types-conversions/negates/etc ...

>>                 warn_logical_operator (loc, code, boolean_type_node,
>> -                                      code_orig_arg1, arg1,
>> -                                      code_orig_arg2, arg2);
>> +                                      code_orig_arg1, fold (arg1),
>> +                                      code_orig_arg2, fold (arg2));
>
>
> I think warn_logical_operator should call back into *_fully_fold. Likewise
> for most similar added calls to fold.

Same as above.  It isn't necessary for further analysis to perform
fully_fold on arg1/arg2.  But of course we can introduce this load
here.

>> @@ -7356,8 +7354,13 @@ build_over_call (struct z_candidate *cand, int
>> flags, tsu
>> bst_flags_t complain)
>>
>>    gcc_assert (j <= nargs);
>>    nargs = j;
>> +  {
>> +    tree *fargs = (!nargs ? argarray : (tree *) alloca (nargs * sizeof
>> (tree)))
>> ;
>> +    for (j = 0; j < nargs; j++)
>> +      fargs[j] = fold_non_dependent_expr (argarray[j]);
>
>
> Similarly, this and build_cxx_call should use cp_fully_fold.

Well, cp_fully_fold wouldn't resolve constexpr ... at least not in completeness.

>> @@ -7602,7 +7614,6 @@ build_cxx_call (tree fn, int nargs, tree *argarray,
>>        && current_function_decl
>>        && DECL_DECLARED_CONSTEXPR_P (current_function_decl))
>>      optimize = 1;
>> -  fn = fold_if_not_in_template (fn);
>>    optimize = optimize_sav;
>
>
> Since we're removing the fold, we can also remove the changes to "optimize".

Yeah, I missed that.  I removed superfluous code-path and committed it
on branch.

>> @@ -443,7 +443,7 @@ build_base_path (enum tree_code code,
>>
>>           t = TREE_TYPE (TYPE_VFIELD (current_class_type));
>>           t = build_pointer_type (t);
>> -         v_offset = convert (t, current_vtt_parm);
>> +         v_offset = fold (convert (t, current_vtt_parm));
>
> fold_convert should work here.

Well, fold_const isn't necessarily the same as fold (convert ())
within C++, due convert handles special cases fold_const doesn't.  At
least I ran here into issues for templates within templates AFAIR.


>> @@ -576,7 +576,6 @@ build_simple_base_path (tree expr, tree binfo)
>>         expr = build3 (COMPONENT_REF,
>>                        cp_build_qualified_type (type, type_quals),
>>                        expr, field, NULL_TREE);
>> -       expr = fold_if_not_in_template (expr);
>
>
> I don't think we need to remove this fold, since it is part of compiler
> internals rather than something the user wrote.  Really, we should represent
> the base conversion with something like a CONVERT_EXPR and only call this
> function when we want to fold it.  But that can wait for a later patch.

Ok.  I remove this fold-case due simply removing
fold_if_not_in_template function.  So well, we could re-add a call for
fold, if not in template.

>> @@ -1046,6 +1048,9 @@ adjust_temp_type (tree type, tree temp)
>>  {
>>    if (TREE_TYPE (temp) == type)
>>      return temp;
>> +  STRIP_NOPS (temp);
>> +  if (TREE_TYPE (temp) == type)
>> +    return temp;
>
> ...

s.a. for why there might be useless NOPS

>>
>>  reduced_constant_expression_p (tree t)
>>  {
>> +  /* Make sure we remove useless initial NOP_EXPRs.  */
>> +  STRIP_NOPS (t);
>
>
>
> Where are these NOPs coming from?

See above ...

>> @@ -1082,7 +1087,10 @@ cxx_bind_parameters_in_call (const constexpr_ctx
>> *ctx, tree t,
>>           && is_dummy_object (x))
>>         {
>>           x = ctx->object;
>> -         x = cp_build_addr_expr (x, tf_warning_or_error);
>> +         if (x)
>> +           x = cp_build_addr_expr (x, tf_warning_or_error);
>> +         else
>> +           x = get_nth_callarg (t, i);
>
>
> This should not be necessary.

Yeah, could be.  It might be something remaining to work-a-round an
older issue.  I will retest to see if we can remove this.

>> @@ -1765,7 +1780,8 @@ cxx_eval_component_reference (const constexpr_ctx
>> *ctx, tree t,
>>        if (field == part)
>>         {
>>           if (value)
>> -           return value;
>> +           return cxx_eval_constant_expression (ctx, value, lval,
>> +                                                non_constant_p,
>> overflow_p);
>
> ...
>>
>> @@ -1849,7 +1865,8 @@ cxx_eval_bit_field_ref (const constexpr_ctx *ctx,
>> tree t,
>>      {
>>        tree bitpos = bit_position (field);
>>        if (bitpos == start && DECL_SIZE (field) == TREE_OPERAND (t, 1))
>> -       return value;
>> +       return cxx_eval_constant_expression (ctx, value, lval,
>> +                                             non_constant_p, overflow_p);
>
>
> This shouldn't be necessary, either; the elements of the CONSTRUCTOR should
> be fully evaluated already.

Well, I will recheck.  I think it had something to do with the
possible none-optimized constant-values, which are requiring here a
folding.

>> @@ -1560,14 +1570,19 @@ cxx_eval_unary_expression (const constexpr_ctx
>> *ctx, tre
>> e t,
>>    location_t loc = EXPR_LOCATION (t);
>>    enum tree_code code = TREE_CODE (t);
>>    tree type = TREE_TYPE (t);
>> -  r = fold_unary_loc (loc, code, type, arg);
>> -  if (r == NULL_TREE)
>> +  if (TREE_CODE (t) == UNARY_PLUS_EXPR)
>> +    r = fold_convert_loc (loc, TREE_TYPE (t), arg);
>
>
> We don't want to handle UNARY_PLUS_EXPR here; we should handle it like
> NOP_EXPR.  And so you shouldn't need the call to unify_constant.

Hmm, ok.  I will replace that.  I liked the unify_constant function
for making it easier to read code-flow ... but yes, it isn't really
something necessary.

>>      case BIT_NOT_EXPR:
>>      case TRUTH_NOT_EXPR:
>>      case FIXED_CONVERT_EXPR:
>> +    case UNARY_PLUS_EXPR:
>>        r = cxx_eval_unary_expression (ctx, t, lval,
>
>
> So this case should be down with NOP_EXPR.

Ok.

>> @@ -2954,19 +2987,15 @@ cxx_eval_constant_expression (const constexpr_ctx
>> *ctx,
>> tree t,
>>    constexpr_ctx new_ctx;
>>    tree r = t;
>>
>> -  if (t == error_mark_node)
>> +  if (!t || t == error_mark_node)
>
>
> Where are null expressions coming from?

I will retest.  AFAIR this might be a work-a-round for
return-expressions.  It got fixed on stage 3 for 4.9, so this change
might be indeed superfluous now.

>>      case SIZEOF_EXPR:
>> +      if (processing_template_decl
>> +         && (!COMPLETE_TYPE_P (TREE_TYPE (t))
>> +         || TREE_CODE (TYPE_SIZE (TREE_TYPE (t))) != INTEGER_CST))
>> +       return t;
>
>
> The type of a SIZEOF_EXPR will always be size_t, so this isn't actually
> accomplishing anything, and should be removed.

Well, true, but I remember that I had seen issues in templates within
template cases where we might get incomlete types.  I will recheck.

>> +      /* See this can happen for case like g++.dg/init/static2.C
>> testcase.  */
>> +      if (!ctx || !ctx->ctor || (lval && !ctx->object)
>> +         || !same_type_ignoring_top_level_qualifiers_p
>> +              (TREE_TYPE (t), TREE_TYPE (ctx->ctor))
>> +         || CONSTRUCTOR_NELTS (ctx->ctor) != 0)
>> +       {
>> +         *non_constant_p = true;
>> +         break;
>> +       }
>
>
> Why can this happen on the branch but not on trunk?  I think the problem is
> elsewhere.
>
>>      case NOP_EXPR:
>>        {
>>         tree oldop = TREE_OPERAND (t, 0);
>> +       if (TREE_CODE (t) == NOP_EXPR && TREE_TYPE (t) == TREE_TYPE
>> (oldop) && TREE_OVERFLOW_P (oldop))
>> +         {
>> +           if (!ctx->quiet)
>> +             permerror (input_location, "overflow in constant
>> expression");
>> +           /* If we're being permissive (and are in an enforcing
>> +               context), ignore the overflow.  */
>> +           if (!flag_permissive)
>> +             *overflow_p = true;
>> +           *non_constant_p = true;
>> +
>> +           return t;
>> +         }
>
>
> This doesn't seem like the right place to handle this; why didn't we
> diagnose the overflow when it happened?

This is recent by convert-change for not-folding.  So a nop-expression
might be present, but isn't necessarily a overflow.  We need to handle
this special now.  Prior we simply added a nop before an
overflow-value to prevent further warnings.

>>  maybe_constant_init (tree t, tree decl)
>>  {
>> +  if (!t)
>> +    return t;
>
>
> Where are null initializers coming from?
>
>>      case MINUS_EXPR:
>>        /* -- a subtraction where both operands are pointers.   */
>>        if (TYPE_PTR_P (TREE_OPERAND (t, 0))
>> -          && TYPE_PTR_P (TREE_OPERAND (t, 1)))
>> +          && TYPE_PTR_P (TREE_OPERAND (t, 1))
>> +         && TREE_OPERAND (t, 0) != TREE_OPERAND (t, 1))
>
>
> Why?  From where are we getting a pointer subtracted from itself?
>
> That said, we should probably just remove this case and the next, as they
> are obsolete.  I'll remove them on the trunk.

Ok.

>> +static tree
>> +cp_fold (tree x, hash_map<tree, tree> *fold_hash)
>> +{
>
> ....
>
> I still think we need a hybrid of this and the constexpr code: it isn't full
> folding if we aren't doing constexpr evaluation.  But we can't just use
> maybe_constant_value because that only folds C++ constant-expressions, and
> we want to fold more things than that.  I suppose one simple approach for
> now would be to call maybe_constant_value from cp_fold.

Well, the functionality of cp_fold and maybe_constant_value (well,
actually how constexpr.c works) are different in cases of
none-constant results.
if we call cp_fold, we really want simplified expression, regardless
if it's result is a constant-value, or not.  Additionally handles
cp_fold parts of the optimization constexpr.c won't do (eg
pointer-arithmetics, more complex statement replacements, etc).
So I don't think it is a really sane thing to merge those two
different functionalities here.  But we can discuss this today in our
meeting further.

>> @@ -614,9 +614,13 @@ cp_fold_convert (tree type, tree expr)
>>      }
>>    else
>>      {
>> -      conv = fold_convert (type, expr);
>> +      if (TREE_CODE (expr) == INTEGER_CST)
>> +        conv = fold_convert (type, expr);
>> +      else
>> +        conv = convert (type, expr);
>
>
> Why?  If we're calling cp_fold_convert in a place where we don't want to
> fold, we should stop calling it rather than change it.
See, that we want to take care that constant-value is found here.
Otherwise we don't want anything folded.   Well, we could introduce
for this a special routine to abstract intention here.

>>  cp_convert_and_check (tree type, tree expr, tsubst_flags_t complain)
>>  {
>> -  tree result;
>> +  tree result, ret;
>>
>>    if (TREE_TYPE (expr) == type)
>>      return expr;
>>
>> -  result = cp_convert (type, expr, complain);
>> +  result = ret = cp_convert (type, expr, complain);
>>
>>    if ((complain & tf_warning)
>>        && c_inhibit_evaluation_warnings == 0)
>> @@ -652,6 +656,7 @@ cp_convert_and_check (tree type, tree expr,
>> tsubst_flags_t complain)
>>        tree stripped = folded;
>>        tree folded_result
>>         = folded != expr ? cp_convert (type, folded, complain) : result;
>> +      folded_result = fold (folded_result);
>>
>>        /* maybe_constant_value wraps an INTEGER_CST with TREE_OVERFLOW in
>> a
>>          NOP_EXPR so that it isn't TREE_CONSTANT anymore.  */
>> @@ -663,7 +668,7 @@ cp_convert_and_check (tree type, tree expr,
>> tsubst_flags_t complain)
>>                                         folded_result);
>>      }
>>
>> -  return result;
>> +  return ret;
>
>
> Why introduce the "ret" variable?  It doesn't seem to do anything different
> from "result".  And instead of the added fold, maybe change the cp_convert
> on the previous line to cp_fold_convert?

Good point, I will look close to it today.

>> @@ -1535,8 +1538,10 @@ build_expr_type_conversion (int desires, tree expr,
>> bool complain)
>> +  tree expr_folded = maybe_constant_value (expr);
>>
>> -  if (expr == null_node
>> +  STRIP_NOPS (expr_folded);
>> +  if (expr_folded == null_node
>
>
> We shouldn't need to fold to check for null_node, it only occurs when
> explicitly written.  And we don't want to fold before calling
> null_ptr_cst_p, since in C++11 only a literal 0 is a null pointer constant.
> For C++98 we already fold in null_ptr_cst_p.
>
>> @@ -8502,16 +8502,18 @@ compute_array_index_type (tree name, tree size,
>> tsubst_flags_t complain)
>> +  /* We need to do fully folding to determine if we have VLA, or not.  */
>> +  tree size_constant = maybe_constant_value (size);
>
>
> Why is this needed?  We already call maybe_constant_value earlier in
> compute_array_index_type.

Well, see above.  We might have constant-value not simplified.  So we
need a way to make sure we simplify in such case, but if it is
none-constant, we don't want an modified expression.  So
maybe_constant_value does this ...

>> -      itype = fold (itype);
>> +      itype = maybe_constant_value (itype);
>> -               itype = variable_size (fold (newitype));
>> +               itype = variable_size (maybe_constant_value (newitype));
>
> Maybe these should use cp_fully_fold?

We could use fully_fold, but we would also modify none-constant
expressions by this.  Do we actually want that here?  I will revisit
this and see if we can assume that

>> @@ -13090,6 +13092,8 @@ build_enumerator (tree name, tree value, tree
>> enumtype, location_t loc)
>> +  if (value)
>> +    value = maybe_constant_value (value);
>
>
> This seems unnecessary, since we call cxx_constant_value below.

See the other places

>>               value = cxx_constant_value (value);
>> +             STRIP_NOPS (value);
>
>
> The only time a constant result should have a NOP_EXPR around it is if it
> isn't really constant.  Why do you want to strip that?


See above ....

>> -          value = convert (ENUM_UNDERLYING_TYPE (enumtype), value);
>> +          value = fold (convert (ENUM_UNDERLYING_TYPE (enumtype),
>> value));
>
>
> fold_convert again.

Like the other case above.  fold_convert isn't necessarily for C++ the
same as 'fold (convert ())'.  For cast from classes it caused issues.
I will revisit if this is still true, but I would assume so.

>> @@ -188,9 +188,9 @@ build_zero_init_1 (tree type, tree nelts, bool
>> static_storage_p,
>> -    init = convert (type, nullptr_node);
>> +    init = fold (convert (type, nullptr_node));
>
>
> fold_convert
>
>> @@ -783,7 +783,8 @@ perform_member_init (tree member, tree init)
>> +      if (init)
>> +       init = fold (init);
>
>
> Why fold here?  This doesn't seem like a place that needs early folding.
>
>> @@ -6480,7 +6480,8 @@ cp_parser_array_notation (location_t loc, cp_parser
>> *parser, tree *init_index,
>> -      *init_index = cp_parser_expression (parser);
>> +      *init_index = cp_parser_expression (parser);
>> +      *init_index = maybe_constant_value (*init_index);
>
> ...
>>
>> +      length_index = maybe_constant_value (length_index);
>
> ...
>>
>> +         stride = maybe_constant_value (stride);
>
>
> Why fold here, rather than later when something really wants a constant?  If
> that ever actually occurs?
>
>> +  /* For offsetof and declaration of types we need
>> +     constant integeral values.
>> +     Also we meed to fold for negative constants so that diagnostic in
>> +     c-family/c-common.c doesn't fail for array-bounds.  */
>> +  if (for_offsetof || decltype_p
>> +      || (TREE_CODE (index) == NEGATE_EXPR && TREE_CODE (TREE_OPERAND
>> (index, 0)) == INTEGER_CST))
>> +    index = maybe_constant_value (index);
>
>
> Likewise.  For offsetof we should either use OFFSETOF_EXPR until late
> folding, or fold in offsetof evaluation.  I don't know why decltype would
> need anything special.  And for diagnostics we should be folding closer to
> the diagnostic.
>
>> @@ -9876,6 +9888,7 @@ cp_parser_label_for_labeled_statement (cp_parser*
>> parser, tree attributes)
>> +       expr = maybe_constant_value (expr);
>
>
> This seems redundant with the call to cxx_constant_value in case_conversion.
>
>> @@ -12190,6 +12204,10 @@ cp_parser_static_assert(cp_parser *parser, bool
>> member_p)
>> +  /* Make sure we folded it completely before doing trying to get
>> +     constant value.  */
>> +  condition = fold_non_dependent_expr (condition);
>
>
> This shouldn't be necessary; if the constexpr code needs to do more folding,
> that should be fixed.
>
>> @@ -16081,6 +16099,7 @@ cp_parser_enumerator_definition (cp_parser*
>> parser, tree type)
>> +      value = maybe_constant_value (value);
>
>
> This seems redundant with the call to cxx_constant_value in
> build_enumerator.
>
>> +             width = maybe_constant_value (width);
>
>
> This seems redundant with the call to cxx_constant_value in
> check_bitfield_decl.
>
> And so on.  It seems like you added maybe_constant_value after every
> occurrence of cp_parser_constant_expression, and I suspect that few are
> actually needed, and the ones that are should go closer to the code that
> really needs a constant.  I'd prefer to avoid calling it at all in parser.c.
>
>> @@ -449,7 +449,7 @@ build_aggr_init_expr (tree type, tree init)
>> -    return convert (type, init);
>> +    return fold (convert (type, init));
>
>
> fold_convert

See prior comments for this ...

>> @@ -3394,6 +3394,8 @@ handle_init_priority_attribute (tree* node,
>> +  if (initp_expr)
>> +    initp_expr = maybe_constant_value (initp_expr);
>
>
> Let's use cxx_constant_value instead of this and the non-constant diagnostic
> just below.

Ok.

>> @@ -3371,7 +3367,7 @@ get_member_function_from_ptrfunc (tree
>> *instance_ptrptr, tree function,
>> -      e2 = fold_convert (TREE_TYPE (e3), e2);
>> +      e2 = fold (convert (TREE_TYPE (e3), e2));
>
>
> Why?

Hmm, I will recheck, but indeed I assume I missed to make it again a
fold_convert out of it again.

>> @@ -3667,6 +3663,10 @@ convert_arguments (tree typelist, vec<tree, va_gc>
>> **valu
>> +      /* For BUILT_IN_NORMAL we want to fold constants.  */
>> +      if (fndecl && DECL_BUILT_IN (fndecl)
>> +         && DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)
>> +       val = fold (val);
>
>
> This should be cp_fully_fold, and lower down, after all the conversions.

Maybe, it is here more a question of load.  It is again more a thing
to make sure we get rid of useless-typeconversions, and doing basic
constant-value folding (eg for '-' 1 -> -1 etc).

>> -         tree xop0 = op0, xop1 = op1, xresult_type = result_type;
>> +         tree xop0 = fold (op0), xop1 = fold (op1), xresult_type =
>> result_type;
>
>
> This seems wrong.  In fact, the whole short_compare business seems like the
> sort of early folding we want to do away with.
>
>> -  if (TREE_OVERFLOW_P (result)
>> +  op0 = fold_non_dependent_expr (op0);
>> +  op1 = fold_non_dependent_expr (op1);
>> +  STRIP_NOPS (op0);
>> +  STRIP_NOPS (op1);
>> +  result_ovl = fold_build2 (resultcode, build_type, op0, op1);
>> +  if (TREE_OVERFLOW_P (result_ovl)
>>        && !TREE_OVERFLOW_P (op0)
>>        && !TREE_OVERFLOW_P (op1))
>> -    overflow_warning (location, result);
>> +    overflow_warning (location, result_ovl);
>
>
> What if we don't try to fold for this warning early, and instead give the
> warning later when we're folding?  I suppose that might apply to lots of the
> warnings that we're currently folding early for.

Yes, I tried to mimic prior-behavior for diagnostics.  As otherwise we
might loose diagnostic.  So for now this is the best place IMO to deal
with that.  Of course in follow-up changes we can do as you suggested,
and improve behavior here

>> @@ -7983,7 +7978,6 @@ expand_ptrmemfunc_cst (tree cst, tree *delta, tree
>> *pfn)
>>        tree binfo = binfo_or_else (orig_class, fn_class);
>>        *delta = build2 (PLUS_EXPR, TREE_TYPE (*delta),
>>                        *delta, BINFO_OFFSET (binfo));
>> -      *delta = fold_if_not_in_template (*delta);
>
>
> I think all the calls to fold_if_not_in_template in expand_ptrmemfunc_cst
> should become regular folds.  Or rather, change the build2 to fold_build2.
> This is very much compiler internals, and we should only get here when
> folding anyway.

Ok, we should just deal with in-template case here.

>> -         gcc_assert (val1->v.val_unsigned == DWARF2_ADDR_SIZE);
>> +         gcc_assert (val1->v.val_unsigned
>> +                     == (unsigned HOST_WIDE_INT) DWARF2_ADDR_SIZE);
>
>
> We need to fix this warning so this change is unnecessary.

Well, those following changes got already acked by Jeff for 4.9.  I
didn't applied it there as I have my concerns for them too, but I
don't see a good chance to modify shared-parts with C here.  Sadly
does C still do folding on cast-operations

>> -      gcc_assert (TREE_OPERAND (t, 0) == decl);
>> +      gcc_assert (TREE_OPERAND (t, 0) == decl || TREE_OPERAND (t, 1) ==
>> decl);
>
>
> This change doesn't seem to have anything to do with delayed folding.

It gets raised by delayed folding, so it is required to run.  So as it
gets triggered by it, we need to deal it here (btw pattern seems
absolutely right - issue will be some canonical operations before done
by fold-machinery ...)

>>                       || (gimple_omp_for_kind (for_stmt)
>> -                         == GF_OMP_FOR_KIND_CILKFOR));
>> +                         == GF_OMP_FOR_KIND_CILKFOR)
>> +                     || (gimple_omp_for_kind (for_stmt)
>> +                         == GF_OMP_FOR_KIND_FOR));
>
>
> Nor this one.

Same thing ...

>> +++ b/gcc/testsuite/g++.dg/ext/offsetof1.C
>> +// { dg-options "-Wno-pointer-arith" }
>
>
> There isn't any user-written pointer arithmetic in this testcase, so any
> such warnings are bogus.
>
>> +++ b/gcc/testsuite/g++.dg/warn/Wconversion-pr34389.C
>> @@ -50,5 +50,5 @@ short  mask5(int x)
>>
>>  short  mask6(short x)
>>  {
>> -  return x & -1;
>> +  return x & -1; // { dg-warning "conversion" }
>
>
> This is also a false positive.
>
>> +++ b/gcc/testsuite/g++.dg/warn/skip-1.C
>> -// Check that we don't warn about code that will not be executed.
>> +// For delayed folding we will warn about code that will not be executed
>> too.
>
>
> This is not an improvement.
>
>> @@ -1791,6 +1791,9 @@ evaluate_stmt (gimple stmt)
>>        && (likelyvalue == CONSTANT || is_gimple_call (stmt)
>>           || (gimple_assign_single_p (stmt)
>>               && gimple_assign_rhs_code (stmt) == ADDR_EXPR))
>> +      && (likelyvalue == CONSTANT || is_gimple_call (stmt)
>> +         || (gimple_assign_single_p (stmt)
>> +             && gimple_assign_rhs_code (stmt) == ADDR_EXPR))
>
>
> Merge error?

Ah, this is indeed unrelated.  It is a early fix of Richi about
pointer-alignment.  I will revert it on branch

>> @@ -1956,6 +1956,8 @@ build_complex (tree type, tree real, tree imag)
>>  {
>>    tree t = make_node (COMPLEX_CST);
>>
>> +  real = fold (real);
>> +  imag = fold (imag);
>
>
> I don't think we want to introduce folding into language-independent code
> like here.

Well, issue is that we want to do folding here for '-' CST etc.  so
this place seemed to me like the best place to deal with.  We can
think about other solution here.  I agree that this place is a bit
awkward.

>> @@ -5062,6 +5063,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);
>
>
> Or here.

Another place dealing with those converts/final simplifications for
constants ... (btw another Jeff acked already IIRC).

> Jason


Kai