From patchwork Wed Feb 28 13:08:59 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Biener X-Patchwork-Id: 86516 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 05E8C3858404 for ; Wed, 28 Feb 2024 13:09:38 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.223.131]) by sourceware.org (Postfix) with ESMTPS id 0DC6F3858C62 for ; Wed, 28 Feb 2024 13:09:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 0DC6F3858C62 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 0DC6F3858C62 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=195.135.223.131 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709125745; cv=none; b=tPPTykrQm1GziZxE/g5AeMiS2n505anuvfzw+5NVId00whSHzlrTke0I3EvezdbPmBRU/JHcnalBtWbNfRkXhn9+lO0z8WM1zisGnsqRTCO8iL0dVh5W4ZEeFRg7sV78P+I5BWnF6uBzFKKQRssWiEDHDSruLNL9n3GtCmO62f4= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709125745; c=relaxed/simple; bh=J7K9KAlNdi78PHRNlrL9JvrsGzVATGcOE2c3HaJbCrs=; h=DKIM-Signature:DKIM-Signature:DKIM-Signature:DKIM-Signature:Date: From:To:Subject:MIME-Version; b=JxrFjb0l+/aW4dv8fj7MzLpspcaB6jQuT4s59lJKWcWMT+UBEAypvL3MV5F+n8Xs05JRiw6cIjuykqQ91uzNMDmvPXtnK009ZO3y4y0PI9WcIUrpLy3z62fejb9kVEiL9yhj4FO4BP7YcPvyArfCNpRIlgtwehMmSu/XuEfetIk= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from [10.168.4.150] (unknown [10.168.4.150]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id DF0401F460 for ; Wed, 28 Feb 2024 13:08:59 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1709125741; h=from:from:reply-to:date:date:to:to:cc:mime-version:mime-version: content-type:content-type; bh=VYFcFx42j2HJlZQQUD59WS8DGLVi5PBvLL7gbeFBJZc=; b=P5eIlsxByTZJ6YNjJz8PfwEfuJhKKegHKsZvQDFDV2OZJwOW3lHny2IyT8CZIF/UVlUlaw JxZc445kmywsgSYQWAZXeL0Slq43rjIuZQ3gefJtERfyeufE7kCXnSnkAwpv7EIpZZcTuK nMAU1BuRgkSCWtHOwowOKmMMEa0O/yI= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1709125741; h=from:from:reply-to:date:date:to:to:cc:mime-version:mime-version: content-type:content-type; bh=VYFcFx42j2HJlZQQUD59WS8DGLVi5PBvLL7gbeFBJZc=; b=HUWV3QP/qlnlYrTyBG8sN61Q+ROKmnJ8zllNMvDxLK1WbPhKlCvYZIhfG0q8rYfI/8E2y9 WHDhYfQmzb8QPbBA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1709125739; h=from:from:reply-to:date:date:to:to:cc:mime-version:mime-version: content-type:content-type; bh=VYFcFx42j2HJlZQQUD59WS8DGLVi5PBvLL7gbeFBJZc=; b=HHSMmn8xolDPDg6db9fBEhZFODB2jkTXrfcxoCW7kHJJ9oxf3DM0FSOweccqPLynxcr1BW Uj2qt5pbFB+WV9yJ2oz5AUt05FdDI+h4hnYNNtpzwOJ6qwrFP7RuD/z5sjs4MspJzB8vNj O4T7wsOC1iBgtyVuhcE+HX7iHJAFm58= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1709125739; h=from:from:reply-to:date:date:to:to:cc:mime-version:mime-version: content-type:content-type; bh=VYFcFx42j2HJlZQQUD59WS8DGLVi5PBvLL7gbeFBJZc=; b=rNpDwIAEVphqgM8Pa3H9i7a0k/xU1x7WGerJfZuvUb36hzvc5EIoZ7Cm8GFmPT3oC85DPS TL2ENlpCxv+9UiBg== Date: Wed, 28 Feb 2024 14:08:59 +0100 (CET) From: Richard Biener To: gcc-patches@gcc.gnu.org Subject: [PATCH 1/2] tree-optimization/114121 - wrong VN with context sensitive range info MIME-Version: 1.0 Authentication-Results: smtp-out2.suse.de; none X-Spam-Score: 2.19 X-Spamd-Result: default: False [2.19 / 50.00]; ARC_NA(0.00)[]; FROM_HAS_DN(0.00)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; NEURAL_HAM_LONG(-0.06)[-0.061]; MIME_GOOD(-0.10)[text/plain]; TO_DN_NONE(0.00)[]; RCPT_COUNT_ONE(0.00)[1]; MISSING_MID(2.50)[]; DKIM_SIGNED(0.00)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; DBL_BLOCKED_OPENRESOLVER(0.00)[tree-ssa-pre.cc:url,tree-ssa-sccvn.cc:url]; NEURAL_SPAM_SHORT(2.85)[0.952]; FUZZY_BLOCKED(0.00)[rspamd.com]; RCVD_COUNT_ZERO(0.00)[0]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; BAYES_HAM(-3.00)[100.00%] X-Spam-Status: No, score=-10.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, MISSING_MID, SPF_HELO_NONE, SPF_PASS, 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: , Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Message-Id: <20240228130938.05E8C3858404@sourceware.org> When VN ends up exploiting range-info specifying the ao_ref offset and max_size we have to make sure to reflect this in the hashtable entry for the recorded expression. The PR113831 fix handled the case where we can encode this in the operands themselves but this bug shows the issue is more widespread. So instead of altering the operands the following instead records this extra info that's possibly used, only throwing it away when the value-numbering didn't come up with a non-VARYING value which is an important detail to preserve CSE as opposed to constant folding which is where all cases currently known popped up. With this the original PR113831 fix can be reverted, see 2/2. Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed. PR tree-optimization/114121 * tree-ssa-sccvn.h (vn_reference_s::offset, vn_reference_s::max_size): New fields. (vn_reference_insert_pieces): Adjust prototype. * tree-ssa-pre.cc (phi_translate_1): Preserve offset/max_size. * tree-ssa-sccvn.cc (vn_reference_eq): Compare offset and size, allow using "don't know" state. (vn_walk_cb_data::finish): Pass along offset/max_size. (vn_reference_lookup_or_insert_for_pieces): Take offset and max_size as argument and use it. (vn_reference_lookup_3): Properly adjust offset and max_size according to the adjusted ao_ref. (vn_reference_lookup_pieces): Initialize offset and max_size. (vn_reference_lookup): Likewise. (vn_reference_lookup_call): Likewise. (vn_reference_insert): Likewise. (visit_reference_op_call): Likewise. (vn_reference_insert_pieces): Take offset and max_size as argument and use it. * gcc.dg/torture/pr114121.c: New testcase. --- gcc/testsuite/gcc.dg/torture/pr114121.c | 35 ++++++++++++++++ gcc/tree-ssa-pre.cc | 5 ++- gcc/tree-ssa-sccvn.cc | 55 ++++++++++++++++++++++--- gcc/tree-ssa-sccvn.h | 3 ++ 4 files changed, 91 insertions(+), 7 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/torture/pr114121.c diff --git a/gcc/testsuite/gcc.dg/torture/pr114121.c b/gcc/testsuite/gcc.dg/torture/pr114121.c new file mode 100644 index 00000000000..9a6ddf2957e --- /dev/null +++ b/gcc/testsuite/gcc.dg/torture/pr114121.c @@ -0,0 +1,35 @@ +/* { dg-do run { target bitint } } */ + +#if __BITINT_MAXWIDTH__ >= 256 +unsigned a, b, c, d, e; +unsigned _BitInt(256) f; + +__attribute__((noipa)) unsigned short +bswap16 (int t) +{ + return __builtin_bswap16 (t); +} + +void +foo (unsigned z, unsigned _BitInt(512) y, unsigned *r) +{ + unsigned t = __builtin_sub_overflow_p (0, y << 509, f); + z *= bswap16 (t); + d = __builtin_sub_overflow_p (c, 3, (unsigned _BitInt(512)) 0); + unsigned q = z + c + b; + unsigned short n = q >> (8 + a); + *r = b + e + n; +} +#endif + +int +main () +{ +#if __BITINT_MAXWIDTH__ >= 256 + unsigned x; + foo (8, 2, &x); + if (x != 8) + __builtin_abort (); +#endif + return 0; +} diff --git a/gcc/tree-ssa-pre.cc b/gcc/tree-ssa-pre.cc index d29214d04f8..75217f5cde1 100644 --- a/gcc/tree-ssa-pre.cc +++ b/gcc/tree-ssa-pre.cc @@ -1666,8 +1666,9 @@ phi_translate_1 (bitmap_set_t dest, if (!newoperands.exists ()) newoperands = operands.copy (); newref = vn_reference_insert_pieces (newvuse, ref->set, - ref->base_set, ref->type, - newoperands, + ref->base_set, + ref->offset, ref->max_size, + ref->type, newoperands, result, new_val_id); newoperands = vNULL; } diff --git a/gcc/tree-ssa-sccvn.cc b/gcc/tree-ssa-sccvn.cc index c20ad85c743..21123644a5a 100644 --- a/gcc/tree-ssa-sccvn.cc +++ b/gcc/tree-ssa-sccvn.cc @@ -438,7 +438,7 @@ static void init_vn_nary_op_from_pieces (vn_nary_op_t, unsigned int, enum tree_code, tree, tree *); static tree vn_lookup_simplify_result (gimple_match_op *); static vn_reference_t vn_reference_lookup_or_insert_for_pieces - (tree, alias_set_type, alias_set_type, tree, + (tree, alias_set_type, alias_set_type, poly_int64, poly_int64, tree, vec, tree); /* Return whether there is value numbering information for a given SSA name. */ @@ -748,6 +748,8 @@ vn_reference_compute_hash (const vn_reference_t vr1) vn_reference_op_compute_hash (vro, hstate); } } + /* Do not hash vr1->offset or vr1->max_size, we want to get collisions + to be able to identify compatible results. */ result = hstate.end (); /* ??? We would ICE later if we hash instead of adding that in. */ if (vr1->vuse) @@ -772,6 +774,16 @@ vn_reference_eq (const_vn_reference_t const vr1, const_vn_reference_t const vr2) if (vr1->vuse != vr2->vuse) return false; + /* The offset/max_size used for the ao_ref during lookup has to be + the same. */ + if (maybe_ne (vr1->offset, vr2->offset) + || maybe_ne (vr1->max_size, vr2->max_size)) + { + /* But nothing known in the prevailing entry is OK to be used. */ + if (maybe_ne (vr1->offset, 0) || known_size_p (vr1->max_size)) + return false; + } + /* If the operands are the same we are done. */ if (vr1->operands == vr2->operands) return true; @@ -2076,6 +2088,7 @@ vn_walk_cb_data::finish (alias_set_type set, alias_set_type base_set, tree val) vec &operands = saved_operands.exists () ? saved_operands : vr->operands; return vn_reference_lookup_or_insert_for_pieces (last_vuse, set, base_set, + vr->offset, vr->max_size, vr->type, operands, val); } @@ -2552,6 +2565,8 @@ static vn_reference_t vn_reference_lookup_or_insert_for_pieces (tree vuse, alias_set_type set, alias_set_type base_set, + poly_int64 offset, + poly_int64 max_size, tree type, vec operands, @@ -2565,15 +2580,18 @@ vn_reference_lookup_or_insert_for_pieces (tree vuse, vr1.type = type; vr1.set = set; vr1.base_set = base_set; + vr1.offset = offset; + vr1.max_size = max_size; vr1.hashcode = vn_reference_compute_hash (&vr1); if (vn_reference_lookup_1 (&vr1, &result)) return result; + if (TREE_CODE (value) == SSA_NAME) value_id = VN_INFO (value)->value_id; else value_id = get_or_alloc_constant_value_id (value); - return vn_reference_insert_pieces (vuse, set, base_set, type, - operands.copy (), value, value_id); + return vn_reference_insert_pieces (vuse, set, base_set, offset, max_size, + type, operands.copy (), value, value_id); } /* Return a value-number for RCODE OPS... either by looking up an existing @@ -3771,6 +3789,8 @@ vn_reference_lookup_3 (ao_ref *ref, tree vuse, void *data_, return (void *)-1; } *ref = r; + vr->offset = r.offset; + vr->max_size = r.max_size; /* Do not update last seen VUSE after translating. */ data->last_vuse_ptr = NULL; @@ -3959,6 +3979,8 @@ vn_reference_lookup_3 (ao_ref *ref, tree vuse, void *data_, if (maybe_ne (ref->size, r.size)) return (void *)-1; *ref = r; + vr->offset = r.offset; + vr->max_size = r.max_size; /* Do not update last seen VUSE after translating. */ data->last_vuse_ptr = NULL; @@ -4023,6 +4045,10 @@ vn_reference_lookup_pieces (tree vuse, alias_set_type set, vr1.type = type; vr1.set = set; vr1.base_set = base_set; + /* We can pretend there's no extra info fed in since the ao_refs offset + and max_size are computed only from the VN reference ops. */ + vr1.offset = 0; + vr1.max_size = -1; vr1.hashcode = vn_reference_compute_hash (&vr1); if ((cst = fully_constant_vn_reference_p (&vr1))) return cst; @@ -4149,6 +4175,8 @@ vn_reference_lookup (tree op, tree vuse, vn_lookup_kind kind, ao_ref_init (&op_ref, op); vr1.set = ao_ref_alias_set (&op_ref); vr1.base_set = ao_ref_base_alias_set (&op_ref); + vr1.offset = 0; + vr1.max_size = -1; vr1.hashcode = vn_reference_compute_hash (&vr1); if (mask == NULL_TREE) if (tree cst = fully_constant_vn_reference_p (&vr1)) @@ -4171,7 +4199,13 @@ vn_reference_lookup (tree op, tree vuse, vn_lookup_kind kind, if (!valueized_anything || !ao_ref_init_from_vn_reference (&r, vr1.set, vr1.base_set, vr1.type, ops_for_ref)) - ao_ref_init (&r, op); + { + ao_ref_init (&r, op); + /* Record the extra info we're getting from the full ref. */ + ao_ref_base (&r); + vr1.offset = r.offset; + vr1.max_size = r.max_size; + } vn_walk_cb_data data (&vr1, r.ref ? NULL_TREE : op, last_vuse_ptr, kind, tbaa_p, mask, redundant_store_removal_p); @@ -4227,6 +4261,8 @@ vn_reference_lookup_call (gcall *call, vn_reference_t *vnresult, vr->punned = false; vr->set = 0; vr->base_set = 0; + vr->offset = 0; + vr->max_size = -1; vr->hashcode = vn_reference_compute_hash (vr); vn_reference_lookup_1 (vr, vnresult); } @@ -4290,6 +4326,10 @@ vn_reference_insert (tree op, tree result, tree vuse, tree vdef) ao_ref_init (&op_ref, op); vr1->set = ao_ref_alias_set (&op_ref); vr1->base_set = ao_ref_base_alias_set (&op_ref); + /* Specifically use an unknown extent here, we're not doing any lookup + and assume the caller didn't either (or it went VARYING). */ + vr1->offset = 0; + vr1->max_size = -1; vr1->hashcode = vn_reference_compute_hash (vr1); vr1->result = TREE_CODE (result) == SSA_NAME ? SSA_VAL (result) : result; vr1->result_vdef = vdef; @@ -4332,7 +4372,8 @@ vn_reference_insert (tree op, tree result, tree vuse, tree vdef) vn_reference_t vn_reference_insert_pieces (tree vuse, alias_set_type set, - alias_set_type base_set, tree type, + alias_set_type base_set, + poly_int64 offset, poly_int64 max_size, tree type, vec operands, tree result, unsigned int value_id) @@ -4349,6 +4390,8 @@ vn_reference_insert_pieces (tree vuse, alias_set_type set, vr1->punned = false; vr1->set = set; vr1->base_set = base_set; + vr1->offset = offset; + vr1->max_size = max_size; vr1->hashcode = vn_reference_compute_hash (vr1); if (result && TREE_CODE (result) == SSA_NAME) result = SSA_VAL (result); @@ -5867,6 +5910,8 @@ visit_reference_op_call (tree lhs, gcall *stmt) vr2->type = vr1.type; vr2->punned = vr1.punned; vr2->set = vr1.set; + vr2->offset = vr1.offset; + vr2->max_size = vr1.max_size; vr2->base_set = vr1.base_set; vr2->hashcode = vr1.hashcode; vr2->result = lhs; diff --git a/gcc/tree-ssa-sccvn.h b/gcc/tree-ssa-sccvn.h index 8ec1de02d30..82f6f7323f9 100644 --- a/gcc/tree-ssa-sccvn.h +++ b/gcc/tree-ssa-sccvn.h @@ -145,6 +145,8 @@ typedef struct vn_reference_s tree vuse; alias_set_type set; alias_set_type base_set; + poly_int64 offset; + poly_int64 max_size; tree type; unsigned punned : 1; vec operands; @@ -268,6 +270,7 @@ tree vn_reference_lookup (tree, tree, vn_lookup_kind, vn_reference_t *, bool, tree * = NULL, tree = NULL_TREE, bool = false); void vn_reference_lookup_call (gcall *, vn_reference_t *, vn_reference_t); vn_reference_t vn_reference_insert_pieces (tree, alias_set_type, alias_set_type, + poly_int64, poly_int64, tree, vec, tree, unsigned int); void print_vn_reference_ops (FILE *, const vec);