From patchwork Mon Feb 25 09:07:10 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Botcazou X-Patchwork-Id: 1047633 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-496965-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=adacore.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="YXpMJri6"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 447GNJ28CTz9s70 for ; Mon, 25 Feb 2019 20:09:15 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:subject:date:message-id:mime-version:content-type :content-transfer-encoding; q=dns; s=default; b=K4NsLNpInWfnM0lH atGEd7rFrT5xL3zs2VcUbhHLX2w6J7mSTHufboV8dWEEO5CPLa0vg0d5bhidIyou JmlGDrlsSCXC5Xs4ToL9VuMp13d51SvRjyPYdxnhNgtLk7jqen9SSUtnI4tgqodM 7Cipu0WihayDp5Cc3h31QZ62AxI= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:subject:date:message-id:mime-version:content-type :content-transfer-encoding; s=default; bh=nw0Gd+9uzsMlU9QcTJZPcF K6PRc=; b=YXpMJri6INpRb5bhBHJoWJ2A51XwZX7w54VyFOUShMqxjTX124jRCn 9Dk6OS4ehfkL37TPnkEYWRl+HOwzRyjO89nFOGpKhuJYW4DRHE3Y68vjvdq3keY0 5au+3agzMhd2Dron85ztDDjDkTRTJmfqWz6rkAREJayE2va3xN00o= Received: (qmail 109054 invoked by alias); 25 Feb 2019 09:09:07 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 109018 invoked by uid 89); 25 Feb 2019 09:09:06 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-10.4 required=5.0 tests=BAYES_00, GIT_PATCH_2, GIT_PATCH_3, KAM_TRUTHINESS, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=folder, forces, wrapped, match.pd X-HELO: smtp.eu.adacore.com Received: from mel.act-europe.fr (HELO smtp.eu.adacore.com) (194.98.77.210) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 25 Feb 2019 09:09:04 +0000 Received: from localhost (localhost [127.0.0.1]) by filtered-smtp.eu.adacore.com (Postfix) with ESMTP id 0D46F8139B for ; Mon, 25 Feb 2019 10:09:02 +0100 (CET) Received: from smtp.eu.adacore.com ([127.0.0.1]) by localhost (smtp.eu.adacore.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ebD9rqOXrMgr for ; Mon, 25 Feb 2019 10:09:01 +0100 (CET) Received: from polaris.localnet (bon31-6-88-161-99-133.fbx.proxad.net [88.161.99.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.eu.adacore.com (Postfix) with ESMTPSA id B83E88138E for ; Mon, 25 Feb 2019 10:09:01 +0100 (CET) From: Eric Botcazou To: gcc-patches@gcc.gnu.org Subject: [patch] Fix wrong code for boolean negation in condition at -O Date: Mon, 25 Feb 2019 10:07:10 +0100 Message-ID: <1939418.1pZo4JS8vg@polaris> MIME-Version: 1.0 Hi, this is a regression present on the mainline and 8 branch, introduced by the new code in edge_info::derive_equivalences dealing with BIT_AND_EXPR for SSA names with boolean range: /* If either operand has a boolean range, then we know its value must be one, otherwise we just know it is nonzero. The former is clearly useful, I haven't seen cases where the latter is helpful yet. */ if (TREE_CODE (rhs1) == SSA_NAME) { if (ssa_name_has_boolean_range (rhs1)) { value = build_one_cst (TREE_TYPE (rhs1)); derive_equivalences (rhs1, value, recursion_limit - 1); } } if (TREE_CODE (rhs2) == SSA_NAME) { if (ssa_name_has_boolean_range (rhs2)) { value = build_one_cst (TREE_TYPE (rhs2)); derive_equivalences (rhs2, value, recursion_limit - 1); } } and visible on the attached Ada testcase at -O1 or above. The sequence of events is as follows: for boolean types with precision > 1 (the normal boolean types in Ada), the gimplifier turns a TRUTH_NOT_EXPR into a BIT_XOR_EXPR with 1 in order to preserve the 0-or-1-value invariant: /* The parsers are careful to generate TRUTH_NOT_EXPR only with operands that are always zero or one. We do not fold here but handle the only interesting case manually, as fold may re-introduce the TRUTH_NOT_EXPR. */ *expr_p = gimple_boolify (*expr_p); if (TYPE_PRECISION (TREE_TYPE (*expr_p)) == 1) *expr_p = build1_loc (input_location, BIT_NOT_EXPR, TREE_TYPE (*expr_p), TREE_OPERAND (*expr_p, 0)); else *expr_p = build2_loc (input_location, BIT_XOR_EXPR, TREE_TYPE (*expr_p), TREE_OPERAND (*expr_p, 0), build_int_cst (TREE_TYPE (*expr_p), 1)); Now this TRUTH_NOT_EXPR is part of a conjunction which has been turned into a BIT_AND_EXPR by the folder, so this gives BIT_AND_EXPR >. After some optimization passes, the second operand of the BIT_AND_EXPR is also folded into 1 and, consequently, the following match.pd pattern kicks in: /* Fold (X & Y) ^ Y and (X ^ Y) & Y as ~X & Y. */ (for opo (bit_and bit_xor) opi (bit_xor bit_and) (simplify (opo:c (opi:c @0 @1) @1) (bit_and (bit_not @0) @1))) and yields BIT_AND_EXPR . This is still correct, in the sense that the 0-or-1-value invariant is preserved. Then the new code in edge_info::derive_equivalences above deduces from this that the BIT_NOT_EXPR has value 1 on one of the edges. But the same function also handles the BIT_NOT_EXPR itself and further deduces that its operand has value ~1 or 254 (the precision of boolean types is 8) on this edge, which breaks the 0-or-1-value invariant and leads to wrong code downstream. Given the new code for BIT_AND_EXPR in edge_info::derive_equivalences for boolean types, I think that the same special treatment must be added for boolean types in the BIT_NOT_EXPR case to preserve the 0-or-1-value invariant. Bootstrapped/regtested on x86_64-suse-linux, OK for mainline and 8 branch? 2019-02-25 Eric Botcazou * tree-ssa-dom.c (edge_info::derive_equivalences) : Fix and move around comment. : Likewise. : Add specific handling for boolean types. 2019-02-25 Eric Botcazou * gnat.dg/opt77.adb: New test. * gnat.dg/opt77_pkg.ad[sb]: Likewise. Index: tree-ssa-dom.c =================================================================== --- tree-ssa-dom.c (revision 268994) +++ tree-ssa-dom.c (working copy) @@ -170,11 +170,10 @@ edge_info::derive_equivalences (tree nam gimple *def_stmt = SSA_NAME_DEF_STMT (name); if (is_gimple_assign (def_stmt)) { - /* We know the result of DEF_STMT was zero. See if that allows - us to deduce anything about the SSA_NAMEs used on the RHS. */ enum tree_code code = gimple_assign_rhs_code (def_stmt); switch (code) { + /* If the result of an OR is zero, then its operands are, too. */ case BIT_IOR_EXPR: if (integer_zerop (value)) { @@ -188,8 +187,7 @@ edge_info::derive_equivalences (tree nam } break; - /* We know the result of DEF_STMT was one. See if that allows - us to deduce anything about the SSA_NAMEs used on the RHS. */ + /* If the result of an AND is nonzero, then its operands are, too. */ case BIT_AND_EXPR: if (!integer_zerop (value)) { @@ -296,7 +294,6 @@ edge_info::derive_equivalences (tree nam break; } - case EQ_EXPR: case NE_EXPR: { @@ -336,7 +333,28 @@ edge_info::derive_equivalences (tree nam case NEGATE_EXPR: { tree rhs = gimple_assign_rhs1 (def_stmt); - tree res = fold_build1 (code, TREE_TYPE (rhs), value); + tree res; + /* If this is a NOT and the operand has a boolean range, then we + know its value must be zero or one. We are not supposed to + have a BIT_NOT_EXPR for boolean types with precision > 1 in + the general case, see e.g. the handling of TRUTH_NOT_EXPR in + the gimplifier, but it can be generated by match.pd out of + a BIT_XOR_EXPR wrapped in a BIT_AND_EXPR. Now the handling + of BIT_AND_EXPR above already forces a specific semantics for + boolean types with precision > 1 so we must do the same here, + otherwise we could change the semantics of TRUTH_NOT_EXPR for + boolean types with precision > 1. */ + if (code == BIT_NOT_EXPR + && TREE_CODE (rhs) == SSA_NAME + && ssa_name_has_boolean_range (rhs)) + { + if (integer_zerop (value)) + res = build_one_cst (TREE_TYPE (rhs)); + else + res = build_zero_cst (TREE_TYPE (rhs)); + } + else + res = fold_build1 (code, TREE_TYPE (rhs), value); derive_equivalences (rhs, res, recursion_limit - 1); break; }