From patchwork Fri Feb 9 08:55:36 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 85503 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 BD7F5385842C for ; Fri, 9 Feb 2024 08:56:17 +0000 (GMT) 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 6AD013858404 for ; Fri, 9 Feb 2024 08:55:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 6AD013858404 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 6AD013858404 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1707468944; cv=none; b=ZpYBqpdQpqN2Eb9l8yr6eJos99eYmohfRWK9UCMY+YfFb1LMBzxMja6TrUmb4mLNbY0UXGA1tsDYSDoQyu7QM4/Ovi+MQI1jJi4SuGzOeM6qneeH2A3CUl7/7beVA7AX1z1gABMaEvmeS5F8ijwGr1sp9Pc3iAwWvxXbHFFVCZA= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1707468944; c=relaxed/simple; bh=cIHukImb6XjKV/CKOnhAMvqZbGc7E/WXEBR8qwa4kEc=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=XIFACKrkuzyYYRrfFbC9rAuHRJdTC/2PW4C73AO1M/NaF/Sxsv42aULaNUquMA3DurIDbbyxOjMsxioOJtgCriPhMWhm0OOq51SMxTUXIhDDJG6JheBwr+bNxXsZemWnZh3x8Sf0EPKgZ+BL79+pU2pMwKw2LsUY9UKGf0IupHw= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1707468942; h=from:from:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type; bh=UDDKprYc167jQ2D9PMTFljYHcafQ/6SnKxeBTvNkgxg=; b=VGeBNR6hOfXXm55Ci5LXweufpAwnHuMCUbB8A96UUJmxN9qZGdH2WAhb3DULIiwdAUZeP8 Q4ecaL6j0RjG4L4AwOhQY1nlDlM0qWoWnR4kJrgOBlgjc9XKY5MjUdqEwISIwLyw7R8o6r rIPzrEWhj/jY5Xz1xKuctUJtv7UYcT4= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-353-a1nk6nbmPJC2Efi2VIUu4g-1; Fri, 09 Feb 2024 03:55:40 -0500 X-MC-Unique: a1nk6nbmPJC2Efi2VIUu4g-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.rdu2.redhat.com [10.11.54.8]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id E8DA085A588; Fri, 9 Feb 2024 08:55:39 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.192.70]) by smtp.corp.redhat.com (Postfix) with ESMTPS id AC950C08EF7; Fri, 9 Feb 2024 08:55:39 +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 4198tbWm639942 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Fri, 9 Feb 2024 09:55:37 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 4198tadH639941; Fri, 9 Feb 2024 09:55:36 +0100 Date: Fri, 9 Feb 2024 09:55:36 +0100 From: Jakub Jelinek To: Richard Biener Cc: gcc-patches@gcc.gnu.org Subject: [PATCH] lower-bitint: Attempt not to emit always true conditions in handle_cast [PR113774] Message-ID: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.8 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, 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 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Jakub Jelinek Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Hi! The following patch is the optimization part of PR113774, where in handle_cast we emit some conditionals which are always true and presumably VRP would figure that out later and clean it up, except that instead thread1 is invoked and threads everything through the conditions, so we end up with really ugly code which is hard to be cleaned up later and then run into PR113831 VN bug and miscompile stuff. handle_cast computes low and high as limb indexes, where idx < low doesn't need any special treatment, just uses the operand's limb, idx >= high cases all the bits in the limb are an extension (so, for unsigned widening cast all those bits are 0, for signed widening cast all those bits are equal to the in earlier code computed sign mask, narrowing cast don't trigger this code) and then the idx == low && idx < high case if it exists need special treatment (some bits are copied, others extended, or all bits are copied but sign mask needs to be computed). The code already attempted to optimize away some unneeded casts, in the first hunk below e.g. for the case like 257 -> 321 bit extension, where low is 4 and high 5 and we use a loop handling the first 4 limbs (2 iterations) with m_upwards_2limb 4 - no special handling is needed in the loop, and the special handling is done on the first limb after the loop and then the last limb after the loop gets the extension only, or in the second hunk where can emit a single comparison instead of 2 e.g. for the low == high case - that must be a zero extension from multiple of limb bits, say 192 -> 328, or for the case where we know the idx == low case happens in the other limb processed in the loop, not the current one. But the testcase shows further cases where we always know some of the comparisons can be folded to true/false, in particular there is 255 -> 257 bit zero extension, so low 3, high 4, m_upwards_2limb 4. The loop handles 2 limbs at the time and for the first limb we were emitting idx < low ? operand[idx] : 0; but because idx goes from 0 with step 2 2 iterations, idx < 3 is always true, so we can just emit operand[idx]. This is handled in the first hunk. In addition to fixing it (that is the " - m_first" part in there) I've rewritten it using low to make it more readable. Similarly, in the other limb we were emitting idx + 1 <= low ? (idx + 1 == low ? operand[idx] & 0x7ff....ff : operand[idx]) : 0 but idx + 1 <= 3 is always true in the loop, so all we should emit is idx + 1 == low ? operand[idx] & 0x7ff....ff : operand[idx], Unfortunately for the latter, when single_comparison is true, we emit just one comparison, but the code which fills the branches will fill it with the operand[idx] and 0 cases (for zero extension, for sign extension similarly), not the operand[idx] (aka copy) and operand[idx] & 0x7ff....ff (aka most significant limb of the narrower precision) cases. Instead of making the code less readable by using single_comparison for that and handling it in the code later differently I've chosen to just emit a condition which will be always true and let cfg cleanup clean it up. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2024-02-09 Jakub Jelinek PR tree-optimization/113774 * gimple-lower-bitint.cc (bitint_large_huge::handle_cast): Don't emit any comparison if m_first and low + 1 is equal to m_upwards_2limb, simplify condition for that. If not single_comparison, not m_first and we can prove that the idx <= low comparison will be always true, emit instead of idx <= low comparison low <= low such that cfg cleanup will optimize it at the end of the pass. * gcc.dg/torture/bitint-57.c: New test. Jakub --- gcc/gimple-lower-bitint.cc.jj 2024-02-08 12:49:40.435313811 +0100 +++ gcc/gimple-lower-bitint.cc 2024-02-08 14:33:36.033220098 +0100 @@ -1350,9 +1350,7 @@ bitint_large_huge::handle_cast (tree lhs if (!tree_fits_uhwi_p (idx)) { if (m_upwards_2limb - && (m_upwards_2limb * limb_prec - <= ((unsigned) TYPE_PRECISION (rhs_type) - - !TYPE_UNSIGNED (rhs_type)))) + && low >= m_upwards_2limb - m_first) { rhs1 = handle_operand (rhs1, idx); if (m_first) @@ -1363,8 +1361,21 @@ bitint_large_huge::handle_cast (tree lhs } bool single_comparison = low == high || (m_upwards_2limb && (low & 1) == m_first); + tree idxc = idx; + if (!single_comparison + && m_upwards_2limb + && !m_first + && low + 1 == m_upwards_2limb) + /* In this case we know that idx <= low always, + so effectively we just needs a single comparison, + idx < low or idx == low, but we'd need to emit different + code for the 2 branches than single_comparison normally + emits. So, instead of special-casing that, emit a + low <= low comparison which cfg cleanup will clean up + at the end of the pass. */ + idxc = size_int (low); g = gimple_build_cond (single_comparison ? LT_EXPR : LE_EXPR, - idx, size_int (low), NULL_TREE, NULL_TREE); + idxc, size_int (low), NULL_TREE, NULL_TREE); edge edge_true_true, edge_true_false, edge_false; if_then_if_then_else (g, (single_comparison ? NULL : gimple_build_cond (EQ_EXPR, idx, --- gcc/testsuite/gcc.dg/torture/bitint-57.c.jj 2024-02-08 14:45:35.033303393 +0100 +++ gcc/testsuite/gcc.dg/torture/bitint-57.c 2024-02-07 18:50:10.759168537 +0100 @@ -0,0 +1,32 @@ +/* PR tree-optimization/113774 */ +/* { dg-do run { target bitint } } */ +/* { dg-options "-std=c23 -pedantic-errors" } */ +/* { dg-skip-if "" { ! run_expensive_tests } { "*" } { "-O0" "-O2" } } */ +/* { dg-skip-if "" { ! run_expensive_tests } { "-flto" } { "" } } */ + +#if __BITINT_MAXWIDTH__ >= 512 +unsigned _BitInt(512) u; +unsigned _BitInt(512) v; + +void +foo (unsigned _BitInt(255) a, unsigned _BitInt(257) b, unsigned _BitInt(512) *r) +{ + b += v; + b |= a - b; + unsigned _BitInt(512) c = b * 6; + unsigned _BitInt(512) h = c >> u; + *r = h; +} +#endif + +int +main () +{ +#if __BITINT_MAXWIDTH__ >= 512 + unsigned _BitInt(512) x; + foo (0x10000000000000000wb, 0x10000000000000001wb, &x); + if (x != 0x1fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffawb) + __builtin_abort (); +#endif + return 0; +}