From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 68485 invoked by alias); 28 Jul 2015 20:10:18 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 68462 invoked by uid 89); 28 Jul 2015 20:10:14 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL,BAYES_00,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-wi0-f181.google.com Received: from mail-wi0-f181.google.com (HELO mail-wi0-f181.google.com) (209.85.212.181) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Tue, 28 Jul 2015 20:10:05 +0000 Received: by wicgb10 with SMTP id gb10so171933316wic.1 for ; Tue, 28 Jul 2015 13:10:02 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.180.90.83 with SMTP id bu19mr10200055wib.91.1438114202333; Tue, 28 Jul 2015 13:10:02 -0700 (PDT) Received: by 10.27.176.72 with HTTP; Tue, 28 Jul 2015 13:10:02 -0700 (PDT) In-Reply-To: References: <557A5214.7060106@redhat.com> <1424811417.1214725.1434125493982.JavaMail.zimbra@redhat.com> <557BAE5A.7030309@redhat.com> <55B661A1.6090308@redhat.com> Date: Tue, 28 Jul 2015 20:35:00 -0000 Message-ID: Subject: Re: C++ delayed folding branch review From: Kai Tietz To: Jason Merrill Cc: Kai Tietz , gcc-patches List Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2015-07/txt/msg02405.txt.bz2 2015-07-28 1:14 GMT+02:00 Kai Tietz : > 2015-07-27 18:51 GMT+02:00 Jason Merrill : >> I've trimmed this to the previously mentioned issues that still need to be >> addressed; I'll do another full review after these are dealt with. > > Thanks for doing this summary of missing parts of prior review. > >> On 06/13/2015 12:15 AM, Jason Merrill wrote: >>> >>> On 06/12/2015 12:11 PM, Kai Tietz wrote: >>>>>> >>>>>> @@ -1052,6 +1054,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; >>>>>> @@ -1430,6 +1438,8 @@ cxx_eval_call_expression (const constexpr_ctx >>>>>> *ctx, >>>>>> tree t, >>>>>> bool >>>>>> reduced_constant_expression_p (tree t) >>>>>> { >>>>>> + /* Make sure we remove useless initial NOP_EXPRs. */ >>>>>> + STRIP_NOPS (t); >>>>> >>>>> >>>>> Within the constexpr code we should be folding away NOPs as they are >>>>> generated, they shouldn't live this long. >>>> >>>> >>>> Well, we might see them on overflows ... >>> >>> >>> We shouldn't within the constexpr code. NOPs for expressions that are >>> non-constant due to overflow are added in >>> cxx_eval_outermost_constant_expr, so we shouldn't see them in the middle >>> of constexpr evaluation. >>> >>>>>> @@ -1088,7 +1093,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 still should not be necessary. >>>> >>>> >>>> Yeah, most likely. But I got initially here some issues, so I don't >>>> see that this code would worsen things. >>> >>> >>> If this code path is hit, that means something has broken my design, and >>> I don't want to just paper over that. Please revert this change. >>> >>>>>> case SIZEOF_EXPR: >>>>>> + if (processing_template_decl >>>>>> + && (!COMPLETE_TYPE_P (TREE_TYPE (t)) >>>>>> + || TREE_CODE (TYPE_SIZE (TREE_TYPE (t))) != INTEGER_CST)) >>>>>> + return t; >>>>> >>>>> >>>>> Why is this necessary? >>>> >>>> >>>> We don't want to resolve SIZEOF_EXPR within template-declarations for >>>> incomplete types, of if its size isn't fixed. Issue is that we >>>> otherwise get issues about expressions without existing type (as usual >>>> within template-declarations for some expressions). >>> >>> >>> Yes, but we shouldn't have gotten this far with a dependent sizeof; >>> maybe_constant_value just returns if >>> instantiation_dependent_expression_p is true. >>> >>>>>> @@ -3391,8 +3431,23 @@ cxx_eval_constant_expression (const >>>>>> constexpr_ctx >>>>>> *ctx, tree t, >>>>>> case CONVERT_EXPR: >>>>>> case VIEW_CONVERT_EXPR: >>>>>> case NOP_EXPR: >>>>>> + case UNARY_PLUS_EXPR: >>>>>> { >>>>>> + enum tree_code tcode = TREE_CODE (t); >>>>>> tree oldop = TREE_OPERAND (t, 0); >>>>>> + >>>>>> + if (tcode == 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; >>>>>> + } >>>>>> tree op = cxx_eval_constant_expression (ctx, oldop, >>>>> >>>>> >>>>> Why doesn't the call to cxx_eval_constant_expression at the bottom here >>>>> handle oldop having TREE_OVERFLOW set? >>>> >>>> >>>> I just handled the case that we see here a wrapping NOP_EXPR around an >>>> overflow. As this isn't handled by cxx_eval_constant_expression. >>> >>> >>> How does it need to be handled? A NOP_EXPR wrapped around an overflow >>> is there to indicated that the expression is non-constant, and it can't >>> be simplified any farther. >>> >>> Please give an example of what was going wrong. >>> >>>>>> @@ -565,6 +571,23 @@ cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p, >>>>>> gimple_seq *post_p) >>>>>> >>>>>> switch (code) >>>>>> { >>>>>> + case SIZEOF_EXPR: >>>>>> + if (SIZEOF_EXPR_TYPE_P (*expr_p)) >>>>>> + *expr_p = cxx_sizeof_or_alignof_type (TREE_TYPE (TREE_OPERAND >>>>>> (*expr_p, >>>>>> + >>>>>> 0)), >>>>>> + SIZEOF_EXPR, false); >>>>>> + else if (TYPE_P (TREE_OPERAND (*expr_p, 0))) >>>>>> + *expr_p = cxx_sizeof_or_alignof_type (TREE_OPERAND (*expr_p, >>>>>> 0), >>>>>> + SIZEOF_EXPR, false); >>>>>> + else >>>>>> + *expr_p = cxx_sizeof_or_alignof_expr (TREE_OPERAND (*expr_p, >>>>>> 0), >>>>>> + SIZEOF_EXPR, false); >>>>>> + if (*expr_p == error_mark_node) >>>>>> + *expr_p = size_one_node; >>>>>> + >>>>>> + *expr_p = maybe_constant_value (*expr_p); >>>>>> + ret = GS_OK; >>>>>> + break; >>>>> >>>>> >>>>> Why are these surviving until gimplification time? >>>> >>>> >>>> This might be still necessary. I will retest, when bootstrap works. >>>> As we now added SIZEOF_EXPR folding to cp_fold, and if we catch all >>>> expressions a sizeof can occure, this shouldn't be necessary anymore. >>>> AFAIR I saw here some issues about initialzation for global-variables, >>>> which weren't caught. >>> >>> >>> Hmm, I wonder why you would see issues with global initializers that >>> aren't seen on trunk? In any case, if the issue is with global >>> initializers, they should be handled sooner, not here. >>> >>>>>> @@ -608,9 +608,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); >>>>> >>>>> >>>>> I still think that cp_fold_convert should always call fold_convert, and >>>>> callers that we don't want to fold should call convert instead, or >>>>> another function that folds only conversion of constants. We had talked >>>>> about the name "fold_cst", but I think that name isn't very clear; would >>>>> it make sense to just have convert always fold conversions of constants? >>>> >>>> >>>> We could introduce that, but we still have the issues about some >>>> unary-operations on constants, too. So we could do for any conversion >>>> afterwards a call to cp_try_fold_to_constant, which should reflect >>>> that pretty well, beside within template-declarations ... >> >> >> Now we've been talking about calling it "fold_simple". > > Yes, rename happened. I will send a patch for that. By reading > cp_fold_convert, I agree that folding can be assumed .... Changed that on branch. Users of cp_fold_convert are looking to me just like artificial-code-generations, and IMO we can keep there usage of cp_fold_convert. >> >>>>>> @@ -1529,8 +1532,11 @@ build_expr_type_conversion (int desires, tree >>>>>> expr, >>>>>> bool complain) >>>>>> tree basetype = TREE_TYPE (expr); >>>>>> tree conv = NULL_TREE; >>>>>> tree winner = NULL_TREE; >>>>>> + /* Want to see if EXPR is a constant. See below checks for >>>>>> null_node. >>>>>> */ >>>>>> + tree expr_folded = cp_try_fold_to_constant (expr); >>>>>> >>>>>> - if (expr == null_node >>>>>> + STRIP_NOPS (expr_folded); >>>>>> + if (expr_folded == null_node >>>>> >>>>> >>>>> Again, we shouldn't need to fold to check for null_node, it only occurs >>>>> when explicitly written. Folding should never produce null_node unless >>>>> the argument was already null_node. >>>> >>>> >>>> Well, we need to do this for diagnostic messages AFAIR. We want to >>>> see if expression folded gets a constant, so that diagnostics getting >>>> displayed right. >>> >>> >>> Again, null_node is special. It indicates that the user typed "__null". >>> That's what we're checking for here. Folding is both unnecessary and >>> undesirable. >>> >>>>>> @@ -1548,7 +1554,7 @@ build_expr_type_conversion (int desires, tree >>>>>> expr, >>>>>> bool complain) >>>>>> switch (TREE_CODE (basetype)) >>>>>> { >>>>>> case INTEGER_TYPE: >>>>>> - if ((desires & WANT_NULL) && null_ptr_cst_p (expr)) >>>>>> + if ((desires & WANT_NULL) && null_ptr_cst_p (expr_folded)) >>>>> >>>>> >>>>> Again, 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. >>>> >>>> >>>> We need to avoid useless conversion, so we should reduce to simple >>>> constant-value ... >>> >>> >>> No. Again, in C++11 only "0" or "0L" is a null pointer constant. A >>> more complex expression that folds to 0 is NOT a null pointer constant. >>> Folding is actively harmful here. >>> >>> And again, in C++98 mode null_ptr_cst_p already folds, so doing it here >>> is redundant. >>> >>> Was I unclear? >>> >>>>>> @@ -8496,16 +8467,18 @@ compute_array_index_type (tree name, tree size, >>>>>> tsubst_flags_t complain) >>>>>> SET_TYPE_STRUCTURAL_EQUALITY (itype); >>>>>> return itype; >>>>>> } >>>>>> - >>>>>> + >>>>>> + /* We need to do fully folding to determine if we have VLA, or >>>>>> not. */ >>>>>> + tree size_constant = cp_try_fold_to_constant (size); >>>>> >>>>> >>>>> Again, we already called maybe_constant_value. >>>> >>>> >>>> Sure, but maybe_constant_value still produces nops ... >>> >>> >>> If someone tries to create an array with a size that involves arithmetic >>> overflow, that's undefined behavior and we should probably give an error >>> rather than fold it away. >>> >>>>>> @@ -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 >>>>>> @@ -13102,6 +13068,7 @@ build_enumerator (tree name, tree value, tree >>>>>> enumtype, tree attributes, >>>>>> if (value != NULL_TREE) >>>>>> { >>>>>> value = cxx_constant_value (value); >>>>>> + STRIP_NOPS (value); >>>>> >>>>> >>>>> Again, 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? >>>> >>>> >>>> As for an enumerator-value we might have overflows, which are silently >>>> ignored. >>> >>> >>> They shouldn't be ignored. C++ requires that the value be constant, and >>> overflow makes it non-constant. >>> >>>> I will recheck this what example we have for this when bootstrap is >>>> working again. >>>> >>>>>> @@ -6575,6 +6578,13 @@ cp_parser_postfix_open_square_expression >>>>>> (cp_parser >>>>>> *parser, >>>>>> index = cp_parser_expression (parser); >>>>>> } >>>>>> >>>>>> + /* 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 = cp_try_fold_to_constant (index); >>>>> >>>>> >>>>> Similarly, for offsetof the folding should happen closer to where it is >>>>> needed. >>>>> >>>>> Why is it needed for decltype, which is querying the type of an >>>>> expression? >>>>> >>>>> For NEGATE_EXPR, we had talked about always folding a NEGATE of a >>>>> constant; this isn't the right place to do it. >>>> >>>> >>>> Same as above, we need in those cases (and for -1 too) the constant >>>> values early anyway. So I saw it as more logical to have done this >>>> conversion as soon as possible after initialization. >>> >>> >>> I don't think this is as soon as possible; we can fold the NEGATE_EXPR >>> immediately when we build it, at the end of cp_build_unary_op. >>> >>> I still wonder why any folding is necessary for decltype. When I ask >>> why, I want to know *why*, not just have you tell me again that it's >>> needed. I don't think it is. >>> >>> For offsetof, I wonder if it makes sense to extend fold_offsetof_1 to >>> handle whatever additional folding is needed here. If not, then fold in >>> finish_offsetof, before calling fold_offsetof. >> >> >> I see that this is now an unconditional fold_simple, but I still don't >> understand why it needs to be folded here, in the parser. > > The point to fold the 'value' here is for cases > 'processing_template_decl' isn't false. We could move it to the > else-case of the 'if (! processing_template_decl)' line for being more > explicit? Well, on looking here in more detail, we might don't that that initial folding here. As for processing_template_decl fold_simple (and cp_fully_fold) doesn't do much. >>>.... >>>> >>>> Anyway, if you prefer, we can do this in builder-routines, and remove >>>> at places constants aren't needed directly after parsing it those calls. >>> >>> >>> I want to delay it to: >>> >>> 1) the places where we actually care about constant values, all of which >>> already call maybe_constant_value or cxx_constant_value, so they >>> shouldn't need much change; and >>> 2) the places where we want a simplified expression for warnings, where >>> we should call fold_simple. >> >> >>> Folding in the parser is wrong, most of all because template >>> substitution doesn't go through the parser. >> >> >> There are still several folds in cp_parser_omp_* that should move later. > > In 'cp_parser_omp_var_list_no_open' we need to fold 'length' can > 'low_bound' as those values getting checked some lines below (see > lines 27936, 27944). We could call here fold_simple instead? > If we would delay it to later, we would need to move diagnostics here too, ? > In 'cp_parser_omp_clause_aligned','cp_parser_omp_clause_safelen', and > in 'cp_parser_omp_clause_simdlen' I tried to fold early to prevent > early cases assuming that OMP-operands are already folded. > I tested this, and it doesn't seems to be necessary anymore due we > perform full converatge here in the cp_fold_r walker now. > I removed here the fold-invocation of them. > > In 'cp_parser_cilk_grainsize' we fold 2nd argument of > 'cp_paser_cild_for' by 'fold_simple'. Not sure if it is worth to move > operand-folding into cp_parser_cilk_for itself, as we have here just > two users of 'cp_parser_cilk_for'. > One time we pass 'integer_zero_node' as this argument, and the other > time a binary-expression, which might be constant value. > But sure we can move it into 'cp_parser_cilk_grainsize'.if you prefer? ? >>> finish_unary_op_expr (location_t loc, enum tree_code code, tree expr, >>> tsubst_flags_t complain) >>> { >>> + tree expr_ovl = expr; >>> tree result = build_x_unary_op (loc, code, expr, complain); >>> + tree result_ovl = result; >>> + >>> + expr_ovl = fold_simple (expr_ovl); >>> + STRIP_NOPS (expr_ovl); >> >> >> Why both fold_simple and STRIP_NOPS? > > If we have an overflow-value encapsulated into an nop_expr, we want to > see that overflow-expression itself. > fold_simple preserves the nop_expr conversion, so we want to remove > it, if present. > Not sure, if this really still can happen, I will do some testing on it. No, it doesn't happen anymore. I removed STRIP_NOP. >>>>>> @@ -441,7 +441,7 @@ build_aggr_init_expr (tree type, tree init) >>>>>> else if (TREE_CODE (init) == AGGR_INIT_EXPR) >>>>>> fn = AGGR_INIT_EXPR_FN (init); >>>>>> else >>>>>> - return convert (type, init); >>>>>> + return fold (convert (type, init)); >>>>> >>>>> >>>>> Why fold here? >>>> >>>> >>>> We had this already in prior thread. fold (convert ()) != >>>> fold_convert () for C++. The fold is just there to make sure we fold >>>> away useless casts. >>> >>> >>> But why here? Can't we fold away useless casts earlier (in convert) or >>> later (when we care about having a simplified expression)? >>> >>>>>> @@ -3664,6 +3660,10 @@ convert_arguments (tree typelist, vec>>>>> va_gc> >>>>>> **values, tree fndecl, >>>>>> && (type == 0 || TREE_CODE (type) != REFERENCE_TYPE)) >>>>>> val = TREE_OPERAND (val, 0); >>>>>> >>>>>> + /* 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); >>>>> >>>>> >>>>> Why? >>>> >>>> >>>> As builtin-handlers are expecting to see constant values. >> >> >> I would think this should be maybe_constant_value then. > > Why? At the end we resolve normal-builtin via 'fold_call_expr'. Of > course we can invoke here maybe_constant_value, but it would end up in > the same folding of a builtin-expression. So calling here directly > 'fold' just short-cuts this. ? > >>>>>> @@ -5026,18 +5023,21 @@ cp_build_binary_op (location_t location, >>>>>> } >>>>>> >>>>>> result = build2 (resultcode, build_type, op0, op1); >>>>>> - result = fold_if_not_in_template (result); >>>>>> if (final_type != 0) >>>>>> result = cp_convert (final_type, result, complain); >>>>>> - >>>>>> - 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); >>>>> >>>>> >>>>> Don't you want to use cp_fully_fold here? >>> >>> >>> ? >> >> >> Introducing *_non_dependent_expr is definitely wrong here. > > I don't remember anymore, why I used here *_non_dependent_expr. I > will see if we can use here instead cp_fully_fold. If we can get rid > of the STRIP_NOPs, I am not sure, as we are interested in > overflow-bit, and don't want to see it encapsulated into nop_expr ... I replaced those *_non_dependent_expr calls by fold_simple calls. The STRIP_NOPS isn't necessary then too. >> >>>>>> @@ -7249,7 +7249,7 @@ gimplify_omp_for (tree *expr_p, gimple_seq >>>>>> *pre_p) >>>>>> /* Handle OMP_FOR_COND. */ >>>>>> t = TREE_VEC_ELT (OMP_FOR_COND (for_stmt), i); >>>>>> gcc_assert (COMPARISON_CLASS_P (t)); >>>>>> - gcc_assert (TREE_OPERAND (t, 0) == decl); >>>>>> + gcc_assert (TREE_OPERAND (t, 0) == decl || TREE_OPERAND (t, >>>>>> 1) == >>>>>> decl); >>>>> >>>>> >>>>> Why didn't delayed folding canonicalize this so that the decl is in op0? >>>> >>>> >>>> Delay folding doesn't canonicalize this. >>> >>> >>> Why not? Doesn't it fold all expressions? >> >> >> ? > > It fold them lately. I will recheck this code-change. It might be no > longer required due recent changes to omp-folding. It could be that > original pattern didn't applied here anymore, and therefore statement > didn't been transformed into its canonical form. Bit I assume this > could be resolved. > >>>> Actually we don't want to touch here anything in parsered tree. We >>>> could do this in generalization-pass before gimplification. Seems to >>>> be something we don't catch for now, which makes me wonder a bit. >>>> >>>>>> @@ -508,7 +508,9 @@ extract_omp_for_data (gomp_for *for_stmt, struct >>>>>> omp_for_data *fd, >>>>>> gcc_assert (gimple_omp_for_kind (for_stmt) >>>>>> == GF_OMP_FOR_KIND_CILKSIMD >>>>>> || (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)); >>>>> >>>>> >>>>> This still seems like a red flag; how is delayed folding changing the >>>>> OMP for kind? >>>> >>>> >>>> It doesn't. The issue is that some canonical operations of fold >>>> aren't happening anymore on which omp depends. >>> >>> >>> That seems like a problem. > > See above. I will come up on this tomorrow. I think this is resolved already. It was related to folding of OMP conditions. I will do bootstrap with this humk removed. >>> @@ -867,7 +867,7 @@ expand_subword_shift (machine_mode op1_mode, optab >>> binoptab, >>> are truncated to the mode size. */ >>> carries = expand_binop (word_mode, reverse_unsigned_shift, >>> outof_input, const1_rtx, 0, unsignedp, >>> methods); >>> - if (shift_mask == BITS_PER_WORD - 1) >>> + if (shift_mask == (unsigned HOST_WIDE_INT) (BITS_PER_WORD - 1)) >> >> >> These should still be unnecessary. > > Yes, they are. We handle now the avoiding of dead-code for constants > (cond-expression, truthif* expressions, and useless convert-warnings). > So this change is something only interesting for different compiler, > but not related to delayed-folding anymore. > I will check, and remove it tomorrow. > >>>>>> @@ -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? > >>> 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. 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 remember that we were talking about adding a standard-folding to > convert for operations on constant-values (as we do for > convert_to_integer). Do you mean this? > >>>>>> @@ -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. >>> >>> It looks like your latest checkin added more redundant folding: >>> >>>> @@ -3311,6 +3311,9 @@ finish_case_label (location_t loc, tree >>>> low_value, tree hi >>>> gh_value) >>>> low_value = case_conversion (type, low_value); >>>> high_value = case_conversion (type, high_value); >>>> >>>> + low_value = cp_fully_fold (low_value); >>>> + high_value = cp_fully_fold (high_value); >>> >>> >>> Again, case_conversion should have already folded constants. >>> >>>> @@ -5776,6 +5776,8 @@ convert_nontype_argument (tree type, tree expr, >>>> tsubst_flags_t complain) >>>> { >>>> tree expr_type; >>>> >>>> + expr = cp_try_fold_to_constant (expr); >>>> + >>>> /* Detect immediately string literals as invalid non-type argument. >>>> This special-case is not needed for correctness (we would easily >>>> catch this later), but only to provide better diagnostic for this >>>> @@ -5852,6 +5854,7 @@ convert_nontype_argument (tree type, tree expr, >>>> tsubst_flags_t complain) >>>> else if (TYPE_PTR_OR_PTRMEM_P (type)) >>>> { >>>> tree folded = maybe_constant_value (expr); >>>> + folded = cp_try_fold_to_constant (expr); >>> >>> >>> And here, convert_nontype_argument already uses >>> maybe_constant_value/cxx_constant_value for folding constants. >> >> >> Jason >> > Kai