From patchwork Mon Sep 30 07:55:46 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Botcazou X-Patchwork-Id: 1990770 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; secure) header.d=adacore.com header.i=@adacore.com header.a=rsa-sha256 header.s=google header.b=PS/AObG/; 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 4XHD0Y3nSPz1xsc for ; Mon, 30 Sep 2024 17:56:13 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 1C12B3860740 for ; Mon, 30 Sep 2024 07:56:11 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-wr1-x435.google.com (mail-wr1-x435.google.com [IPv6:2a00:1450:4864:20::435]) by sourceware.org (Postfix) with ESMTPS id 7E9A53858C41 for ; Mon, 30 Sep 2024 07:55:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 7E9A53858C41 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=adacore.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=adacore.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 7E9A53858C41 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::435 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1727682951; cv=none; b=MRkUnNvPh68E6rZAJJPrkGr8z4gNPka3nnNcHge5mWZgyen8hwfsJQexnXZdtvN7Etnuc9F9wsJzNJaKznsflsCtcb7gVcpNwzQ6mjhENa739th+hsGZn+326iEzd9tdtrTfD0nZwDp8wHlnaLUYo9P6Uu+JhqRXVB09HuyoYLA= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1727682951; c=relaxed/simple; bh=lJ+LDMHbKGxqqCeF/Nzh9ClZPuzo0RgfyVFxZyB1F/8=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=YBAzizgAuHlZRiGXbKKIILin4StcJnDXT6yUsr5nXk1L+UVw/q06+QKMCm/4fOPl9ex236wx79C1NaN0a7IeRnoogDqZHlGW9ZoHgLhmGrEZyVvYwQeNW7oxo2P9uvY9MK0Ihg8p7hGm3tACajn2o/vt/zOQNZa/HdQJUOtfih8= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-wr1-x435.google.com with SMTP id ffacd0b85a97d-37ce14ab7eeso1355728f8f.2 for ; Mon, 30 Sep 2024 00:55:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=adacore.com; s=google; t=1727682948; x=1728287748; darn=gcc.gnu.org; h=content-transfer-encoding:mime-version:message-id:date:subject:to :from:from:to:cc:subject:date:message-id:reply-to; bh=dgPWlUtyCme8oUDGSdcPY+VVEwa4S8/bFlDKiqUYjBw=; b=PS/AObG/ymlUtFoToxTmyJSA1qJA9R9c/YFwqzAc0sOUScXZLGTXhwAQyEyuJL8ddo 69KK1jh+RF1/gnJETAJCnqswtqQmFsesb7UA7Ol/rSap+8/S5hZ2xdN5VlKeKTqNMjv/ qCLoYmEmbdtM9jrNXGpUqzGB4g7yt1EHo/vw7vMz0Q6SnnhqUR5lNzgYcE3R+EzxXgaU btTDoB62fnYyFbQjd8orzCEXsZEd8YV66fQk0mLNgGODOaYpFLgpnvE/1mLRn9woYEQH VIDZ4KL5f3yA/dIjVpVzRLzlNio5TSJxThzVnpI9k5zCCOuRjTrWErjOgPrvGRQdlasm nL9g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727682948; x=1728287748; h=content-transfer-encoding:mime-version:message-id:date:subject:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=dgPWlUtyCme8oUDGSdcPY+VVEwa4S8/bFlDKiqUYjBw=; b=Grzl790gSU2Cq6e+14K7BpQBK5GXohYiXNxx294aoVLEc7DCYg//XKHUaN/h5IdR+M w53F3L7DrXwhk8NuHMGBFKZ+zJWv4xe4cdomVfPbRd0Mqp07rqAX/pESpy+WYVfq4PM8 OmMQMjZLrNwVjTyhwhOxaHucStt1FTYCBdB0VYlWlGdFeq9yTRiwLOTHREgFzBh3K7qB PrH51vSjZZPt1DmZHdbnv+NHUSm1kgvnkpiXoo/2igWg7d15bMx5zskvZaEBFPfdvC4J kPAmpZbE/ljEbbCyJI50ZKzm2lTOtpz/cAu78SOIr1OOUT++FGE5CV5djlmlUg4A6f0w Lx6g== X-Gm-Message-State: AOJu0Yy6p8y+vamlr/oBgELRyhSB/by0jHK6Zfn+ZLT610xZ8A3NXNt5 oZTLxPqMNRCqCYmA9uHgOirFeHhgUgc+HNDibZTxE49qGHd2SpErOYdqr9kFrn+L0/4ODiSdpq8 = X-Google-Smtp-Source: AGHT+IHklNHqevWD+aW0PAEKqRX62AP5yQSjuep9urjTB+JRsBy7umJBrQiS9fs9s2+lnVXgCT6elQ== X-Received: by 2002:a5d:49c2:0:b0:37c:d270:e5cf with SMTP id ffacd0b85a97d-37cd5aa6ce1mr9074992f8f.33.1727682947982; Mon, 30 Sep 2024 00:55:47 -0700 (PDT) Received: from fomalhaut.localnet ([2a01:e0a:8d5:d990:e654:e8ff:fe8f:2ce6]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-37cd564d42esm8412235f8f.14.2024.09.30.00.55.46 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 30 Sep 2024 00:55:47 -0700 (PDT) From: Eric Botcazou X-Google-Original-From: Eric Botcazou To: gcc-patches@gcc.gnu.org Subject: [PATCH] Fix crash with constant initializer Date: Mon, 30 Sep 2024 09:55:46 +0200 Message-ID: <2215682.Hq7AAxBmiT@fomalhaut> MIME-Version: 1.0 X-Spam-Status: No, score=-11.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, 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 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 Hi, the attached Ada testcase compiled with -O2 -gnatn makes the compiler crash in vect_can_force_dr_alignment_p during SLP vectorization: if (decl_in_symtab_p (decl) && !symtab_node::get (decl)->can_increase_alignment_p ()) return false; because symtab_node::get (decl) returns a null node. The phenomenon occurs for a pair of twin symbols listed like so in .cgraph: Opt7_Pkg.T12b/17 (Opt7_Pkg.T12b) Type: variable definition analyzed Visibility: semantic_interposition external public artificial Aux: @0x44d45e0 References: Referring: opt7_pkg__enum_name_table/13 (addr) opt7_pkg__enum_name_table/13 (addr) Availability: not-ready Varpool flags: initialized read-only const-value-known Opt7_Pkg.T8b/16 (Opt7_Pkg.T8b) Type: variable definition analyzed Visibility: semantic_interposition external public artificial Aux: @0x7f9fda3fff00 References: Referring: opt7_pkg__enum_name_table/13 (addr) opt7_pkg__enum_name_table/13 (addr) Availability: not-ready Varpool flags: initialized read-only const-value-known with: opt7_pkg__enum_name_table/13 (Opt7_Pkg.Enum_Name_Table) Type: variable definition analyzed Visibility: semantic_interposition external public Aux: @0x44d45e0 References: Opt7_Pkg.T8b/16 (addr) Opt7_Pkg.T8b/16 (addr) Opt7_Pkg.T12b/17 (addr) Opt7_Pkg.T12b/17 (addr) Referring: opt7_pkg__image/2 (read) opt7_pkg__image/2 (read) opt7_pkg__image/2 (read) opt7_pkg__image/2 (read) opt7_pkg__image/2 (read) opt7_pkg__image/2 (read) opt7_pkg__image/2 (read) opt7_pkg__image/2 (read) Availability: not-ready Varpool flags: initialized read-only const-value-known being the crux of the matter. What happens is that symtab_remove_unreachable_nodes leaves the last symbol in kind of a limbo state: in .remove_symbols, we have: opt7_pkg__enum_name_table/13 (Opt7_Pkg.Enum_Name_Table) Type: variable Body removed by symtab_remove_unreachable_nodes Visibility: externally_visible semantic_interposition external public References: Referring: opt7_pkg__image/2 (read) opt7_pkg__image/2 (read) Availability: not_available Varpool flags: initialized read-only const-value-known This means that the "body" (DECL_INITIAL) of the symbol has been disregarded during reachability analysis, causing the first two symbols to be discarded: Reclaiming variables: Opt7_Pkg.T12b/17 Opt7_Pkg.T8b/16 but the DECL_INITIAL is explicitly preserved for later constant folding, which makes it possible to retrofit the DECLs corresponding to the first two symbols in the GIMPLE IR and ultimately leads vect_can_force_dr_alignment_p to crash. The decision to disregard the "body" (DECL_INITIAL) of the symbol is made in the first process_references present in ipa.cc: if (node->definition && !node->in_other_partition && ((!DECL_EXTERNAL (node->decl) || node->alias) || (possible_inline_candidate_p (node) /* We use variable constructors during late compilation for constant folding. Keep references alive so partitioning knows about potential references. */ || (VAR_P (node->decl) && (flag_wpa || flag_incremental_link == INCREMENTAL_LINK_LTO) && dyn_cast (node) ->ctor_useable_for_folding_p ())))) because neither flag_wpa nor flag_incremental_link = INCREMENTAL_LINK_LTO is true, while the decision to ultimately preserve the DECL_INITIAL is made later in remove_unreachable_nodes: /* Keep body if it may be useful for constant folding. */ if ((flag_wpa || flag_incremental_link == INCREMENTAL_LINK_LTO) || ((init = ctor_for_folding (vnode->decl)) == error_mark_node)) vnode->remove_initializer (); else DECL_INITIAL (vnode->decl) = init; I think that the testcase shows that the "body" of ctor_useable_for_folding_p symbols must always be considered for reachability analysis (which could make the above test on ctor_for_folding useless). But implementing that introduces a regression for g++.dg/ipa/devirt-39.C, because the vtable is preserved and in turn forces the method to be preserved, hence the special case for vtables. The test also renames the first process_references function in ipa.cc to clear the confusion with the second function in the same file. Bootstrapped/regtested on x86-64/Linux, OK for the mainline? 2024-09-30 Eric Botcazou * ipa.cc (process_references): Rename into... (mark_references): ...this. Always mark referenced external variables as reachable if they are usable for folding, except for vtables. (symbol_table::remove_unreachable_nodes): Adjust to renaming. 2024-09-30 Eric Botcazou * gnat.dg/specs/opt7.ads: New test. * gnat.dg/specs/opt7_pkg.ads: New helper. * gnat.dg/specs/opt7_pkg.adb: Likewise. diff --git a/gcc/ipa.cc b/gcc/ipa.cc index c453fca5d9b..342fa058160 100644 --- a/gcc/ipa.cc +++ b/gcc/ipa.cc @@ -124,41 +124,39 @@ possible_inline_candidate_p (symtab_node *node) return lookup_attribute ("always_inline", DECL_ATTRIBUTES (node->decl)); } -/* Process references. */ +/* Mark references from SNODE as (members of) REACHABLE and add them to the + queue starting at FIRST. */ static void -process_references (symtab_node *snode, - symtab_node **first, - hash_set *reachable) +mark_references (symtab_node *snode, symtab_node **first, + hash_set *reachable) { - int i; struct ipa_ref *ref = NULL; - for (i = 0; snode->iterate_reference (i, ref); i++) + for (int i = 0; snode->iterate_reference (i, ref); i++) { symtab_node *node = ref->referred; symtab_node *body = node->ultimate_alias_target (); - if (node->definition && !node->in_other_partition - && ((!DECL_EXTERNAL (node->decl) || node->alias) - || (possible_inline_candidate_p (node) - /* We use variable constructors during late compilation for - constant folding. Keep references alive so partitioning - knows about potential references. */ - || (VAR_P (node->decl) - && (flag_wpa - || flag_incremental_link - == INCREMENTAL_LINK_LTO) - && dyn_cast (node) - ->ctor_useable_for_folding_p ())))) + if (node->definition + && !node->in_other_partition + && (!DECL_EXTERNAL (node->decl) + || node->alias + || possible_inline_candidate_p (node) + /* We use variable constructors during late compilation for + constant folding, but vtables are handled separately. */ + || (VAR_P (node->decl) + && !DECL_VIRTUAL_P (node->decl) + && dyn_cast (node) + ->ctor_useable_for_folding_p ()))) { - /* Be sure that we will not optimize out alias target - body. */ + /* Be sure that we will not optimize out alias target body. */ if (DECL_EXTERNAL (node->decl) && node->alias && symtab->state < IPA_SSA_AFTER_INLINING) reachable->add (body); reachable->add (node); } + enqueue_node (node, first, reachable); } } @@ -409,8 +407,9 @@ symbol_table::remove_unreachable_nodes (FILE *file) && !reachable.add (next)) enqueue_node (next, &first, &reachable); } + /* Mark references as reachable. */ - process_references (node, &first, &reachable); + mark_references (node, &first, &reachable); } if (cgraph_node *cnode = dyn_cast (node))