From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x534.google.com (mail-ed1-x534.google.com [IPv6:2a00:1450:4864:20::534]) by sourceware.org (Postfix) with ESMTPS id 4CB90385AC21 for ; Fri, 25 Jun 2021 08:29:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4CB90385AC21 Received: by mail-ed1-x534.google.com with SMTP id s15so12258792edt.13 for ; Fri, 25 Jun 2021 01:29:38 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=59SMI+/4JXqRqb71Zb1ZlCpsxb9Icv6WloSiULHYwjI=; b=ZMncZhnSEhl0wH+0NRQY09aRrF2UKWiDx0TCGQ5EmzgJrzrUtE10P/irE0ztFzKTPC X/7BiTXD27dDSF9op3ABoPL0MLFnfd3oNOTebxiBTLFC/ax7Bkh+DyX14HJMZeZpT81W CIS5n5VNsDroyu/ZFHi8lB9HVl9v9rPFjrtJj13XWzSLOi++XzLZGLKKuytJ++K1IiXH Mecwt0Uk4UY8d3WtlfiAl74lWjt6DYQxtql0GXz5gSj7APyKGZpy5HpQgpfxR8RwlAXS 240C9eNnCdZOFNVluY4qnjPey2Y76vWUsiNu1n+3Lx8ns0cLPan1TIDk9Qrso+P9vHCp FvdA== X-Gm-Message-State: AOAM532aZkPw8UOkMPJvOdGo/GJiDHltCSnskkGgOc19tkRLjPxeKW/7 7AzHNif4Y9KyT8nWdv5hUdqR5V/G5sIaU2XFnYGoaIeRoB8= X-Google-Smtp-Source: ABdhPJwFpOyIM5DZyseAGyTE6HpBr/Tquw/xn5SPcLZRg20j+krk3pjanTgGTYjUmWEAmwp0wgjifF5HX6cn4I/dL1g= X-Received: by 2002:a05:6402:42c9:: with SMTP id i9mr12933920edc.61.1624609776967; Fri, 25 Jun 2021 01:29:36 -0700 (PDT) MIME-Version: 1.0 References: <20210615055922.27205-5-tbsaunde@tbsaunde.org> <9a54c5e1-2d41-f499-e176-84913b810b67@gmail.com> <125764da-111e-7e44-b2ba-8389644423e8@gmail.com> <08396d8a-ff3e-ba6e-756a-ad4226b920a7@gmail.com> In-Reply-To: From: Richard Biener Date: Fri, 25 Jun 2021 10:29:25 +0200 Message-ID: Subject: Re: [PATCH 5/6] make get_domminated_by_region return a auto_vec To: Richard Biener via Gcc-patches , Richard Biener , Trevor Saunders , Martin Sebor , Richard Sandiford Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-3.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: Fri, 25 Jun 2021 08:29:40 -0000 On Thu, Jun 24, 2021 at 6:28 PM Richard Sandiford wrote: > > Richard Biener via Gcc-patches writes: > > On Wed, Jun 23, 2021 at 12:22 PM Richard Sandiford > > wrote: > >> > >> Richard Biener writes: > >> > On Wed, Jun 23, 2021 at 7:23 AM Trevor Saunders wrote: > >> >> > >> >> On Tue, Jun 22, 2021 at 02:01:24PM -0600, Martin Sebor wrote: > >> >> > On 6/21/21 1:15 AM, Richard Biener wrote: > >> > [...] > >> >> > > > >> >> > > But maybe I'm misunderstanding C++ too much :/ > >> >> > > > >> >> > > Well, I guess b) from above means auto_vec<> passing to > >> >> > > vec<> taking functions will need changes? > >> >> > > >> >> > Converting an auto_vec object to a vec slices off its data members. > >> >> > The auto_vec specialization has no data members so that's not > >> >> > a bug in and of itself, but auto_vec does have data members > >> >> > so that would be a bug. The risk is not just passing it to > >> >> > functions by value but also returning it. That risk was made > >> >> > worse by the addition of the move ctor. > >> >> > >> >> I would agree that the conversion from auto_vec<> to vec<> is > >> >> questionable, and should get some work at some point, perhaps just > >> >> passingauto_vec references is good enough, or perhaps there is value in > >> >> some const_vec view to avoid having to rely on optimizations, I'm not > >> >> sure without looking more at the usage. > >> > > >> > We do need to be able to provide APIs that work with both auto_vec<> > >> > and vec<>, I agree that those currently taking a vec<> by value are > >> > fragile (and we've had bugs there before), but I'm not ready to say > >> > that changing them all to [const] vec<>& is OK. The alternative > >> > would be passing a const_vec<> by value, passing that along to > >> > const vec<>& APIs should be valid then (I can see quite some API > >> > boundary cleanups being necessary here ...). > >> > >> FWIW, as far as const_vec<> goes, we already have array_slice, which is > >> mostly a version of std::span. (The only real reason for not using > >> std::span itself is that it requires a later version of C++.) > >> > >> Taking those as arguments has the advantage that we can pass normal > >> arrays as well. > > > > It's not really a "const" thing it seems though it certainly would not expose > > any API that would reallocate the vector (which is the problematic part > > of passing vec<> by value, not necessarily changing elements in-place). > > > > Since array_slice doesn't inherit most of the vec<> API transforming an > > API from vec<> to array_slice<> will be also quite some work. But I > > agree it might be useful for generic API stuff. > > Which API functions would be the most useful ones to have? We could > add them if necessary. I think we'll see when introducing uses. I guess that vec<> to const vec<>& changes will be mostly fine but for vec<> to vec<>& I might prefer vec<> to array_slice<> since that makes clear the caller won't re-allocate. We'll see what those APIs would require then. > There again, for things like searching and sorting, I think it would > be better to use where possible. It should usually be > more efficient than the void* callback approach that the C code tended > to use. Not sure whether really is better, we've specifically replaced libc qsort calls with our own sort to avoid all sorts of host isues and the vec<>::bsearch is inline and thus the callbacks can be inlined. Richard. > Thanks, > Richard