From patchwork Wed Nov 23 10:37:09 2022
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
X-Patchwork-Submitter: Jakub Jelinek
X-Patchwork-Id: 61023
Return-Path:
X-Original-To: patchwork@sourceware.org
Delivered-To: patchwork@sourceware.org
Received: from server2.sourceware.org (localhost [IPv6:::1])
by sourceware.org (Postfix) with ESMTP id AA91C3853D50
for ; Wed, 23 Nov 2022 10:37:45 +0000 (GMT)
DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org AA91C3853D50
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org;
s=default; t=1669199865;
bh=SAcDaZZq0qsZCGhnmgkwpOdcrpz2u4fcJJUJEBZRp1Q=;
h=Date:To:Cc:Subject:References:In-Reply-To:List-Id:
List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe:
From:Reply-To:From;
b=JP0MgerF68LwUyzxCixprwpEV9RPIiQMNZ2Sv+tdKlxlCEgRpCWUioJCpXnLpSsZZ
hkztpz0yXwi10U0VRHONZLjkBCFftm0PURod+Zh8jyCDNs9knZGbPjzZGuEyNFyAHg
qNSQ5NZIImtZQGLb21BBFCs+yQnYe+zSLdW2Kq0w=
X-Original-To: gcc-patches@gcc.gnu.org
Delivered-To: gcc-patches@gcc.gnu.org
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 D39513858D32
for ; Wed, 23 Nov 2022 10:37:17 +0000 (GMT)
DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D39513858D32
Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com
[66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS
(version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id
us-mta-213-Z2DsiOLxNxiIuIhnX5xziA-1; Wed, 23 Nov 2022 05:37:15 -0500
X-MC-Unique: Z2DsiOLxNxiIuIhnX5xziA-1
Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com
[10.11.54.3])
(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))
(No client certificate requested)
by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 8682785A588;
Wed, 23 Nov 2022 10:37:15 +0000 (UTC)
Received: from tucnak.zalov.cz (unknown [10.39.194.202])
by smtp.corp.redhat.com (Postfix) with ESMTPS id 18A2A1121314;
Wed, 23 Nov 2022 10:37:14 +0000 (UTC)
Received: from tucnak.zalov.cz (localhost [127.0.0.1])
by tucnak.zalov.cz (8.17.1/8.17.1) with ESMTPS id 2ANAbASD2532446
(version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT);
Wed, 23 Nov 2022 11:37:10 +0100
Received: (from jakub@localhost)
by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 2ANAb9Zl2532445;
Wed, 23 Nov 2022 11:37:09 +0100
Date: Wed, 23 Nov 2022 11:37:09 +0100
To: Jason Merrill ,
"Joseph S. Myers" ,
Marek Polacek
Cc: gcc-patches@gcc.gnu.org
Subject: [PATCH] c-family: Incremental fix for -Wsign-compare BIT_NOT_EXPR
handling [PR107465]
Message-ID:
References:
MIME-Version: 1.0
In-Reply-To:
X-Scanned-By: MIMEDefang 3.1 on 10.11.54.3
X-Mimecast-Spam-Score: 0
X-Mimecast-Originator: redhat.com
Content-Disposition: inline
X-Spam-Status: No, score=-3.5 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH,
DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE,
RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE,
TXREP autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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-Patchwork-Original-From: Jakub Jelinek via Gcc-patches
From: Jakub Jelinek
Reply-To: Jakub Jelinek
Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org
Sender: "Gcc-patches"
Hi!
There can be too many extensions and seems I didn't get everything right in
the previously posted patch.
The following incremental patch ought to fix that.
The code can deal with quite a few sign/zero extensions at various spots
and it is important to deal with all of them right.
On the argument that contains BIT_NOT_EXPR we have:
MSB bits#4 bits#3 BIT_NOT_EXPR bits#2 bits#1 LSB
where bits#1 is one or more bits (TYPE_PRECISION (TREE_TYPE (arg0))
at the end of the function) we don't know anything about, for the purposes
of this warning it is VARYING that is inverted with BIT_NOT_EXPR to some other
VARYING bits;
bits#2 is one or more bits (TYPE_PRECISION (TREE_TYPE (op0)) -
TYPE_PRECISION (TREE_TYPE (arg0)) at the end of the function)
which are known to be 0 before the BIT_NOT_EXPR and 1 after it.
bits#3 is zero or more bits from the TYPE_PRECISION (TREE_TYPE (op0))
at the end of function to the TYPE_PRECISION (TREE_TYPE (op0)) at the
end of the function to TYPE_PRECISION (TREE_TYPE (op0)) at the start
of the function, which are either zero extension or sign extension.
And bits#4 is zero or more bits from the TYPE_PRECISION (TREE_TYPE (op0))
at the start of the function to TYPE_PRECISION (result_type), which
again can be zero or sign extension.
Now, vanilla trunk as well as the previously posted patch mishandles the
case where bits#3 are sign extended (as bits#2 are known to be all set,
that means bits#3 are all set too) but bits#4 are zero extended and are
thus all 0.
The patch fixes it by tracking the lowest bit which is known to be clear
above the known to be set bits (if any, otherwise it is precision of
result_type).
Ok for trunk if it passes bootstrap/regtest?
2022-11-23 Jakub Jelinek
PR c/107465
* c-warn.cc (warn_for_sign_compare): Don't warn for unset bits
above innermost zero extension of BIT_NOT_EXPR result.
* c-c++-common/Wsign-compare-2.c (f18): New test.
Jakub
--- gcc/c-family/c-warn.cc.jj 2022-11-23 10:04:53.000000000 +0100
+++ gcc/c-family/c-warn.cc 2022-11-23 11:19:38.928113842 +0100
@@ -2344,13 +2344,33 @@ warn_for_sign_compare (location_t locati
have all bits set that are set in the ~ operand when it is
extended. */
+ /* bits0 is the bit index of op0 extended to result_type, which will
+ be always 0 and so all bits above it. If there is a BIT_NOT_EXPR
+ in that operand possibly sign or zero extended to op0 and then
+ possibly further sign or zero extended to result_type, bits0 will
+ be the precision of result type if all the extensions involved
+ if any are sign extensions, and will be the place of the innermost
+ zero extension otherwise. We warn only if BIT_NOT_EXPR's operand is
+ zero extended from some even smaller precision, in that case after
+ BIT_NOT_EXPR some bits below bits0 will be guaranteed to be set.
+ Similarly for bits1. */
+ int bits0 = TYPE_PRECISION (result_type);
+ if (TYPE_UNSIGNED (TREE_TYPE (op0)))
+ bits0 = TYPE_PRECISION (TREE_TYPE (op0));
tree arg0 = c_common_get_narrower (op0, &unsignedp0);
if (TYPE_PRECISION (TREE_TYPE (arg0)) == TYPE_PRECISION (TREE_TYPE (op0)))
unsignedp0 = TYPE_UNSIGNED (TREE_TYPE (op0));
+ else if (unsignedp0)
+ bits0 = TYPE_PRECISION (TREE_TYPE (arg0));
op0 = arg0;
+ int bits1 = TYPE_PRECISION (result_type);
+ if (TYPE_UNSIGNED (TREE_TYPE (op1)))
+ bits1 = TYPE_PRECISION (TREE_TYPE (op1));
tree arg1 = c_common_get_narrower (op1, &unsignedp1);
if (TYPE_PRECISION (TREE_TYPE (arg1)) == TYPE_PRECISION (TREE_TYPE (op1)))
unsignedp1 = TYPE_UNSIGNED (TREE_TYPE (op1));
+ else if (unsignedp1)
+ bits1 = TYPE_PRECISION (TREE_TYPE (arg1));
op1 = arg1;
if ((TREE_CODE (op0) == BIT_NOT_EXPR)
@@ -2360,6 +2380,7 @@ warn_for_sign_compare (location_t locati
{
std::swap (op0, op1);
std::swap (unsignedp0, unsignedp1);
+ std::swap (bits0, bits1);
}
int unsignedp;
@@ -2378,16 +2399,8 @@ warn_for_sign_compare (location_t locati
&& bits < HOST_BITS_PER_WIDE_INT)
{
HOST_WIDE_INT mask = HOST_WIDE_INT_M1U << bits;
- if (unsignedp0)
- {
- bits = TYPE_PRECISION (TREE_TYPE (op0));
- if (bits < TYPE_PRECISION (result_type)
- && bits < HOST_BITS_PER_WIDE_INT)
- mask &= ~(HOST_WIDE_INT_M1U << bits);
- }
- bits = TYPE_PRECISION (result_type);
- if (bits < HOST_BITS_PER_WIDE_INT)
- mask &= ~(HOST_WIDE_INT_M1U << bits);
+ if (bits0 < HOST_BITS_PER_WIDE_INT)
+ mask &= ~(HOST_WIDE_INT_M1U << bits0);
if ((mask & constant) != mask)
{
if (constant == 0)
@@ -2405,24 +2418,7 @@ warn_for_sign_compare (location_t locati
< TYPE_PRECISION (TREE_TYPE (op0)))
&& unsignedp
&& unsignedp1
- /* If unsignedp0, the BIT_NOT_EXPR result is
- zero extended, so say if op0 is unsigned char
- variable, BIT_NOT_EXPR is unsigned short and
- result type int and op0 has value 0x55, the
- int value will be 0xffaa, or for op0 0xaa it
- will be 0xff55. In these cases, warn if
- op1 is unsigned and narrower than unsigned short.
- While if unsignedp0 is false, the BIT_NOT_EXPR
- result is sign extended and because of the
- above TYPE_PRECISION comparison we know the
- MSB of BIT_NOT_EXPR is set (perhaps with some
- further bits below it). The sign extension will
- then ensure all bits above BIT_NOT_EXPR up to
- result_type's precision are set. */
- && (TYPE_PRECISION (TREE_TYPE (op1))
- < TYPE_PRECISION (unsignedp0
- ? TREE_TYPE (op0)
- : result_type)))
+ && TYPE_PRECISION (TREE_TYPE (op1)) < bits0)
warning_at (location, OPT_Wsign_compare,
"comparison of promoted bitwise complement "
"of an unsigned value with unsigned");
--- gcc/testsuite/c-c++-common/Wsign-compare-2.c.jj 2022-11-23 10:04:53.000000000 +0100
+++ gcc/testsuite/c-c++-common/Wsign-compare-2.c 2022-11-23 11:05:10.497862219 +0100
@@ -104,3 +104,9 @@ f17 (unsigned char x, unsigned short y)
{
return (unsigned short) (~(unsigned short) x) == y; /* { dg-bogus "comparison of promoted bitwise complement of an unsigned value with unsigned" } */
}
+
+int
+f18 (unsigned char x)
+{
+ return (unsigned int) (short) (~(unsigned short) x) == 0xffffff05ULL; /* { dg-bogus "comparison of promoted bitwise complement of an unsigned value with constant" } */
+}