From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ot1-x330.google.com (mail-ot1-x330.google.com [IPv6:2607:f8b0:4864:20::330]) by sourceware.org (Postfix) with ESMTPS id DD205385DC15 for ; Tue, 20 Jul 2021 21:52:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org DD205385DC15 Received: by mail-ot1-x330.google.com with SMTP id a17-20020a9d3e110000b02904ce97efee36so233514otd.7 for ; Tue, 20 Jul 2021 14:52:51 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language; bh=cwOYjMXr33B6TFfC2KDoZl24GhY5VP8x+MeFTb1xaNU=; b=P6aobaQOvLvwIBnpp2mugN+YDkvAF0zHH1Qq5puOvLQs0XsNP6/0MINrx4dEDOIM66 zMw8e70SG3Siyyl/DJMf4bjEVa/Oerrnjh811s0VKYCCdVpCSJV9F5WynHyzsiNXQrcI ueL356FjaIB0wDiCtOeWpG25peQUS106UoYHc0nNcQpqXcbdE3mKUxwRjaNILwZY2zIq qC29YRvoIqddJ1nbeyXpcHsbPldU0ZD8/N9kdNyWJKe8K+tiYiMJUJMS21/3yP3Fv9kc k78TAf/dV23r3/EGBQvvxHz6JyCYISIDKz8iIf8cKkhcF8VTFThdySNZNjUNXIqzGVGD IKxA== X-Gm-Message-State: AOAM531xDysTy93R+ZUYOBe1Ck2Xa+BJ2ZZyqQ9biSaw7Qc+zo5ygwVv 6P/oQIuL7HQyLL3VUhtxyb1imtnAnuQ= X-Google-Smtp-Source: ABdhPJx799b0AogcGHqoGNfGHEStlam2tx7W7E97YbPQMR2kbmsYa7HS4AX8koEZe+UAOU7cgyvbsg== X-Received: by 2002:a05:6830:11c9:: with SMTP id v9mr23304596otq.15.1626817971026; Tue, 20 Jul 2021 14:52:51 -0700 (PDT) Received: from [192.168.0.41] (75-166-102-22.hlrn.qwest.net. [75.166.102.22]) by smtp.gmail.com with ESMTPSA id t144sm4644795oih.57.2021.07.20.14.52.50 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 20 Jul 2021 14:52:50 -0700 (PDT) Subject: Re: [PING][PATCH] define auto_vec copy ctor and assignment (PR 90904) To: Jason Merrill , Jonathan Wakely , Richard Biener Cc: gcc-patches References: <390c6652-0a1f-e8c4-d70d-56ced2f7b0fb@gmail.com> <0ee2488c-04a1-df3b-dd4f-92eec51a4ab2@redhat.com> <7ebb37f3-76c3-8e00-7852-c93bf142043a@gmail.com> <2c60a7d0-3f60-b72f-c0f2-6fc7a4900740@redhat.com> <072a4715-2248-d836-948d-50426160de47@gmail.com> <6ed67ff9-12d3-516a-f7ec-13dd913ed54b@gmail.com> <187b41ce-d00c-a204-7817-349f6c251a5f@redhat.com> From: Martin Sebor Message-ID: Date: Tue, 20 Jul 2021 15:52:49 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 MIME-Version: 1.0 In-Reply-To: <187b41ce-d00c-a204-7817-349f6c251a5f@redhat.com> Content-Type: multipart/mixed; boundary="------------26B7EA46A823CC455716F24F" Content-Language: en-US X-Spam-Status: No, score=-8.7 required=5.0 tests=BAYES_00, BODY_8BITS, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, KAM_SHORT, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 20 Jul 2021 21:52:55 -0000 This is a multi-part message in MIME format. --------------26B7EA46A823CC455716F24F Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit On 7/20/21 2:08 PM, Jason Merrill wrote: > On 7/20/21 2:34 PM, Martin Sebor wrote: >> On 7/14/21 10:23 AM, Jason Merrill wrote: >>> On 7/14/21 10:46 AM, Martin Sebor wrote: >>>> On 7/13/21 9:39 PM, Jason Merrill wrote: >>>>> On 7/13/21 4:02 PM, Martin Sebor wrote: >>>>>> On 7/13/21 12:37 PM, Jason Merrill wrote: >>>>>>> On 7/13/21 10:08 AM, Jonathan Wakely wrote: >>>>>>>> On Mon, 12 Jul 2021 at 12:02, Richard Biener wrote: >>>>>>>>> Somebody with more C++ knowledge than me needs to approve the >>>>>>>>> vec.h changes - I don't feel competent to assess all effects of >>>>>>>>> the change. >>>>>>>> >>>>>>>> They look OK to me except for: >>>>>>>> >>>>>>>> -extern vnull vNULL; >>>>>>>> +static constexpr vnull vNULL{ }; >>>>>>>> >>>>>>>> Making vNULL have static linkage can make it an ODR violation to >>>>>>>> use >>>>>>>> vNULL in templates and inline functions, because different >>>>>>>> instantiations will refer to a different "vNULL" in each >>>>>>>> translation >>>>>>>> unit. >>>>>>> >>>>>>> The ODR says this is OK because it's a literal constant with the >>>>>>> same value (6.2/12.2.1). >>>>>>> >>>>>>> But it would be better without the explicit 'static'; then in >>>>>>> C++17 it's implicitly inline instead of static. >>>>>> >>>>>> I'll remove the static. >>>>>> >>>>>>> >>>>>>> But then, do we really want to keep vNULL at all?  It's a weird >>>>>>> blurring of the object/pointer boundary that is also dependent on >>>>>>> vec being a thin wrapper around a pointer.  In almost all cases >>>>>>> it can be replaced with {}; one exception is == comparison, where >>>>>>> it seems to be testing that the embedded pointer is null, which >>>>>>> is a weird thing to want to test. >>>>>> >>>>>> The one use case I know of for vNULL where I can't think of >>>>>> an equally good substitute is in passing a vec as an argument by >>>>>> value.  The only way to do that that I can think of is to name >>>>>> the full vec type (i.e., the specialization) which is more typing >>>>>> and less generic than vNULL.  I don't use vNULL myself so I wouldn't >>>>>> miss this trick if it were to be removed but others might feel >>>>>> differently. >>>>> >>>>> In C++11, it can be replaced by {} in that context as well. >>>> >>>> Cool.  I thought I'd tried { } here but I guess not. >>>> >>>>> >>>>>> If not, I'm all for getting rid of vNULL but with over 350 uses >>>>>> of it left, unless there's some clever trick to make the removal >>>>>> (mostly) effortless and seamless, I'd much rather do it independently >>>>>> of this initial change. I also don't know if I can commit to making >>>>>> all this cleanup. >>>>> >>>>> I already have a patch to replace all but one use of vNULL, but >>>>> I'll hold off with it until after your patch. >>>> >>>> So what's the next step?  The patch only removes a few uses of vNULL >>>> but doesn't add any.  Is it good to go as is (without the static and >>>> with the additional const changes Richard suggested)?  This patch is >>>> attached to my reply to Richard: >>>> https://gcc.gnu.org/pipermail/gcc-patches/2021-July/575199.html >>> >>> As Richard wrote: >>> >>>> The pieces where you change vec<> passing to const vec<>& and the few >>>> where you change vec<> * to const vec<> * are OK - this should make the >>>> rest a smaller piece to review. >>> >>> Please go ahead and apply those changes and send a new patch with the >>> remainder of the changes. >> >> I have just pushed r12-2418: >> https://gcc.gnu.org/pipermail/gcc-cvs/2021-July/350886.html >> >>> >>> A few other comments: >>> >>>> -                       omp_declare_simd_clauses); >>>> +                       *omp_declare_simd_clauses); >>> >>> Instead of doing this indirection in all of the callers, let's change >>> c_finish_omp_declare_simd to take a pointer as well, and do the >>> indirection in initializing a reference variable at the top of the >>> function. >> >> Okay. >> >>> >>>> +    sched_init_luids (bbs.to_vec ()); >>>> +    haifa_init_h_i_d (bbs.to_vec ()); >>> >>> Why are these to_vec changes needed when you are also changing the >>> functions to take const&? >> >> Calling to_vec() here isn't necessary so I've removed it. >> >>> >>>> -  vec checks = LOOP_VINFO_CHECK_NONZERO (loop_vinfo); >>>> +  vec checks = LOOP_VINFO_CHECK_NONZERO (loop_vinfo).to_vec (); >>> >>> Why not use a reference here and in other similar spots? >> >> Sure, that works too. >> >> Attached is what's left of the original changes now that r12-2418 >> has been applied. > >> @@ -3364,7 +3364,8 @@ static void >>  vect_check_lower_bound (loop_vec_info loop_vinfo, tree expr, bool >> unsigned_p, >>              poly_uint64 min_value) >>  { >> -  vec lower_bounds = LOOP_VINFO_LOWER_BOUNDS >> (loop_vinfo); >> +  vec lower_bounds >> +    = LOOP_VINFO_LOWER_BOUNDS (loop_vinfo).to_vec (); >>    for (unsigned int i = 0; i < lower_bounds.length (); ++i) >>      if (operand_equal_p (lower_bounds[i].expr, expr, 0)) >>        { >> @@ -3466,7 +3467,7 @@ vect_prune_runtime_alias_test_list >> (loop_vec_info loop_vinfo) >>    typedef pair_hash >> tree_pair_hash; >>    hash_set compared_objects; >> >> -  vec may_alias_ddrs = LOOP_VINFO_MAY_ALIAS_DDRS (loop_vinfo); >> +  vec may_alias_ddrs = LOOP_VINFO_MAY_ALIAS_DDRS >> (loop_vinfo).to_vec (); > > These could also be references. Even const references it turns out for some of them. > That leaves this as the only remaining use of to_vec: > >>    ipa_call_arg_values (ipa_auto_call_arg_values *aavals) >> -    : m_known_vals (aavals->m_known_vals), >> -      m_known_contexts (aavals->m_known_contexts), >> -      m_known_aggs (aavals->m_known_aggs), >> -      m_known_value_ranges (aavals->m_known_value_ranges) >> +    : m_known_vals (aavals->m_known_vals.to_vec ()), >> +      m_known_contexts (aavals->m_known_contexts.to_vec ()), >> +      m_known_aggs (aavals->m_known_aggs.to_vec ()), >> +      m_known_value_ranges (aavals->m_known_value_ranges.to_vec ()) > > I think we could handle this by deriving ipa_auto_call_arg_values from > ipa_call_arg_values like auto_vec is derived from vec, but perhaps > dealing with the IPA datastructures could be saved for the next stage of > overhauling vec.  Maybe for now just change the name to_vec to something > clearer that new code shouldn't use it, e.g. to_vec_legacy. Done in the attached patch. Martin --------------26B7EA46A823CC455716F24F Content-Type: text/x-patch; charset=UTF-8; name="gcc-90904.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="gcc-90904.diff" PR middle-end/90904 - vec assignment and copying undefined gcc/ChangeLog: PR middle-end/90904 * vec.c (test_copy_assign): New function. (vec_c_tests): Call it. * vec.h (vec_assign): New function. (auto_vec copy ctor): Define. (auto_vec::operator=): Define. (auto_vec_no_copy): New class template. (auto_string_vec): Disable copying/assignment. diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c index 9a56e0c04c6..fa3c9fa3a1b 100644 --- a/gcc/c/c-parser.c +++ b/gcc/c/c-parser.c @@ -1489,7 +1489,8 @@ static tree c_parser_std_attribute_specifier_sequence (c_parser *); static void c_parser_external_declaration (c_parser *); static void c_parser_asm_definition (c_parser *); static void c_parser_declaration_or_fndef (c_parser *, bool, bool, bool, - bool, bool, tree *, vec, + bool, bool, tree * = NULL, + vec * = NULL, bool have_attrs = false, tree attrs = NULL, struct oacc_routine_data * = NULL, @@ -1774,13 +1775,12 @@ c_parser_external_declaration (c_parser *parser) an @interface or @protocol with prefix attributes). We can only tell which after parsing the declaration specifiers, if any, and the first declarator. */ - c_parser_declaration_or_fndef (parser, true, true, true, false, true, - NULL, vNULL); + c_parser_declaration_or_fndef (parser, true, true, true, false, true); break; } } -static void c_finish_omp_declare_simd (c_parser *, tree, tree, vec); +static void c_finish_omp_declare_simd (c_parser *, tree, tree, vec *); static void c_finish_oacc_routine (struct oacc_routine_data *, tree, bool); /* Build and add a DEBUG_BEGIN_STMT statement with location LOC. */ @@ -1890,11 +1890,15 @@ static void c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok, bool static_assert_ok, bool empty_ok, bool nested, bool start_attr_ok, - tree *objc_foreach_object_declaration, - vec omp_declare_simd_clauses, - bool have_attrs, tree attrs, - struct oacc_routine_data *oacc_routine_data, - bool *fallthru_attr_p) + tree *objc_foreach_object_declaration + /* = NULL */, + vec *omp_declare_simd_clauses + /* = NULL */, + bool have_attrs /* = false */, + tree attrs /* = NULL_TREE */, + struct oacc_routine_data *oacc_routine_data + /* = NULL */, + bool *fallthru_attr_p /* = NULL */) { struct c_declspecs *specs; tree prefix_attrs; @@ -2150,7 +2154,7 @@ c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok, C_DTR_NORMAL, &dummy); if (declarator == NULL) { - if (omp_declare_simd_clauses.exists ()) + if (omp_declare_simd_clauses) c_finish_omp_declare_simd (parser, NULL_TREE, NULL_TREE, omp_declare_simd_clauses); if (oacc_routine_data) @@ -2250,7 +2254,7 @@ c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok, chainon (postfix_attrs, all_prefix_attrs)); if (!d) d = error_mark_node; - if (omp_declare_simd_clauses.exists ()) + if (omp_declare_simd_clauses) c_finish_omp_declare_simd (parser, d, NULL_TREE, omp_declare_simd_clauses); } @@ -2262,7 +2266,7 @@ c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok, chainon (postfix_attrs, all_prefix_attrs)); if (!d) d = error_mark_node; - if (omp_declare_simd_clauses.exists ()) + if (omp_declare_simd_clauses) c_finish_omp_declare_simd (parser, d, NULL_TREE, omp_declare_simd_clauses); init_loc = c_parser_peek_token (parser)->location; @@ -2342,7 +2346,7 @@ c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok, warn_parm_array_mismatch (lastloc, d, parms); } } - if (omp_declare_simd_clauses.exists ()) + if (omp_declare_simd_clauses) { tree parms = NULL_TREE; if (d && TREE_CODE (d) == FUNCTION_DECL) @@ -2496,9 +2500,9 @@ c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok, while (c_parser_next_token_is_not (parser, CPP_EOF) && c_parser_next_token_is_not (parser, CPP_OPEN_BRACE)) c_parser_declaration_or_fndef (parser, false, false, false, - true, false, NULL, vNULL); + true, false); store_parm_decls (); - if (omp_declare_simd_clauses.exists ()) + if (omp_declare_simd_clauses) c_finish_omp_declare_simd (parser, current_function_decl, NULL_TREE, omp_declare_simd_clauses); if (oacc_routine_data) @@ -5699,7 +5703,7 @@ c_parser_compound_statement_nostart (c_parser *parser) bool fallthru_attr_p = false; c_parser_declaration_or_fndef (parser, true, !have_std_attrs, true, true, true, NULL, - vNULL, have_std_attrs, std_attrs, + NULL, have_std_attrs, std_attrs, NULL, &fallthru_attr_p); if (last_stmt && !fallthru_attr_p) @@ -5731,7 +5735,7 @@ c_parser_compound_statement_nostart (c_parser *parser) last_label = false; mark_valid_location_for_stdc_pragma (false); c_parser_declaration_or_fndef (parser, true, true, true, true, - true, NULL, vNULL); + true); /* Following the old parser, __extension__ does not disable this diagnostic. */ restore_extension_diagnostics (ext); @@ -6782,7 +6786,7 @@ c_parser_for_statement (c_parser *parser, bool ivdep, unsigned short unroll, || c_parser_nth_token_starts_std_attributes (parser, 1)) { c_parser_declaration_or_fndef (parser, true, true, true, true, true, - &object_expression, vNULL); + &object_expression); parser->objc_could_be_foreach_context = false; if (c_parser_next_token_is_keyword (parser, RID_IN)) @@ -6813,7 +6817,7 @@ c_parser_for_statement (c_parser *parser, bool ivdep, unsigned short unroll, ext = disable_extension_diagnostics (); c_parser_consume_token (parser); c_parser_declaration_or_fndef (parser, true, true, true, true, - true, &object_expression, vNULL); + true, &object_expression); parser->objc_could_be_foreach_context = false; restore_extension_diagnostics (ext); @@ -11277,7 +11281,7 @@ c_parser_objc_methodprotolist (c_parser *parser) } else c_parser_declaration_or_fndef (parser, false, false, true, - false, true, NULL, vNULL); + false, true); break; } } @@ -17273,12 +17277,12 @@ c_parser_oacc_routine (c_parser *parser, enum pragma_context context) while (c_parser_next_token_is (parser, CPP_KEYWORD) && c_parser_peek_token (parser)->keyword == RID_EXTENSION); c_parser_declaration_or_fndef (parser, true, true, true, false, true, - NULL, vNULL, false, NULL, &data); + NULL, NULL, false, NULL, &data); restore_extension_diagnostics (ext); } else c_parser_declaration_or_fndef (parser, true, true, true, false, true, - NULL, vNULL, false, NULL, &data); + NULL, NULL, false, NULL, &data); } } @@ -18385,8 +18389,7 @@ c_parser_omp_for_loop (location_t loc, c_parser *parser, enum tree_code code, vec_safe_push (for_block, c_begin_compound_stmt (true)); this_pre_body = push_stmt_list (); c_in_omp_for = true; - c_parser_declaration_or_fndef (parser, true, true, true, true, true, - NULL, vNULL); + c_parser_declaration_or_fndef (parser, true, true, true, true, true); c_in_omp_for = false; if (this_pre_body) { @@ -20327,12 +20330,12 @@ c_parser_omp_declare_simd (c_parser *parser, enum pragma_context context) while (c_parser_next_token_is (parser, CPP_KEYWORD) && c_parser_peek_token (parser)->keyword == RID_EXTENSION); c_parser_declaration_or_fndef (parser, true, true, true, false, true, - NULL, clauses); + NULL, &clauses); restore_extension_diagnostics (ext); } else c_parser_declaration_or_fndef (parser, true, true, true, false, true, - NULL, clauses); + NULL, &clauses); break; case pragma_struct: case pragma_param: @@ -20353,7 +20356,7 @@ c_parser_omp_declare_simd (c_parser *parser, enum pragma_context context) if (c_parser_next_tokens_start_declaration (parser)) { c_parser_declaration_or_fndef (parser, true, true, true, true, - true, NULL, clauses); + true, NULL, &clauses); restore_extension_diagnostics (ext); break; } @@ -20362,7 +20365,7 @@ c_parser_omp_declare_simd (c_parser *parser, enum pragma_context context) else if (c_parser_next_tokens_start_declaration (parser)) { c_parser_declaration_or_fndef (parser, true, true, true, true, true, - NULL, clauses); + NULL, &clauses); break; } error ("%<#pragma omp declare %s%> must be followed by " @@ -20843,8 +20846,10 @@ c_finish_omp_declare_variant (c_parser *parser, tree fndecl, tree parms) static void c_finish_omp_declare_simd (c_parser *parser, tree fndecl, tree parms, - vec clauses) + vec *pclauses) { + vec &clauses = *pclauses; + /* Normally first token is CPP_NAME "simd" or "variant". CPP_EOF there indicates error has been reported and CPP_PRAGMA that c_finish_omp_declare_simd has already processed the tokens. */ diff --git a/gcc/dominance.c b/gcc/dominance.c index 6a262ce8283..cc63391a39a 100644 --- a/gcc/dominance.c +++ b/gcc/dominance.c @@ -1227,7 +1227,7 @@ recompute_dominator (enum cdi_direction dir, basic_block bb) from BBS. */ static void -prune_bbs_to_update_dominators (vec bbs, +prune_bbs_to_update_dominators (vec &bbs, bool conservative) { unsigned i; @@ -1379,7 +1379,7 @@ determine_dominators_for_sons (struct graph *g, vec bbs, a block of BBS in the current dominance tree dominate it. */ void -iterate_fix_dominators (enum cdi_direction dir, vec bbs, +iterate_fix_dominators (enum cdi_direction dir, vec &bbs, bool conservative) { unsigned i; diff --git a/gcc/dominance.h b/gcc/dominance.h index 1a8c248ee98..970da02c594 100644 --- a/gcc/dominance.h +++ b/gcc/dominance.h @@ -78,7 +78,7 @@ checking_verify_dominators (cdi_direction dir) basic_block recompute_dominator (enum cdi_direction, basic_block); extern void iterate_fix_dominators (enum cdi_direction, - vec , bool); + vec &, bool); extern void add_to_dominance_info (enum cdi_direction, basic_block); extern void delete_from_dominance_info (enum cdi_direction, basic_block); extern basic_block first_dom_son (enum cdi_direction, basic_block); diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h index 2fe220acf84..579c5bca516 100644 --- a/gcc/ipa-prop.h +++ b/gcc/ipa-prop.h @@ -499,10 +499,10 @@ public: get reallocated, the member vectors and the underlying auto_vecs would get out of sync. */ ipa_call_arg_values (ipa_auto_call_arg_values *aavals) - : m_known_vals (aavals->m_known_vals), - m_known_contexts (aavals->m_known_contexts), - m_known_aggs (aavals->m_known_aggs), - m_known_value_ranges (aavals->m_known_value_ranges) + : m_known_vals (aavals->m_known_vals.to_vec_legacy ()), + m_known_contexts (aavals->m_known_contexts.to_vec_legacy ()), + m_known_aggs (aavals->m_known_aggs.to_vec_legacy ()), + m_known_value_ranges (aavals->m_known_value_ranges.to_vec_legacy ()) {} /* If m_known_vals (vector of known "scalar" values) is sufficiantly long, diff --git a/gcc/tree-ssa-pre.c b/gcc/tree-ssa-pre.c index d2a7395dd8f..ebe95cc6c73 100644 --- a/gcc/tree-ssa-pre.c +++ b/gcc/tree-ssa-pre.c @@ -3107,7 +3107,7 @@ create_expression_by_pieces (basic_block block, pre_expr expr, static bool insert_into_preds_of_block (basic_block block, unsigned int exprnum, - vec avail) + vec &avail) { pre_expr expr = expression_for_id (exprnum); pre_expr newphi; diff --git a/gcc/tree-vect-data-refs.c b/gcc/tree-vect-data-refs.c index 6995efba899..415ca01a02b 100644 --- a/gcc/tree-vect-data-refs.c +++ b/gcc/tree-vect-data-refs.c @@ -212,7 +212,7 @@ vect_mark_for_runtime_alias_test (ddr_p ddr, loop_vec_info loop_vinfo) static void vect_check_nonzero_value (loop_vec_info loop_vinfo, tree value) { - vec checks = LOOP_VINFO_CHECK_NONZERO (loop_vinfo); + const vec &checks = LOOP_VINFO_CHECK_NONZERO (loop_vinfo); for (unsigned int i = 0; i < checks.length(); ++i) if (checks[i] == value) return; @@ -2349,8 +2349,8 @@ vect_enhance_data_refs_alignment (loop_vec_info loop_vinfo) if (do_versioning) { - vec may_misalign_stmts - = LOOP_VINFO_MAY_MISALIGN_STMTS (loop_vinfo); + const vec &may_misalign_stmts + = LOOP_VINFO_MAY_MISALIGN_STMTS (loop_vinfo); stmt_vec_info stmt_info; /* It can now be assumed that the data references in the statements @@ -3364,7 +3364,8 @@ static void vect_check_lower_bound (loop_vec_info loop_vinfo, tree expr, bool unsigned_p, poly_uint64 min_value) { - vec lower_bounds = LOOP_VINFO_LOWER_BOUNDS (loop_vinfo); + vec &lower_bounds + = LOOP_VINFO_LOWER_BOUNDS (loop_vinfo); for (unsigned int i = 0; i < lower_bounds.length (); ++i) if (operand_equal_p (lower_bounds[i].expr, expr, 0)) { @@ -3466,10 +3467,10 @@ vect_prune_runtime_alias_test_list (loop_vec_info loop_vinfo) typedef pair_hash tree_pair_hash; hash_set compared_objects; - vec may_alias_ddrs = LOOP_VINFO_MAY_ALIAS_DDRS (loop_vinfo); + const vec &may_alias_ddrs = LOOP_VINFO_MAY_ALIAS_DDRS (loop_vinfo); vec &comp_alias_ddrs = LOOP_VINFO_COMP_ALIAS_DDRS (loop_vinfo); - vec &check_unequal_addrs + const vec &check_unequal_addrs = LOOP_VINFO_CHECK_UNEQUAL_ADDRS (loop_vinfo); poly_uint64 vect_factor = LOOP_VINFO_VECT_FACTOR (loop_vinfo); tree scalar_loop_iters = LOOP_VINFO_NITERS (loop_vinfo); @@ -5339,7 +5340,7 @@ vect_store_lanes_supported (tree vectype, unsigned HOST_WIDE_INT count, I4: 6 14 22 30 7 15 23 31. */ void -vect_permute_store_chain (vec_info *vinfo, vec dr_chain, +vect_permute_store_chain (vec_info *vinfo, vec &dr_chain, unsigned int length, stmt_vec_info stmt_info, gimple_stmt_iterator *gsi, diff --git a/gcc/tree-vect-slp-patterns.c b/gcc/tree-vect-slp-patterns.c index ad209deca79..b8d09b7832e 100644 --- a/gcc/tree-vect-slp-patterns.c +++ b/gcc/tree-vect-slp-patterns.c @@ -746,7 +746,7 @@ vect_match_call_complex_mla (slp_tree node, unsigned child, of the negate node. */ static inline bool -vect_normalize_conj_loc (vec args, bool *neg_first_p = NULL) +vect_normalize_conj_loc (vec &args, bool *neg_first_p = NULL) { gcc_assert (args.length () == 2); bool neg_found = false; diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c index d71552296bb..d88f65f3550 100644 --- a/gcc/tree-vect-stmts.c +++ b/gcc/tree-vect-stmts.c @@ -4462,7 +4462,7 @@ static void vect_create_vectorized_demotion_stmts (vec_info *vinfo, vec *vec_oprnds, int multi_step_cvt, stmt_vec_info stmt_info, - vec vec_dsts, + vec &vec_dsts, gimple_stmt_iterator *gsi, slp_tree slp_node, enum tree_code code) { diff --git a/gcc/tree-vectorizer.h b/gcc/tree-vectorizer.h index deb22477e28..45b61fa4793 100644 --- a/gcc/tree-vectorizer.h +++ b/gcc/tree-vectorizer.h @@ -1990,8 +1990,8 @@ extern bool vect_grouped_store_supported (tree, unsigned HOST_WIDE_INT); extern bool vect_store_lanes_supported (tree, unsigned HOST_WIDE_INT, bool); extern bool vect_grouped_load_supported (tree, bool, unsigned HOST_WIDE_INT); extern bool vect_load_lanes_supported (tree, unsigned HOST_WIDE_INT, bool); -extern void vect_permute_store_chain (vec_info *, - vec ,unsigned int, stmt_vec_info, +extern void vect_permute_store_chain (vec_info *, vec &, + unsigned int, stmt_vec_info, gimple_stmt_iterator *, vec *); extern tree vect_setup_realignment (vec_info *, stmt_vec_info, gimple_stmt_iterator *, diff --git a/gcc/vec.c b/gcc/vec.c index f9dbb2cac31..6d767cc12c1 100644 --- a/gcc/vec.c +++ b/gcc/vec.c @@ -38,16 +38,6 @@ along with GCC; see the file COPYING3. If not see #include "diagnostic-core.h" #endif -/* vNULL is an empty type with a template cast operation that returns - a zero-initialized vec instance. Use this when you want - to assign nil values to new vec instances or pass a nil vector as - a function call argument. - - We use this technique because vec must be PODs (they are - stored in unions and passed in vararg functions), this means that - they cannot have ctors/dtors. */ -vnull vNULL; - /* Vector memory usage. */ class vec_usage: public mem_usage { @@ -282,6 +272,42 @@ safe_push_range (vec &v, int start, int limit) v.safe_push (i); } +/* Verify forms of initialization. */ + +static void +test_init () +{ + { + vec v1{ }; + ASSERT_EQ (0, v1.length ()); + + vec v2 (v1); + ASSERT_EQ (0, v2.length ()); + } + + { + vec v1 = vec(); + ASSERT_EQ (0, v1.length ()); + + vec v2 = v1; + ASSERT_EQ (0, v2.length ()); + } + + { + vec v1 (vNULL); + ASSERT_EQ (0, v1.length ()); + v1.safe_push (1); + + vec v2 (v1); + ASSERT_EQ (1, v1.length ()); + v2.safe_push (1); + + ASSERT_EQ (2, v1.length ()); + ASSERT_EQ (2, v2.length ()); + v1.release (); + } +} + /* Verify that vec::quick_push works correctly. */ static void @@ -547,6 +573,7 @@ test_auto_delete_vec () void vec_c_tests () { + test_init (); test_quick_push (); test_safe_push (); test_truncate (); diff --git a/gcc/vec.h b/gcc/vec.h index 30ef9a69473..b3f47b1f65b 100644 --- a/gcc/vec.h +++ b/gcc/vec.h @@ -541,18 +541,16 @@ vec_copy_construct (T *dst, const T *src, unsigned n) ::new (static_cast(dst)) T (*src); } -/* Type to provide NULL values for vec. This is used to - provide nil initializers for vec instances. Since vec must be - a POD, we cannot have proper ctor/dtor for it. To initialize - a vec instance, you can assign it the value vNULL. This isn't - needed for file-scope and function-local static vectors, which - are zero-initialized by default. */ -struct vnull -{ - template - CONSTEXPR operator vec () const { return vec(); } -}; -extern vnull vNULL; +/* Type to provide zero-initialized values for vec. This is + used to provide nil initializers for vec instances. Since vec must + be a trivially copyable type that can be copied by memcpy and zeroed + out by memset, it must have defaulted default and copy ctor and copy + assignment. To initialize a vec either use value initialization + (e.g., vec() or vec v{ };) or assign it the value vNULL. This isn't + needed for file-scope and function-local static vectors, which are + zero-initialized by default. */ +struct vnull { }; +constexpr vnull vNULL{ }; /* Embeddable vector. These vectors are suitable to be embedded @@ -1431,10 +1429,34 @@ gt_pch_nx (vec *v, gt_pointer_operator op, void *cookie) As long as we use C++03, we cannot have constructors nor destructors in classes that are stored in unions. */ +template +class auto_vec; + template struct vec { public: + /* Default ctors to ensure triviality. Use value-initialization + (e.g., vec() or vec v{ };) or vNULL to create a zero-initialized + instance. */ + vec () = default; + vec (const vec &) = default; + /* Initialization from the generic vNULL. */ + vec (vnull): m_vec () { } + /* Same as default ctor: vec storage must be released manually. */ + ~vec () = default; + + /* Defaulted same as copy ctor. */ + vec& operator= (const vec &) = default; + + /* Prevent implicit conversion from auto_vec. Use auto_vec::to_vec() + instead. */ + template + vec (auto_vec &) = delete; + + template + void operator= (auto_vec &) = delete; + /* Memory allocation and deallocation for the embedded vector. Needed because we cannot have proper ctors/dtors defined. */ void create (unsigned nelems CXX_MEM_STAT_INFO); @@ -1522,7 +1544,7 @@ public: want to ask for internal storage for vectors on the stack because if the size of the vector is larger than the internal storage that space is wasted. */ -template +template class auto_vec : public vec { public: @@ -1549,6 +1571,14 @@ public: this->release (); } + /* Explicitly convert to the base class. There is no conversion + from a const auto_vec because a copy of the returned vec can + be used to modify *THIS. + This is a legacy function not to be used in new code. */ + vec to_vec_legacy () { + return *static_cast *>(this); + } + private: vec m_auto; T m_data[MAX (N - 1, 1)]; @@ -1602,6 +1632,14 @@ public: return *this; } + /* Explicitly convert to the base class. There is no conversion + from a const auto_vec because a copy of the returned vec can + be used to modify *THIS. + This is a legacy function not to be used in new code. */ + vec to_vec_legacy () { + return *static_cast *>(this); + } + // You probably don't want to copy a vector, so these are deleted to prevent // unintentional use. If you really need a copy of the vectors contents you // can use copy (). @@ -1781,7 +1819,7 @@ template inline vec vec::copy (ALONE_MEM_STAT_DECL) const { - vec new_vec = vNULL; + vec new_vec{ }; if (length ()) new_vec.m_vec = m_vec->copy (ALONE_PASS_MEM_STAT); return new_vec; --------------26B7EA46A823CC455716F24F--