From patchwork Tue Mar 28 13:19:33 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Andrew MacLeod X-Patchwork-Id: 1762329 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha256 header.s=default header.b=FBGXLE2k; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4Pm9Jz30Txz1yYS for ; Wed, 29 Mar 2023 00:20:01 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id CE2083854806 for ; Tue, 28 Mar 2023 13:19:59 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org CE2083854806 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1680009599; bh=6VMvy1sSfhoaUUbmThQuBU+RWmSV2aTRhz+4HEuWT4c=; h=Date:Subject:To:Cc:References:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=FBGXLE2k7IYqOwRvHE7S0KDsGNF0S2DvevDDOJgecWD6mj/srETW+spGmrtwVfTeA 0DkHCrXCOugrXR5yWKfKflja3Yh1+BZ9k1NIFX0Fuk20X9KkgOeRDCfOf46lyeWgij j+aF6YIhuo0644jsdSJtQBEpH/BDBNtBd1c13kpM= 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 164123858C62 for ; Tue, 28 Mar 2023 13:19:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 164123858C62 Received: from mail-qt1-f200.google.com (mail-qt1-f200.google.com [209.85.160.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-621-Qf4mjvdiMreHoCndYoOGMQ-1; Tue, 28 Mar 2023 09:19:36 -0400 X-MC-Unique: Qf4mjvdiMreHoCndYoOGMQ-1 Received: by mail-qt1-f200.google.com with SMTP id x5-20020a05622a000500b003e259c363f9so8102816qtw.22 for ; Tue, 28 Mar 2023 06:19:36 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680009575; h=in-reply-to:from:references:cc:to:content-language:subject :user-agent:mime-version:date:message-id:x-gm-message-state:from:to :cc:subject:date:message-id:reply-to; bh=C6Qhd8IpYqcMe0OyihDozgHG3PgvEcNDtlGWBO6+UvM=; b=0U9OQoCKC/LNlByqm7NqY4MUKBjjc1QP6X4ucElWNtZRxPc9MjlK+vmIJkEqaX5oN5 Kl3gKvsaVDfXjyPydHtvktHSTv2VQMOJAVN8KWW3E0z6DEdMg6TR38yzWnyMpIEtezIw dxAOwsft/B+5NTdQJl0MZpfY0/HwFcGDBZ3nX0Cw+aiylBpR/pEHRfvpkkhE4gPhCtYP xR4b9N3K3ZNN7KgJcEFK4+ux6VqLqt27PRXEyNMRQzKUUo/gVjHwbofx8wwXqrezVm68 bBSpYRyBQhy//O8CvmXVwvqEg1jkg1/Tab26mllPSA6HtjykGEumhfQIMYdLPeTqAJIT AqJw== X-Gm-Message-State: AO0yUKWcSWIzWHCxUlrm26catWXkgGJZQ12R1y91NtIbZxM7qnTHat18 ukldqufiLtw0H+WxesVXI9tzMyfq2HYxmURe/Csv0Uij/psl0juRnFKvNfs4GnhIVzke2sl0ilg 1FdZYZ5ErhpjeSE2WBw== X-Received: by 2002:ac8:5dcb:0:b0:3e4:6329:448a with SMTP id e11-20020ac85dcb000000b003e46329448amr25688662qtx.65.1680009575704; Tue, 28 Mar 2023 06:19:35 -0700 (PDT) X-Google-Smtp-Source: AK7set/IIauTe7i8YBRmMddkGbFzsiJYEvE2+YKUiCmo+GcokUAbNAI0TbIa8uKAObGSmhMNXA5UfQ== X-Received: by 2002:ac8:5dcb:0:b0:3e4:6329:448a with SMTP id e11-20020ac85dcb000000b003e46329448amr25688631qtx.65.1680009575375; Tue, 28 Mar 2023 06:19:35 -0700 (PDT) Received: from ?IPV6:2607:fea8:a263:f600::759b? ([2607:fea8:a263:f600::759b]) by smtp.gmail.com with ESMTPSA id d10-20020ac8668a000000b003e39106bdb2sm5074104qtp.31.2023.03.28.06.19.34 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 28 Mar 2023 06:19:34 -0700 (PDT) Message-ID: Date: Tue, 28 Mar 2023 09:19:33 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 Subject: [PATCH] PR tree-optimization/109274 -Fix compute_operand when op1 == op2 symbolically. To: Jakub Jelinek , Aldy Hernandez Cc: gcc-patches , Richard Biener References: <2e0b9177-f8ce-d55a-d6bb-71eb89a9700d@redhat.com> <86a51d9f-209b-33c4-053f-530210266c4c@redhat.com> In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, 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: Andrew MacLeod via Gcc-patches From: Andrew MacLeod Reply-To: Andrew MacLeod Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org Sender: "Gcc-patches" On 3/24/23 12:36, Jakub Jelinek wrote: > On Fri, Mar 24, 2023 at 11:52:30AM -0400, Andrew MacLeod wrote: >> Thanks.. Ive incorporated it into my commit  too. > Note, both my earlier version of the patch and your patch regress: > FAIL: gcc.dg/tree-ssa/vrp-float-3a.c scan-tree-dump-not evrp "link_error" > FAIL: gcc.dg/tree-ssa/vrp-float-4a.c scan-tree-dump-not evrp "link_error" > > Jakub > OK, that was fun. I commented in the PR, but the root issue is the way I was trying to communicate symbolic equivalence on operands to the compute_operand routines. [1, 1 ]= a_1 != a_1 I was creating a relation record between op1 and op2 indicating they were equivalent.    THis record then gets passed up the gori call chain. Reality is that this is not a true equivalence... without looking a the ranges, we dont know that that is true for general application, and and furthermore, when applied to something like a1 != a1, you can see the problem... Once I corrected the value_relation record to create records with the same operand, things went south. What we really need is to locally identify when op1 and op2 are the same symbol, and if there is no other information, pass it locally on that one statement  to the range-op handler. Then, once we have processed the statement, we invoke the handler for that statement to cvreate a record which is passed up the chain. so for:     a_1 = b_4 + 1.0     [1, 1] = (a_1 != a_1) compute_operand_1 starts with no relation record, recognizes symbolically op1 and op2 are the same, and passes EQ_EXPR locally as the op1_op2 relation to the handler for NE_EXPR.   That handler utilizes the range of op2 to detemine whether != is true or not based on knowledge that op1 and op2 are the same value.     (for integer always false, for float, takes a look at NAN) and produces a result. Before invoking compute_operand to calculate b_4 with the result of a_1,   handler.op1_op2_relation (lhs); is invoked to determine if there is a relation generated by the statement, which will generate  (a_1 != a_1), and pass that to compute operand for use in evaluating b_4. Bootstraps on x86_64-pc-linux-gnu with no regressions.  OK for trunk? Andrew commit 97f9c24fe471b9701cf3ba89901e146a9c52f3ad Author: Andrew MacLeod Date: Fri Mar 24 11:21:20 2023 -0400 Fix compute_operand when op1 == op2 symbolically. First, class value_relation should not sanitize records. just create what is asked. Second., if there is not a relation record, compute_operand was creating one for op1 == op2 if op1 and op2 were the same symbol. This is not the correct way to communicate the information, as that record will continue to be passed along the GORI unwind chain. Instead, simply pass that information locally to the opX_range routine for only the current statement. PR tree-optimization/109265 PR tree-optimization/109274 gcc/ * gimple-range-gori.cc (gori_compute::compute_operand_range): Do not create a relation record is op1 and op2 are the same symbol. (gori_compute::compute_operand1_range): Pass op1 == op2 to the handler for this stmt, but create a new record only if this statement generates a relation based on the ranges. (gori_compute::compute_operand2_range): Ditto. * value-relation.h (value_relation::set_relation): Always create the record that is requested. gcc/testsuite/ * gcc.dg/pr109274.c: New. * gfortran.dg/pr109265.f90: New. diff --git a/gcc/gimple-range-gori.cc b/gcc/gimple-range-gori.cc index 3c0044881fb..b9c3ba1a314 100644 --- a/gcc/gimple-range-gori.cc +++ b/gcc/gimple-range-gori.cc @@ -623,21 +623,6 @@ gori_compute::compute_operand_range (vrange &r, gimple *stmt, tree op1 = gimple_range_ssa_p (handler.operand1 ()); tree op2 = gimple_range_ssa_p (handler.operand2 ()); - // If there is a relation, use it instead of any passed in. This will allow - // multiple relations to be processed in compound logicals. - if (op1 && op2) - { - relation_kind k = handler.op1_op2_relation (lhs); - // If there is no relation, and op1 == op2, create a relation. - if (!vrel_ptr && k == VREL_VARYING && op1 == op2) - k = VREL_EQ; - if (k != VREL_VARYING) - { - vrel.set_relation (k, op1, op2); - vrel_ptr = &vrel; - } - } - // Handle end of lookup first. if (op1 == name) return compute_operand1_range (r, handler, lhs, name, src, vrel_ptr); @@ -1093,6 +1078,7 @@ gori_compute::compute_operand1_range (vrange &r, const vrange &lhs, tree name, fur_source &src, value_relation *rel) { + value_relation local_rel; gimple *stmt = handler.stmt (); tree op1 = handler.operand1 (); tree op2 = handler.operand2 (); @@ -1101,6 +1087,7 @@ gori_compute::compute_operand1_range (vrange &r, relation_trio trio; if (rel) trio = rel->create_trio (lhs_name, op1, op2); + relation_kind op_op = trio.op1_op2 (); Value_Range op1_range (TREE_TYPE (op1)); Value_Range tmp (TREE_TYPE (op1)); @@ -1113,10 +1100,26 @@ gori_compute::compute_operand1_range (vrange &r, if (op2) { src.get_operand (op2_range, op2); - relation_kind op_op = trio.op1_op2 (); + + // If there is a relation betwen op1 and op2, use it instead. + // This allows multiple relations to be processed in compound logicals. + if (gimple_range_ssa_p (op1) && gimple_range_ssa_p (op2)) + { + relation_kind k = handler.op1_op2_relation (lhs); + if (k != VREL_VARYING) + { + op_op = k; + local_rel.set_relation (op_op, op1, op2); + rel = &local_rel; + } + } + if (op_op != VREL_VARYING) refine_using_relation (op1, op1_range, op2, op2_range, src, op_op); + // If op1 == op2, create a new trio for just this call. + if (op1 == op2 && gimple_range_ssa_p (op1)) + trio = relation_trio (trio.lhs_op1 (), trio.lhs_op2 (), VREL_EQ); if (!handler.calc_op1 (tmp, lhs, op2_range, trio)) return false; } @@ -1185,6 +1188,7 @@ gori_compute::compute_operand2_range (vrange &r, const vrange &lhs, tree name, fur_source &src, value_relation *rel) { + value_relation local_rel; gimple *stmt = handler.stmt (); tree op1 = handler.operand1 (); tree op2 = handler.operand2 (); @@ -1201,9 +1205,26 @@ gori_compute::compute_operand2_range (vrange &r, if (rel) trio = rel->create_trio (lhs_name, op1, op2); relation_kind op_op = trio.op1_op2 (); + + // If there is a relation betwen op1 and op2, use it instead. + // This allows multiple relations to be processed in compound logicals. + if (gimple_range_ssa_p (op1) && gimple_range_ssa_p (op2)) + { + relation_kind k = handler.op1_op2_relation (lhs); + if (k != VREL_VARYING) + { + op_op = k; + local_rel.set_relation (op_op, op1, op2); + rel = &local_rel; + } + } + if (op_op != VREL_VARYING) refine_using_relation (op1, op1_range, op2, op2_range, src, op_op); + // If op1 == op2, create a new trio for this stmt. + if (op1 == op2 && gimple_range_ssa_p (op1)) + trio = relation_trio (trio.lhs_op1 (), trio.lhs_op2 (), VREL_EQ); // Intersect with range for op2 based on lhs and op1. if (!handler.calc_op2 (tmp, lhs, op1_range, trio)) return false; diff --git a/gcc/testsuite/gcc.dg/pr109274.c b/gcc/testsuite/gcc.dg/pr109274.c new file mode 100644 index 00000000000..5dbc0232f8e --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr109274.c @@ -0,0 +1,16 @@ +/* PR tree-optimization/109274 */ +/* { dg-do compile } */ +/* { dg-options "-O2 " } */ + +float a, b, c; +int d; +float bar (void); + +void +foo (void) +{ + a = 0 * -(2.0f * c); + d = a != a ? 0 : bar (); + b = c; +} + diff --git a/gcc/testsuite/gfortran.dg/pr109265.f90 b/gcc/testsuite/gfortran.dg/pr109265.f90 new file mode 100644 index 00000000000..0d7124c7741 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr109265.f90 @@ -0,0 +1,39 @@ +! PR tree-optimization/109265 +! { dg-do compile } +! { dg-options "-O3 -w" } + +module pr109265 + integer, parameter :: r8 = selected_real_kind (12) +contains + subroutine foo (b, c, d, e, f) + implicit none + logical :: b + real (kind = r8) :: c, d, e, f, i + if (b) then + c = bar (c * d, e) + i = bar (f, c) + call baz (i) + call baz (-i) + end if + end subroutine foo + function bar (a, b) + implicit none + real (kind = r8) :: bar + real (kind = r8) :: a, b + bar = a + b + end function bar + subroutine baz (b) + implicit none + real (kind = r8) :: b, d, e, f, g, h, i + d = b + i = 0 + e = d + f = d + g = d + 10 continue + if ((e.eq.d) .and. (f.eq.d) .and. (g.eq.d) .and. (h.eq.d)) then + h = i + goto 10 + end if + end subroutine baz +end module pr109265 diff --git a/gcc/value-relation.h b/gcc/value-relation.h index 36a75862cc7..3177ecb1ad0 100644 --- a/gcc/value-relation.h +++ b/gcc/value-relation.h @@ -445,13 +445,6 @@ value_relation::set_relation (relation_kind r, tree n1, tree n2) { gcc_checking_assert (TREE_CODE (n1) == SSA_NAME && TREE_CODE (n2) == SSA_NAME); - if (n1 == n2 && r != VREL_EQ) - { - related = VREL_VARYING; - name1 = NULL_TREE; - name2 = NULL_TREE; - return; - } related = r; name1 = n1; name2 = n2;