From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id 851833985450 for ; Wed, 14 Jul 2021 16:23:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 851833985450 Received: from mail-qv1-f70.google.com (mail-qv1-f70.google.com [209.85.219.70]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-190-fdE8Uf_1N9eR9xMz50VARQ-1; Wed, 14 Jul 2021 12:23:25 -0400 X-MC-Unique: fdE8Uf_1N9eR9xMz50VARQ-1 Received: by mail-qv1-f70.google.com with SMTP id c5-20020a0562141465b02902e2f9404330so2003609qvy.9 for ; Wed, 14 Jul 2021 09:23:24 -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 :content-transfer-encoding; bh=dywt6XZ6HdFMYNKFe7fl9Oaqc4+yxNz0Gb3L0Ffh3MA=; b=D+5PxNE1ERkIpV/SuC5AiQo3w7QAnz/beFYHNlrj1GRny+SUQJ9wULp5+YnhUBYFP8 9lismPwRL8w8iBOU0GlcTdp8QZYUpgaDYz1NVdEOQavI+OeDcXYiD4jHg868RjQBJws/ zMFd3wOrW3FjJquhyTpuWp2W0WtqH5Xn7jaPNOLaKf7Scc7au8B24czId/Y29ag7lVIq bShLXa45iK0oObjXM0ouYGg/NSezvDB3fpoCQ0JS1l8hc3YC/4Pz+/2IoyWVhZnpooY2 61dIQbYr6GqD74G1L2eyFIYy4RmdYUQkqI+nHwXgyVHdt+slaXxBVrSNqxbmaYjLEMMB mgmg== X-Gm-Message-State: AOAM530tmeF0zsGwFsf1wAdCH1Ponl7SlE0lM+zWKXoUlqw0jaTd71nH e7HtOnaJUt2jbZfpTr40BhpihMbNRH6jb3zcUCeKXEq9oE+DKQjIuDcukyuhautCg+L6fRYPvCx s4BexNN+JHm2rvLAZLoylbnqZKreC7+uZqteDoJroECSr+e5qBQgDBbmXQhXf+mxfiA== X-Received: by 2002:ad4:56a4:: with SMTP id bd4mr2431413qvb.6.1626279803942; Wed, 14 Jul 2021 09:23:23 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwXuyYlMKOkfnWQtGBRhJPRH27P++3jMRcovzwMpEhMMPV8ycj9uFCMYPai2GryOyT0x5iA6A== X-Received: by 2002:ad4:56a4:: with SMTP id bd4mr2431375qvb.6.1626279803574; Wed, 14 Jul 2021 09:23:23 -0700 (PDT) Received: from [192.168.1.148] (130-44-159-43.s11817.c3-0.arl-cbr1.sbo-arl.ma.cable.rcncustomer.com. [130.44.159.43]) by smtp.gmail.com with ESMTPSA id 5sm1266255qkr.100.2021.07.14.09.23.22 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 14 Jul 2021 09:23:22 -0700 (PDT) Subject: Re: [PING][PATCH] define auto_vec copy ctor and assignment (PR 90904) To: Martin Sebor , Jonathan Wakely , Richard Biener Cc: gcc-patches References: <49569f1d-9856-55c7-b9e9-578bbd7c7b7a@gmail.com> <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> From: Jason Merrill Message-ID: Date: Wed, 14 Jul 2021 12:23:22 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <072a4715-2248-d836-948d-50426160de47@gmail.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-7.4 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, KAM_SHORT, NICE_REPLY_A, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, 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: Wed, 14 Jul 2021 16:23:28 -0000 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. 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. > + 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&? > - 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? Jason