From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 83454 invoked by alias); 3 Jun 2016 15:07:16 -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 83445 invoked by uid 89); 3 Jun 2016 15:07:15 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 spammy=fond, him, his, opportunity X-HELO: mail-qg0-f44.google.com Received: from mail-qg0-f44.google.com (HELO mail-qg0-f44.google.com) (209.85.192.44) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Fri, 03 Jun 2016 15:07:14 +0000 Received: by mail-qg0-f44.google.com with SMTP id j96so3369482qge.0 for ; Fri, 03 Jun 2016 08:07:14 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=AFaK9LoMVPmuOegZbUGvG4Y/WmZH8YhNPxNmpQDqXYU=; b=UUyAR9BEZRrz4Y8qDzD4YBZLS2hbIh6sH/4CcVxi7Gw1JQwYWXkuVUQYGPZOf28yV1 b8uq3cGRlJEDkpRAqLq/8lmEdZ8NIdzC4Ih2WAx+CNjJe5ceShK3OMtXgL9a7RTqMKWZ bH/Qoghm16sZ8wBmgeT/JtATELsgBJlXHESr8ixNt8bPPO1Zc25dcT+0AvfqS8wvgdqx SRdV+w+Bn/YwNXBdo8wAmr9I/YlnAXnIFkkI7Bhpxo5tyRmxkL2IbZxH+hcyr+6EEFB/ C/cVeRcfYdwt9w8a2RqBq+TWuwTTXu3dvMryAyyZG6B2r9HptVkGFf7v8MnxYPlLwuml YSVg== X-Gm-Message-State: ALyK8tLKxcj9BvItqpXEVUbMohrIfvdY5rYcVjbXztbOCwQXAc80WJqCUlwLMhb6IAsHnQ== X-Received: by 10.141.44.4 with SMTP id v4mr3625739qhe.29.1464966432385; Fri, 03 Jun 2016 08:07:12 -0700 (PDT) Received: from [192.168.0.26] (75-166-216-131.hlrn.qwest.net. [75.166.216.131]) by smtp.gmail.com with ESMTPSA id y12sm1613030qhy.32.2016.06.03.08.07.10 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 03 Jun 2016 08:07:11 -0700 (PDT) Subject: Re: [PATCH] integer overflow checking builtins in constant expressions To: Jakub Jelinek References: <57263154.5080401@gmail.com> <20160531215025.GK28550@tucnak.redhat.com> <574E13DD.9040401@gmail.com> <20160601075212.GL28550@tucnak.redhat.com> <574EFC8F.2040400@gmail.com> <574FA3BC.8090603@gmail.com> <20160602072316.GY28550@tucnak.redhat.com> <20160602073404.GZ28550@tucnak.redhat.com> Cc: Gcc Patch List , Jason Merrill , "Joseph S. Myers" From: Martin Sebor Message-ID: <57519D1D.4000208@gmail.com> Date: Fri, 03 Jun 2016 15:07:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: <20160602073404.GZ28550@tucnak.redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2016-06/txt/msg00282.txt.bz2 On 06/02/2016 01:34 AM, Jakub Jelinek wrote: > On Thu, Jun 02, 2016 at 09:23:16AM +0200, Jakub Jelinek wrote: >> Also, perhaps just a documentation thing, it would be good to clarify the >> NULL last argument. From the POV of generating efficient code, I think we >> should say something that the last argument (for the GNU builtins) must be >> either a pointer to a valid object, or NULL/nullptr constant cast expression >> cast to a pointer type, but nothing else. That is actually what your patch >> implements. But, I'd like to make sure that >> int *p = NULL; >> if (__builtin_add_overflow (a, b, p)) >> ... >> is actually not valid, otherwise we unnecessarily pessimize many of the GNU >> style calls (those that don't pass &var), where instead of >> tem = ADD_OVERFLOW (a, b); >> *p = REALPART_EXPR (tem); >> ovf = IMAGPART_EXPR (tem); >> we'd need to emit instead >> tem = ADD_OVERFLOW (a, b); >> ovf = IMAGPART_EXPR (tem); >> if (p != NULL) >> *p = REALPART_EXPR (tem); > > Though, perhaps that is too ugly, that it has different semantics for > __builtin_add_overflow (a, b, (int *) NULL) > and for > int *p = NULL; > __builtin_add_overflow (a, b, p) > Maybe the cleanest would be to just add 3 extra builtins, again, > typegeneric, > __builtin_{add,sub,mul}_overflow_p > where either the arguments would be instead of integertype1, integertype2, > integertype3 * rather integertype1, integertype2, integertype3 > and we'd only care about the type, not value, of the last argument, > so use it like __builtin_add_overflow_p (a, b, (__typeof ((a) + (b))) 0) > or handle those 3 similarly to __builtin_va_arg, and use > __builtin_add_overflow_p (a, b, int); > I think I prefer the former though. I'm not sure I understand your concern but at this point I would prefer to keep things as they are. I like that the functionality that was requested in c/68120 could be provided under the existing interface, and I'm not fond of the idea of adding yet another set of built-ins just to get at a bit that's already available via the existing ones (in C++ 11, even in constexpr contexts, provided the built-ins were allowed there). I've also spent far more time on this patch than I had planned and I'm way behind on the tasks I was asked to work on. That said, in c/68120 the requester commented that he's considering submitting a request for yet another enhancement in this area, so I think letting him play with what we've got now for a bit will be an opportunity to get his feedback and tweak the API further based on it. Martin