From patchwork Sun Nov 3 22:22:09 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lewis Hyatt X-Patchwork-Id: 2005827 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20230601 header.b=FtmWGEwp; dkim-atps=neutral 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=server2.sourceware.org; envelope-from=gcc-patches-bounces~incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=patchwork.ozlabs.org) Received: from server2.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 (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4XhThN6NG9z1xwV for ; Mon, 4 Nov 2024 09:25:56 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 1B3C7385841D for ; Sun, 3 Nov 2024 22:25:55 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-qv1-xf2e.google.com (mail-qv1-xf2e.google.com [IPv6:2607:f8b0:4864:20::f2e]) by sourceware.org (Postfix) with ESMTPS id 079E23858C56 for ; Sun, 3 Nov 2024 22:22:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 079E23858C56 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 079E23858C56 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::f2e ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1730672568; cv=none; b=vizw/lfiha6tTWN/niQpSamZ4cjR0OuIQRkcGhIGZxfJsegz4lJvntBmM6nOQ3wJjqbMbroERsyiyd7SNE3XXjtcpLLHkoyWupSsFUG32uSppAiHqOIIrbu2oABOCq3DMoA8YNnFvN8X8FN6uHvwWU8tUivTUA2o7pP8xg0bhTs= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1730672568; c=relaxed/simple; bh=6P4h5spvSKVkS5oE6M/C0vS6W1aMLqA4Pf9lXx75vnI=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=Aim9xpZOLxUjVcUE6Eg7oAsOvkINyEhxmL8f3NPXFkirxXW/hdT7EkrDgkv+RoWgekTZwIDkmlh9/VASQgrKNUPa/3MDUUquIGmuMvXUJ+uZT6GUBlsvUU6YhpICXA/9xYPfJwcWisrJOG8QO/qni1fU4uMcow8wrDoW06kKhUY= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-qv1-xf2e.google.com with SMTP id 6a1803df08f44-6cc1b20ce54so32335216d6.0 for ; Sun, 03 Nov 2024 14:22:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1730672565; x=1731277365; darn=gcc.gnu.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=G4Ft5DMmJiRWxGTzHRMQr4BXfSkLaJ56kREhiA8n7DM=; b=FtmWGEwpouMf2DLt/xNW1WrHQEUjN//E0NS7om7WD49XYQUKa+DbXb7gRki6p2bGy7 vfCkCXKZCkaBpaaomugs5Zpph1ZR7FR0XXnZmu1kxbo12rA9irPtFsC9dZO9OcVA5yg5 31jB7v+JLBsMrjmfaKcupPlkF58Y/noebS2ZSyARRPzKHGXbxRyaLXgvW1Qkt8AGvpl6 qdFTAf6mJNTV5heMUIBKiL31u0gIa3GYD6sofZ1kQV6ZmfQXunVkpN/MChmwCPnumd/7 eTGahALUmuWV7uzddWg4jQXQVlz8LIPxc8jXzkJYvKbYwzwAUxYaGuJbwfyFr9vn2Y2/ S6zQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730672565; x=1731277365; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=G4Ft5DMmJiRWxGTzHRMQr4BXfSkLaJ56kREhiA8n7DM=; b=Jbc6IdEXNue1HrnYQAzkDN7CNbghos5i8/E5RxH1mYrsqfcN36xMY9YkbsQLT83qIG kHFtgD2ClBmezNfVUwuL5ZUotidxNzSJt7j3qD2yMBChCt7pyBOV00GhMSEo13YpsbWg CN/EWdff6MGRxDVdjGlB5tAVCHhcSuaSpOmzgidjral77vOxDySpX/KJrP8/3zFTZr2A /MO+154ukiqL4gqXM5uhdqTA/r5COk9HbFVRvCU3Kqecy8gre/QOFLFn803rmYBcNRLr YnRqCXplBPDVCa/s6uaWj3wAP7hmRW7szwiTHEsau3hRhrK4JBQze1J8SWxkpXrcE49O l7yQ== X-Gm-Message-State: AOJu0YynYn75c+ThXwXgc6Zi3DYId75r57uiVgo6O5WZm1vyZ+yOr6E0 bMraJcvf/it3t03FQTtVHKJR8xn6K+G5PKTzG+Guml44UCS9iJP8D5F+JA== X-Google-Smtp-Source: AGHT+IFOL+T9hyfcWuz6OszGCLiDxoOpn38sSvovGPbv6HaiC/j+GIN9HUbx7STM+NGFw9sq3Aq35A== X-Received: by 2002:a05:6214:4387:b0:6d1:85a7:3cc0 with SMTP id 6a1803df08f44-6d35c0f5494mr133343506d6.12.1730672565327; Sun, 03 Nov 2024 14:22:45 -0800 (PST) Received: from localhost.localdomain ([173.54.240.58]) by smtp.gmail.com with ESMTPSA id 6a1803df08f44-6d35415b2fesm42025866d6.92.2024.11.03.14.22.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 03 Nov 2024 14:22:44 -0800 (PST) From: Lewis Hyatt To: gcc-patches@gcc.gnu.org Cc: Lewis Hyatt Subject: [PATCH 04/15] tree-phinodes: Use 4 instead of 2 as the minimum number of phi args Date: Sun, 3 Nov 2024 17:22:09 -0500 Message-Id: <20241103222220.933471-5-lhyatt@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20241103222220.933471-1-lhyatt@gmail.com> References: <20241103222220.933471-1-lhyatt@gmail.com> MIME-Version: 1.0 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~incoming=patchwork.ozlabs.org@gcc.gnu.org Currently, when we allocate a gphi object, we round up the capacity for the trailing arguments array such that it will make full use of the page size that ggc will allocate. While there is also an explicit minimum of 2 arguments, in practice after rounding to the ggc page size there is always room for at least 4. It seems we have some code that has come to depend on there being this much room before reallocation of a PHI is required. For example, the function loop_version () used during loop optimization will make sure there is room for an additional edge on each PHI that it processes. But there are call sites which cache a PHI pointer prior to calling loop_version () and assume it remains valid afterward, thus implicitly assuming that the PHI will have spare capacity. Examples include split_loop () and gen_parallel_loop (). This works fine now, but if the size of a gphi becomes larger, e.g. due to configuring location_t to be a 64-bit type, then on 32-bit platforms it ends up being possible to get a gphi with only 2 arguments of capacity, causing the above call sites of loop_version () to fail. (They store a pointer to a gphi object that no longer has the same meaning it did before it got reallocated.) The testcases gcc.dg/torture/pr113707-2.c and gcc.dg/graphite/pr81945.c exhibit that failure mode. It may be necessary to adjust those call sites to make this more robust, but in the meantime, changing the minimum from 2 to 4 does no harm given the minimum is practically 4 anyway, and it resolves the issue for 32-bit platforms. gcc/ChangeLog: * tree-phinodes.cc (MIN_PHI_ARGS): New constant. (allocate_phi_node): Change from hard-coded value 2 to MIN_PHI_ARGS, which is now 4. (ideal_phi_node_len): Likewise. (release_phi_node): Likewise. --- gcc/tree-phinodes.cc | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/gcc/tree-phinodes.cc b/gcc/tree-phinodes.cc index 5a7e4a94e57..9d8e16ac200 100644 --- a/gcc/tree-phinodes.cc +++ b/gcc/tree-phinodes.cc @@ -63,11 +63,12 @@ along with GCC; see the file COPYING3. If not see walking the elements of the last array entry would result in finding less than .1% additional reusable PHI nodes. - Note that we can never have less than two PHI argument slots. Thus, - the -2 on all the calculations below. */ + Note that we can never have less than MIN_PHI_ARGS argument slots. Thus, + the subtraction of MIN_PHI_ARGS on all the calculations below. */ #define NUM_BUCKETS 10 -static GTY ((deletable (""))) vec *free_phinodes[NUM_BUCKETS - 2]; +#define MIN_PHI_ARGS 4 +static GTY ((deletable (""))) vec *free_phinodes[NUM_BUCKETS - MIN_PHI_ARGS]; static unsigned long free_phinode_count; static int ideal_phi_node_len (int); @@ -94,17 +95,18 @@ static inline gphi * allocate_phi_node (size_t len) { gphi *phi; - size_t bucket = NUM_BUCKETS - 2; + size_t bucket = NUM_BUCKETS - MIN_PHI_ARGS; size_t size = sizeof (struct gphi) + (len - 1) * sizeof (struct phi_arg_d); if (free_phinode_count) - for (bucket = len - 2; bucket < NUM_BUCKETS - 2; bucket++) + for (bucket = len - MIN_PHI_ARGS; bucket < NUM_BUCKETS - MIN_PHI_ARGS; + bucket++) if (free_phinodes[bucket]) break; /* If our free list has an element, then use it. */ - if (bucket < NUM_BUCKETS - 2 + if (bucket < NUM_BUCKETS - MIN_PHI_ARGS && gimple_phi_capacity ((*free_phinodes[bucket])[0]) >= len) { free_phinode_count--; @@ -145,9 +147,8 @@ ideal_phi_node_len (int len) size_t size, new_size; int log2, new_len; - /* We do not support allocations of less than two PHI argument slots. */ - if (len < 2) - len = 2; + /* We do not support allocations of less than MIN_PHI_ARGS argument slots. */ + len = MAX (len, MIN_PHI_ARGS); /* Compute the number of bytes of the original request. */ size = sizeof (struct gphi) @@ -225,14 +226,13 @@ release_phi_node (gimple *phi) /* Immediately return the memory to the allocator when we would only ever re-use it for a smaller size allocation. */ - if (len - 2 >= NUM_BUCKETS - 2) + if (len >= NUM_BUCKETS) { ggc_free (phi); return; } - bucket = len > NUM_BUCKETS - 1 ? NUM_BUCKETS - 1 : len; - bucket -= 2; + bucket = len - MIN_PHI_ARGS; vec_safe_push (free_phinodes[bucket], phi); free_phinode_count++; }