From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x535.google.com (mail-ed1-x535.google.com [IPv6:2a00:1450:4864:20::535]) by sourceware.org (Postfix) with ESMTPS id 531A63858C55 for ; Sat, 1 Oct 2022 05:01:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 531A63858C55 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-ed1-x535.google.com with SMTP id y8so8323269edc.10 for ; Fri, 30 Sep 2022 22:01:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date; bh=eksqrBnkBf8+drVhmHggX8+/aBNk6Wy16c801yqY5u4=; b=j/v/emULuug4kLraKB2IHDlh3b5MW9xZBTifGbzoWizUgCAo7tsJ51kb1I7oV936ZW MunsAb2u9eauLoNZY+mNeupYFVSCPSitoeLZ1dDrLL6FtOIVqs+UzOuD9rSLLEkEmKcU f4ZVSSGyAWTHXZ2KvwxbqINSbua8I3eCzdYZycuxtAYFe1CtrXEarfmsQeWwi54zTRmj nY8SB14+N2qqBguVRKsVNU/sL/QV+45E9q0mymsyPdLv1iE6avndCzpog2iVPbKqKABL 55n794vnhR335loZkDLupA/SSOgpgdB10p0k1odFA/rA4HAexQg73tFKL679fnMbmsxl yNng== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date; bh=eksqrBnkBf8+drVhmHggX8+/aBNk6Wy16c801yqY5u4=; b=IldrUJ3tCZ7PoD4rtPVj+nDUjL1vtPnRNSyY5knAaIrjjAMRdG6Q1FGVLAZgd8quHQ FDYuLiYlKpbypNXWQHOC1aOtPslPPG91yvvd4zeoKcpK5v+HdEFyTZ3gahneli50IrLh eKSxrX9UPvTn82w2AGIhtWZzePdYA+kfmWk5SjeZ+84OVqGeVpHWs/Mbwp2ypseQiug+ cZIGVQEE/43GBk8h3eK4LoXaEI0QTJIhVUuUfX7dWA3DUrEIl0WHShiolvW+BYwqlLUO osFmm5f7ASw4gMYHz6ZjsOjFMQWFpShSZ+Dymm5qRZYyhBp3bpqJhRRCTn7X0IfCO3Rn 0Q9Q== X-Gm-Message-State: ACrzQf0of//kM8WXscl7GozDm8LjirXTAnfVzF88onIJ0gavC+tuAgsh nCdrAklbGmzXhSjflKGyGQM= X-Google-Smtp-Source: AMsMyM5xXTiumLFJO0tVwoDSPqUrmX22gYg4cThhIi1paSoSYjwTOOgTfkYQMkJuQalmrk5JfvS0gw== X-Received: by 2002:a05:6402:414f:b0:456:c2c1:23ec with SMTP id x15-20020a056402414f00b00456c2c123ecmr10593450eda.420.1664600509762; Fri, 30 Sep 2022 22:01:49 -0700 (PDT) Received: from nbbrfq ([2001:871:227:1d92:3b3f:9f87:ac65:9ebc]) by smtp.gmail.com with ESMTPSA id a7-20020a1709063e8700b00782fbb7f5f7sm2114111ejj.113.2022.09.30.22.01.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 30 Sep 2022 22:01:49 -0700 (PDT) Date: Sat, 1 Oct 2022 07:01:40 +0200 From: Bernhard Reutner-Fischer To: Andrew MacLeod via Gcc-patches Cc: rep.dot.nop@gmail.com, Andrew MacLeod Subject: Re: [PATCH] Refine ranges using relations in GORI. Message-ID: <20221001070140.235eee02@nbbrfq> In-Reply-To: References: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-10.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,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 List-Id: Hi Andrew! On Thu, 29 Sep 2022 18:36:53 -0400 Andrew MacLeod via Gcc-patches wrote: > diff --git a/gcc/gimple-range-gori.cc b/gcc/gimple-range-gori.cc > index 57a7e820749..b37d03cddda 100644 > --- a/gcc/gimple-range-gori.cc > +++ b/gcc/gimple-range-gori.cc > @@ -934,6 +934,115 @@ gori_compute::compute_logical_operands (vrange &true_range, vrange &false_range, > src.get_operand (false_range, name); > } > > + > +// This routine will try to refine the ranges of OP1 and OP2 given a relation > +// K between them. In order to perform this refinement, one of the operands > +// must be in the definition chain of the other. The use is refined using > +// op1/op2_range on the statement, and the defintion is then recalculated > +// using the relation. s/defintion/definition/g And i'd add a 'kind' before K: given a relation_kind K We've accumulated quite some "defintion" in the meantime. I think arm expresses it quite well: gcc/config/arm/unspecs.md:;; Unspec defintions. nvptx OTOH only supports weak defintions. Either way, maybe you can correct this typo in at least gimple-range-gori.cc and value-query.cc ? > + > +bool > +gori_compute::refine_using_relation (tree op1, vrange &op1_range, > + tree op2, vrange &op2_range, > + fur_source &src, relation_kind k) > +{ > + gcc_checking_assert (TREE_CODE (op1) == SSA_NAME); > + gcc_checking_assert (TREE_CODE (op2) == SSA_NAME); > + gcc_checking_assert (k != VREL_VARYING && k != VREL_UNDEFINED); > + > + bool change = false; > + bool op1_def_p = in_chain_p (op2, op1); > + if (!op1_def_p) > + if (!in_chain_p (op1, op2)) > + return false; > + > + tree def_op = op1_def_p ? op1 : op2; > + tree use_op = op1_def_p ? op2 : op1; > + > + if (!op1_def_p) > + k = relation_swap (k); > + > + // op1_def is true if we want to look up op1, otherwise we want op2. > + // if neither is the case, we returned in the above check. I think this comment should be moved up to before setting def_op and s/op1_def/op1_def_p/ And i hope this ends up as a single if block even if written like above :) > + > + gimple *def_stmt = SSA_NAME_DEF_STMT (def_op); > + gimple_range_op_handler op_handler (def_stmt); > + if (!op_handler) > + return false; > + tree def_op1 = op_handler.operand1 (); > + tree def_op2 = op_handler.operand2 (); > + // if the def isn't binary, the relation will not be useful. > + if (!def_op2) > + return false; > + > + // Determine if op2 is directly referenced as an operand. > + if (def_op1 == use_op) > + { > + // def_stmt has op1 in the 1st operand position. > + Value_Range other_op (TREE_TYPE (def_op2)); > + src.get_operand (other_op, def_op2); > + > + // Using op1_range as the LHS, and relation REL, evaluate op2. > + tree type = TREE_TYPE (def_op1); > + Value_Range new_result (type); > + if (!op_handler.op1_range (new_result, type, > + op1_def_p ? op1_range : op2_range, > + other_op, k)) > + return false; > + if (op1_def_p) > + { > + change |= op2_range.intersect (new_result); > + // Recalculate op2. > + if (op_handler.fold_range (new_result, type, op2_range, other_op)) > + { > + change |= op1_range.intersect (new_result); > + } > + } > + else > + { > + change |= op1_range.intersect (new_result); > + // Recalculate op1. > + if (op_handler.fold_range (new_result, type, op1_range, other_op)) > + { > + change |= op2_range.intersect (new_result); > + } > + } > + } > + else if (def_op2 == use_op) > + { > + // def_stmt has op1 in the 1st operand position. Maybe i'm confused by the swapping, but that's the 2nd operand position, isn't it? Maybe you forgot to adjust the comments in one of the blocks? thanks, > + Value_Range other_op (TREE_TYPE (def_op1)); > + src.get_operand (other_op, def_op1); > + > + // Using op1_range as the LHS, and relation REL, evaluate op2. > + tree type = TREE_TYPE (def_op2); > + Value_Range new_result (type); > + if (!op_handler.op2_range (new_result, type, > + op1_def_p ? op1_range : op2_range, > + other_op, k)) > + return false; > + if (op1_def_p) > + { > + change |= op2_range.intersect (new_result); > + // Recalculate op1. > + if (op_handler.fold_range (new_result, type, other_op, op2_range)) > + { > + change |= op1_range.intersect (new_result); > + } > + } > + else > + { > + change |= op1_range.intersect (new_result); > + // Recalculate op2. > + if (op_handler.fold_range (new_result, type, other_op, op1_range)) > + { > + change |= op2_range.intersect (new_result); > + } > + } > + } > + return change; > +} > + > // Calculate a range for NAME from the operand 1 position of STMT > // assuming the result of the statement is LHS. Return the range in > // R, or false if no range could be calculated. > @@ -947,6 +1056,7 @@ gori_compute::compute_operand1_range (vrange &r, > gimple *stmt = handler.stmt (); > tree op1 = handler.operand1 (); > tree op2 = handler.operand2 (); > + tree lhs_name = gimple_get_lhs (stmt); > > Value_Range op1_range (TREE_TYPE (op1)); > Value_Range tmp (TREE_TYPE (op1)); > @@ -959,7 +1069,21 @@ gori_compute::compute_operand1_range (vrange &r, > if (op2) > { > src.get_operand (op2_range, op2); > - if (!handler.calc_op1 (tmp, lhs, op2_range)) > + relation_kind k = VREL_VARYING; > + if (rel) > + { > + if (lhs_name == rel->op1 () && op1 == rel->op2 ()) > + k = rel->kind (); > + else if (lhs_name == rel->op2 () && op1 == rel->op1 ()) > + k = relation_swap (rel->kind ()); > + else if (op1 == rel->op1 () && op2 == rel->op2 ()) > + refine_using_relation (op1, op1_range, op2, op2_range, src, > + rel->kind ()); > + else if (op1 == rel->op2 () && op2 == rel->op1 ()) > + refine_using_relation (op1, op1_range, op2, op2_range, src, > + relation_swap (rel->kind ())); > + } > + if (!handler.calc_op1 (tmp, lhs, op2_range, k)) > return false; > } > else > @@ -967,7 +1091,7 @@ gori_compute::compute_operand1_range (vrange &r, > // We pass op1_range to the unary operation. Nomally it's a > // hidden range_for_type parameter, but sometimes having the > // actual range can result in better information. > - if (!handler.calc_op1 (tmp, lhs, op1_range)) > + if (!handler.calc_op1 (tmp, lhs, op1_range, VREL_VARYING)) > return false; > } > > @@ -1030,6 +1154,7 @@ gori_compute::compute_operand2_range (vrange &r, > gimple *stmt = handler.stmt (); > tree op1 = handler.operand1 (); > tree op2 = handler.operand2 (); > + tree lhs_name = gimple_get_lhs (stmt); > > Value_Range op1_range (TREE_TYPE (op1)); > Value_Range op2_range (TREE_TYPE (op2)); > @@ -1037,9 +1162,24 @@ gori_compute::compute_operand2_range (vrange &r, > > src.get_operand (op1_range, op1); > src.get_operand (op2_range, op2); > + relation_kind k = VREL_VARYING; > + if (rel) > + { > + if (lhs_name == rel->op1 () && op2 == rel->op2 ()) > + k = rel->kind (); > + else if (lhs_name == rel->op2 () && op2 == rel->op1 ()) > + k = relation_swap (rel->kind ()); > + else if (op1 == rel->op1 () && op2 == rel->op2 ()) > + refine_using_relation (op1, op1_range, op2, op2_range, src, > + rel->kind ()); > + else if (op1 == rel->op2 () && op2 == rel->op1 ()) > + refine_using_relation (op1, op1_range, op2, op2_range, src, > + relation_swap (rel->kind ())); > + } > + > > // Intersect with range for op2 based on lhs and op1. > - if (!handler.calc_op2 (tmp, lhs, op1_range)) > + if (!handler.calc_op2 (tmp, lhs, op1_range, k)) > return false; > > unsigned idx; > diff --git a/gcc/gimple-range-gori.h b/gcc/gimple-range-gori.h > index 1fff3e6255a..c7a32162a1b 100644 > --- a/gcc/gimple-range-gori.h > +++ b/gcc/gimple-range-gori.h > @@ -166,6 +166,9 @@ public: > bool has_edge_range_p (tree name, edge e); > void dump (FILE *f); > private: > + bool refine_using_relation (tree op1, vrange &op1_range, > + tree op2, vrange &op2_range, > + fur_source &src, relation_kind k); > bool may_recompute_p (tree name, edge e); > bool may_recompute_p (tree name, basic_block bb = NULL); > bool compute_operand_range (vrange &r, gimple *stmt, const vrange &lhs,