From patchwork Tue Mar 26 09:58:44 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 87659 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 338683858435 for ; Tue, 26 Mar 2024 09:59:53 +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.133.124]) by sourceware.org (Postfix) with ESMTPS id ABF1E3858C56 for ; Tue, 26 Mar 2024 09:58:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org ABF1E3858C56 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 ABF1E3858C56 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1711447138; cv=none; b=waD50nXpSbOIpu5aJwaP7Jzhayg1XaXUA9axOGx9pcGrp+UlKtB6aLD+7EHuY5fg5cVyCazdt2iCAFsTeVY5XCyWb3AVt3l77UNBdHBILeuwJXvoevwQSaUufdlt39q0r/YZVIpOgSAlu68EBCyvvlxd1pL7rqsjnGLvQknPvE4= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1711447138; c=relaxed/simple; bh=fQdWOMh8t8jZ1Au3nl41G1UXTFcqgR+9QQ0EiQZUchg=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=MSeFMzv/W/2bR3BhVxBMZeMCFH0ekPZmB3pQT5BV1WlhNB9kv/fU1d7ffCe0W78BUUNwiX7LaoKKuHDxzPitr3STMFOgae6kytmST0tIMfQfzv5X5E6tErsY8jSh+iLU/0lCA4HpCX/FvcH9TX9Gr5BqyQD81IKgEDIePGYkjB0= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1711447136; 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=KqzSp0UaL5tdPvZTeUMdim2fpW+FgAsL71pW6TXqFIk=; b=LaMoN8HZ5UXPaTCE5Hh3Q/KjzP+LxN/qNDU4IPTPTvNQQOIZIl8TQzzsqNMYBeD1ct9IUj LJGxVgIQZj68vUkSj3HntuNLsIYOndVqGHwGXcVfsOY/GYKguWlM3NWBp37rewrjRVpOO5 27dHXNjdJxEysqYqdJYhQYqmMPGbpF8= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-302-wRxSbO6hMLqpSDuXuUYFRA-1; Tue, 26 Mar 2024 05:58:52 -0400 X-MC-Unique: wRxSbO6hMLqpSDuXuUYFRA-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 19F7929AA394; Tue, 26 Mar 2024 09:58:52 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.192.57]) by smtp.corp.redhat.com (Postfix) with ESMTPS id CFD3EC017A1; Tue, 26 Mar 2024 09:58:51 +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 42Q9wjpT2917785 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Tue, 26 Mar 2024 10:58:45 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 42Q9wiHA2917784; Tue, 26 Mar 2024 10:58:44 +0100 Date: Tue, 26 Mar 2024 10:58:44 +0100 From: Jakub Jelinek To: Richard Biener Cc: gcc-patches@gcc.gnu.org Subject: [PATCH] fold-const: Punt on MULT_EXPR in extract_muldiv MIN/MAX_EXPR case [PR111151] 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.7 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_H4, RCVD_IN_MSPIKE_WL, 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.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! As I've tried to explain in the comments, the extract_muldiv_1 MIN/MAX_EXPR optimization is wrong for code == MULT_EXPR. If the multiplication is done in unsigned type or in signed type with -fwrapv, it is fairly obvious that max (a, b) * c in many cases isn't equivalent to max (a * c, b * c) (or min if c is negative) due to overflows, but even for signed with undefined overflow, the optimization could turn something without UB in it (where say a * c invokes UB, but max (or min) picks the other operand where b * c doesn't). As for division/modulo, I think it is in most cases safe, except if the problematic INT_MIN / -1 case could be triggered, but we can just punt for MAX_EXPR because for MIN_EXPR if one operand is INT_MIN, we'd pick that operand already. It is just for completeness, match.pd already has an optimization which turns x / -1 into -x, so the division by zero is mostly theoretical. That is also why in the testcase the i case isn't actually miscompiled without the patch, while the c and f cases are. Bootstrapped/regtested on x86_64-linux and i686-linux, additionally bootstrapped/regtested on x86_64-linux with statistics gathering when the patch changes behavior and it is solely on the new testcase and nothing else during the bootstrap/regtest. Ok for trunk? 2024-03-26 Jakub Jelinek PR middle-end/111151 * fold-const.cc (extract_muldiv_1) : Punt for MULT_EXPR altogether, or for MAX_EXPR if c is -1. * gcc.c-torture/execute/pr111151.c: New test. Jakub --- gcc/fold-const.cc.jj 2024-03-11 09:42:04.544588951 +0100 +++ gcc/fold-const.cc 2024-03-25 11:48:12.133625285 +0100 @@ -7104,6 +7104,27 @@ extract_muldiv_1 (tree t, tree c, enum t if (TYPE_UNSIGNED (ctype) != TYPE_UNSIGNED (type)) break; + /* Punt for multiplication altogether. + MAX (1U + INT_MAX, 1U) * 2U is not equivalent to + MAX ((1U + INT_MAX) * 2U, 1U * 2U), the former is + 0U, the latter is 2U. + MAX (INT_MIN / 2, 0) * -2 is not equivalent to + MIN (INT_MIN / 2 * -2, 0 * -2), the former is + well defined 0, the latter invokes UB. + MAX (INT_MIN / 2, 5) * 5 is not equivalent to + MAX (INT_MIN / 2 * 5, 5 * 5), the former is + well defined 25, the latter invokes UB. */ + if (code == MULT_EXPR) + break; + /* For division/modulo, punt on c being -1 for MAX, as + MAX (INT_MIN, 0) / -1 is not equivalent to + MIN (INT_MIN / -1, 0 / -1), the former is well defined + 0, the latter invokes UB (or for -fwrapv is INT_MIN). + MIN (INT_MIN, 0) / -1 already invokes UB, so the + transformation won't make it worse. */ + else if (tcode == MAX_EXPR && integer_minus_onep (c)) + break; + /* MIN (a, b) / 5 -> MIN (a / 5, b / 5) */ sub_strict_overflow_p = false; if ((t1 = extract_muldiv (op0, c, code, wide_type, --- gcc/testsuite/gcc.c-torture/execute/pr111151.c.jj 2024-03-25 11:50:27.199744988 +0100 +++ gcc/testsuite/gcc.c-torture/execute/pr111151.c 2024-03-26 10:41:51.003384032 +0100 @@ -0,0 +1,21 @@ +/* PR middle-end/111151 */ + +int +main () +{ + unsigned a = (1U + __INT_MAX__) / 2U; + unsigned b = 1U; + unsigned c = (a * 2U > b * 2U ? a * 2U : b * 2U) * 2U; + if (c != 0U) + __builtin_abort (); + int d = (-__INT_MAX__ - 1) / 2; + int e = 10; + int f = (d * 2 > e * 5 ? d * 2 : e * 5) * 6; + if (f != 120) + __builtin_abort (); + int g = (-__INT_MAX__ - 1) / 2; + int h = 0; + int i = (g * 2 > h * 5 ? g * 2 : h * 5) / -1; + if (i != 0) + __builtin_abort (); +}