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.129.124]) by sourceware.org (Postfix) with ESMTPS id 1E6813858D1E for ; Tue, 20 Jun 2023 15:22:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 1E6813858D1E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1687274568; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=qBYvjUeFww30z4ap15ZG6sXoqTuIXeiT7VYPxzTmuPw=; b=E9NTBxTlRpyVwARUyQU/jmDosQ1/IV1jdsIuCrLOWxPSnAq+H7ZK7Tl6aaRJAS8JsPmTn9 BEJVtHZ1zFAMFgeZ1C7BS4vAfnoYf3kibXp10+WhLsM82T23DFD4vwA0cvw9IyA4xsmnT+ E0u3PECrM3UHwmWbXzY2u37L/UXlCvQ= Received: from mail-ua1-f69.google.com (209.85.222.69 [209.85.222.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-441-B4PRH5PfPvG2MRNbXOiu8w-1; Tue, 20 Jun 2023 11:22:27 -0400 X-MC-Unique: B4PRH5PfPvG2MRNbXOiu8w-1 Received: by mail-ua1-f69.google.com with SMTP id a1e0cc1a2514c-791bbac1571so25345241.2 for ; Tue, 20 Jun 2023 08:21:57 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687274514; x=1689866514; h=mime-version:references:message-id:in-reply-to:subject:cc:to:date :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=eRmXBz/DvS0BcjTUp76a+X4OV1/d3XiJlYCl8iXzgIg=; b=L5/B9ne2zsRLH4dS0Efs3lcw5cc5MxUwSZrdbNsOF12dhnacsPCzzo6FHoFyBJfDVj YHbTnVHreycYi2d64FAeNXlK22gaToyiTbe0F0RwcbyI2+aI2VC4i5LTIqSGZbZda5Dm eiCtE92PhqsUOnKicJSRsCvinzuxfXz4rd4YR6igxailkySZzNBNA48UbXiVA0ffAiBd q19dCjmZlZdIx7lWcshQYetdSmWSB6scEbV6MNBGIC76VpQeeOmxOjtNYvN4kEq3DwoC DNKsOaBO2DrEjzu/FycgHvHmC/IHQAG4SSHP7eyi1xyTtuCMyBg31Bna4F9zFya6Njx3 qiiw== X-Gm-Message-State: AC+VfDxRE/or8+qj0EYX2ZXZhbD3qdgxde1hRjvktzASUOI3+a/kXqaV 3DhqdYdyCsPfnUp65c6CnoiZ91suEmTnyjhot6S+1PFM/CpB3JpHyV8NgegfeAEWVryLcDaYNyP yVFUbQynZK/IcZvqBrg== X-Received: by 2002:a67:e404:0:b0:43b:4486:8b84 with SMTP id d4-20020a67e404000000b0043b44868b84mr4165744vsf.21.1687274514222; Tue, 20 Jun 2023 08:21:54 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ7QrE3RS2uXLSj2YNGdrKlqhCd1T5Gktmz4opXVMOMIUhW8C30x4ZncSkH9/SV3HtGjAYsteA== X-Received: by 2002:a67:e404:0:b0:43b:4486:8b84 with SMTP id d4-20020a67e404000000b0043b44868b84mr4165727vsf.21.1687274513908; Tue, 20 Jun 2023 08:21:53 -0700 (PDT) Received: from [192.168.1.130] (ool-457670bb.dyn.optonline.net. [69.118.112.187]) by smtp.gmail.com with ESMTPSA id v16-20020a0ce1d0000000b0063014585a9asm1361752qvl.56.2023.06.20.08.21.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 20 Jun 2023 08:21:53 -0700 (PDT) From: Patrick Palka X-Google-Original-From: Patrick Palka Date: Tue, 20 Jun 2023 11:21:52 -0400 (EDT) To: Ken Matsui cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH 1/2] c++: implement __remove_pointer built-in trait In-Reply-To: Message-ID: References: <20230615122129.20213-1-kmatsui@cs.washington.edu> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: multipart/mixed; boundary="8323329-1099053026-1687274513=:278865" X-Spam-Status: No, score=-13.8 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. --8323329-1099053026-1687274513=:278865 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT On Sat, 17 Jun 2023, Ken Matsui via Gcc-patches wrote: > Hi, > > I conducted a benchmark for remove_pointer as well as is_object. Just > like the is_object benchmark, here is the benchmark code: > > https://github.com/ken-matsui/gcc-benches/blob/main/remove_pointer_benchmark.cc > > On my computer, using the gcc HEAD of this patch for a release build, > the patch with -DUSE_BUILTIN took 8.7% less time and used 4.3-4.9% > less memory on average compared to not using it. Although the > performance improvement was not as significant as with is_object, the > benchmark demonstrated that the compilation was consistently more > efficient. Thanks for the benchmark. The improvement is lesser than I expected, but that might be because the benchmark is "biased": template struct Instantiator : Instantiator { static_assert(!std::is_pointer_v>); }; This only invokes remove_pointer_t on the non-pointer type Instantiator, and so the benchmark doesn't factor in the performance of the trait when invoked on pointer types, and traits typically will have different performance characteristics depending on the kind of type it's given. To more holistically assess the real-world performance of the trait the benchmark should also consider pointer types and maybe also cv-qualified types (given that the original implementation is in terms of __remove_cv_t and thus performance of the original implementation may be sensitive to cv-qualification). So we should probably uniformly benchmark these classes of types, via doing e.g.: static_assert(!std::is_pointer_v>); static_assert(!std::is_pointer_v>); static_assert(!std::is_pointer_v>); static_assert(!std::is_pointer_v>); (We could consider other kinds of types too, e.g. reference types and integral types, but it seems clear based on the implementations being benchmarked that performance won't be sensitive to reference-ness or integral-ness.) > > Sincerely, > Ken Matsui > > On Thu, Jun 15, 2023 at 5:22 AM Ken Matsui wrote: > > > > This patch implements built-in trait for std::remove_pointer. > > > > gcc/cp/ChangeLog: > > > > * cp-trait.def: Define __remove_pointer. > > * semantics.cc (finish_trait_type): Handle CPTK_REMOVE_POINTER. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/ext/has-builtin-1.C: Test existence of __remove_pointer. > > * g++.dg/ext/remove_pointer.C: New test. > > > > Signed-off-by: Ken Matsui > > --- > > gcc/cp/cp-trait.def | 1 + > > gcc/cp/semantics.cc | 4 ++ > > gcc/testsuite/g++.dg/ext/has-builtin-1.C | 3 ++ > > gcc/testsuite/g++.dg/ext/remove_pointer.C | 51 +++++++++++++++++++++++ > > 4 files changed, 59 insertions(+) > > create mode 100644 gcc/testsuite/g++.dg/ext/remove_pointer.C > > > > diff --git a/gcc/cp/cp-trait.def b/gcc/cp/cp-trait.def > > index 8b7fece0cc8..07823e55579 100644 > > --- a/gcc/cp/cp-trait.def > > +++ b/gcc/cp/cp-trait.def > > @@ -90,6 +90,7 @@ DEFTRAIT_EXPR (IS_DEDUCIBLE, "__is_deducible ", 2) > > DEFTRAIT_TYPE (REMOVE_CV, "__remove_cv", 1) > > DEFTRAIT_TYPE (REMOVE_REFERENCE, "__remove_reference", 1) > > DEFTRAIT_TYPE (REMOVE_CVREF, "__remove_cvref", 1) > > +DEFTRAIT_TYPE (REMOVE_POINTER, "__remove_pointer", 1) > > DEFTRAIT_TYPE (UNDERLYING_TYPE, "__underlying_type", 1) > > DEFTRAIT_TYPE (TYPE_PACK_ELEMENT, "__type_pack_element", -1) > > > > diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc > > index 8fb47fd179e..885c7a6fb64 100644 > > --- a/gcc/cp/semantics.cc > > +++ b/gcc/cp/semantics.cc > > @@ -12373,6 +12373,10 @@ finish_trait_type (cp_trait_kind kind, tree type1, tree type2, > > if (TYPE_REF_P (type1)) > > type1 = TREE_TYPE (type1); > > return cv_unqualified (type1); > > + case CPTK_REMOVE_POINTER: > > + if (TYPE_PTR_P (type1)) > > + type1 = TREE_TYPE (type1); > > + return type1; Maybe add a newline before the 'case' to visually separate it from the previous 'case'? LGTM otherwise, thanks! > > > > case CPTK_TYPE_PACK_ELEMENT: > > return finish_type_pack_element (type1, type2, complain); > > diff --git a/gcc/testsuite/g++.dg/ext/has-builtin-1.C b/gcc/testsuite/g++.dg/ext/has-builtin-1.C > > index f343e153e56..e21e0a95509 100644 > > --- a/gcc/testsuite/g++.dg/ext/has-builtin-1.C > > +++ b/gcc/testsuite/g++.dg/ext/has-builtin-1.C > > @@ -146,3 +146,6 @@ > > #if !__has_builtin (__remove_cvref) > > # error "__has_builtin (__remove_cvref) failed" > > #endif > > +#if !__has_builtin (__remove_pointer) > > +# error "__has_builtin (__remove_pointer) failed" > > +#endif > > diff --git a/gcc/testsuite/g++.dg/ext/remove_pointer.C b/gcc/testsuite/g++.dg/ext/remove_pointer.C > > new file mode 100644 > > index 00000000000..7b13db93950 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/ext/remove_pointer.C > > @@ -0,0 +1,51 @@ > > +// { dg-do compile { target c++11 } } > > + > > +#define SA(X) static_assert((X),#X) > > + > > +SA(__is_same(__remove_pointer(int), int)); > > +SA(__is_same(__remove_pointer(int*), int)); > > +SA(__is_same(__remove_pointer(int**), int*)); > > + > > +SA(__is_same(__remove_pointer(const int*), const int)); > > +SA(__is_same(__remove_pointer(const int**), const int*)); > > +SA(__is_same(__remove_pointer(int* const), int)); > > +SA(__is_same(__remove_pointer(int** const), int*)); > > +SA(__is_same(__remove_pointer(int* const* const), int* const)); > > + > > +SA(__is_same(__remove_pointer(volatile int*), volatile int)); > > +SA(__is_same(__remove_pointer(volatile int**), volatile int*)); > > +SA(__is_same(__remove_pointer(int* volatile), int)); > > +SA(__is_same(__remove_pointer(int** volatile), int*)); > > +SA(__is_same(__remove_pointer(int* volatile* volatile), int* volatile)); > > + > > +SA(__is_same(__remove_pointer(const volatile int*), const volatile int)); > > +SA(__is_same(__remove_pointer(const volatile int**), const volatile int*)); > > +SA(__is_same(__remove_pointer(const int* volatile), const int)); > > +SA(__is_same(__remove_pointer(volatile int* const), volatile int)); > > +SA(__is_same(__remove_pointer(int* const volatile), int)); > > +SA(__is_same(__remove_pointer(const int** volatile), const int*)); > > +SA(__is_same(__remove_pointer(volatile int** const), volatile int*)); > > +SA(__is_same(__remove_pointer(int** const volatile), int*)); > > +SA(__is_same(__remove_pointer(int* const* const volatile), int* const)); > > +SA(__is_same(__remove_pointer(int* volatile* const volatile), int* volatile)); > > +SA(__is_same(__remove_pointer(int* const volatile* const volatile), int* const volatile)); > > + > > +SA(__is_same(__remove_pointer(int&), int&)); > > +SA(__is_same(__remove_pointer(const int&), const int&)); > > +SA(__is_same(__remove_pointer(volatile int&), volatile int&)); > > +SA(__is_same(__remove_pointer(const volatile int&), const volatile int&)); > > + > > +SA(__is_same(__remove_pointer(int&&), int&&)); > > +SA(__is_same(__remove_pointer(const int&&), const int&&)); > > +SA(__is_same(__remove_pointer(volatile int&&), volatile int&&)); > > +SA(__is_same(__remove_pointer(const volatile int&&), const volatile int&&)); > > + > > +SA(__is_same(__remove_pointer(int[3]), int[3])); > > +SA(__is_same(__remove_pointer(const int[3]), const int[3])); > > +SA(__is_same(__remove_pointer(volatile int[3]), volatile int[3])); > > +SA(__is_same(__remove_pointer(const volatile int[3]), const volatile int[3])); > > + > > +SA(__is_same(__remove_pointer(int(int)), int(int))); > > +SA(__is_same(__remove_pointer(int(*const)(int)), int(int))); > > +SA(__is_same(__remove_pointer(int(*volatile)(int)), int(int))); > > +SA(__is_same(__remove_pointer(int(*const volatile)(int)), int(int))); > > -- > > 2.41.0 > > > > --8323329-1099053026-1687274513=:278865--