From patchwork Thu Feb 15 08:05:34 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 1899192 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=MNhSsgBq; 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 4Tb71H60L4z23gM for ; Thu, 15 Feb 2024 19:06:09 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id A7E2F386185B for ; Thu, 15 Feb 2024 08:06:07 +0000 (GMT) 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.133.124]) by sourceware.org (Postfix) with ESMTPS id AE35A3861814 for ; Thu, 15 Feb 2024 08:05:43 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org AE35A3861814 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org AE35A3861814 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1707984349; cv=none; b=AzaUBSYbGf+Dpr6Fm5SBNg5nftgRu67t39QEiYK/Nf8SjTuPyynwAfpCXWDMFqz8gX99iCD9LFPZJRk2jc4wOdz/7OhEgK2fc/EmJlP9kcuxVmKpDQTDpuNphNP1T2HXKLKPn7pgcH8CYEW6/+d7GzA/S2jvI+4iStMrJUMSJ40= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1707984349; c=relaxed/simple; bh=oxzln7+5SactaaIOlPVD+ilH0UaepIie6v9MKUmocBk=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=jS0aCLHNL2L8JEky7ofdUYRvUc9fbFPTt5nDS3YxKcXRp2M6ORXIUe/JLXbNAc612K2JCP9kUsmUVA1oUh6QJ/BL+MyLbUQH05OIR6soDeYzea84hsNL/7fZjYqoFTQ2D/W+tqFeEMzpgi/gxjglswS7FRBTURs+B6C3T55+zq8= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1707984343; h=from:from:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type; bh=tyzG4H14/iYBN/u6bux1QhTTtcUf3GtJsc8DdL0I5qE=; b=MNhSsgBq0xhzXqT68A94jeidpI2I6ouY4nfc7RmEpBKIff5E3GfDYioYMMaIyrZWVpf7Up fSVzYTJiqMoYDLUXq6qJoj8NaiAe8eH5ESLwk6g7HQQY0fEMZzwyl7Qy+hKdnGKWYHF+KO mnyye4uBII820rkUU7adFr1KAnUJams= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-674-_mosVWN2MIWDdgtYf3Q0Ag-1; Thu, 15 Feb 2024 03:05:39 -0500 X-MC-Unique: _mosVWN2MIWDdgtYf3Q0Ag-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 0CAB0185A780; Thu, 15 Feb 2024 08:05:39 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.192.8]) by smtp.corp.redhat.com (Postfix) with ESMTPS id C052B475014; Thu, 15 Feb 2024 08:05:38 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.17.1/8.17.1) with ESMTPS id 41F85ZYd1207631 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Thu, 15 Feb 2024 09:05:36 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 41F85ZDI1207630; Thu, 15 Feb 2024 09:05:35 +0100 Date: Thu, 15 Feb 2024 09:05:34 +0100 From: Jakub Jelinek To: Richard Biener , Jeff Law , Vladimir Makarov Cc: gcc-patches@gcc.gnu.org Subject: [PATCH] expand: Fix handling of asm goto outputs vs. PHI argument adjustments [PR113921] Message-ID: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.9 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Disposition: inline X-Spam-Status: No, score=-3.8 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, KAM_SHORT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, 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: , Reply-To: Jakub Jelinek Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org Hi! The Linux kernel and the following testcase distilled from it is miscompiled, because tree-outof-ssa.cc (eliminate_phi) emits some fixups on some of the edges (but doesn't commit edge insertions). Later expand_asm_stmt emits further instructions on the same edge. Now the problem is that expand_asm_stmt uses insert_insn_on_edge to add its own fixups, but that function appends to the existing sequence on the edge if any. And the bug triggers when the fixup sequence emitted by eliminate_phi uses a pseudo which the fixup sequence emitted by expand_asm_stmt later on sets. So, we end up with (set (reg A) (asm_operands ...)) and on one of the edges queued sequence (set (reg C) (reg B)) // added by eliminate_phi (set (reg B) (reg A)) // added by expand_asm_stmt That is wrong, what we emit by expand_asm_stmt needs to be as close to the asm_operands as possible (they aren't known until expand_asm_stmt is called, the PHI fixup code assumes it is reg B which holds the right value) and the PHI adjustments need to be done after it. So, the following patch introduces a prepend_insn_to_edge function and uses it from expand_asm_stmt, so that we queue (set (reg B) (reg A)) // added by expand_asm_stmt (set (reg C) (reg B)) // added by eliminate_phi instead and so the value from the asm_operands output propagates correctly to the PHI result. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? I think we need to backport it to all release branches (fortunately non-supported compilers aren't affected because GCC 11 was the first one to support asm goto with outputs), in cfgexpand.cc it won't apply cleanly due to the PR113415 fix, but manually applying it there will work. 2024-02-15 Jakub Jelinek PR middle-end/113921 * cfgrtl.h (prepend_insn_to_edge): New declaration. * cfgrtl.cc (insert_insn_on_edge): Clarify behavior in function comment. (prepend_insn_to_edge): New function. * cfgexpand.cc (expand_asm_stmt): Use prepend_insn_to_edge instead of insert_insn_on_edge. * gcc.target/i386/pr113921.c: New test. Jakub --- gcc/cfgrtl.h.jj 2024-01-03 11:51:42.576577897 +0100 +++ gcc/cfgrtl.h 2024-02-14 21:19:13.029797669 +0100 @@ -38,6 +38,7 @@ extern edge try_redirect_by_replacing_ju extern void emit_barrier_after_bb (basic_block bb); extern basic_block force_nonfallthru_and_redirect (edge, basic_block, rtx); extern void insert_insn_on_edge (rtx, edge); +extern void prepend_insn_to_edge (rtx, edge); extern void commit_one_edge_insertion (edge e); extern void commit_edge_insertions (void); extern void print_rtl_with_bb (FILE *, const rtx_insn *, dump_flags_t); --- gcc/cfgrtl.cc.jj 2024-01-03 11:51:28.900767705 +0100 +++ gcc/cfgrtl.cc 2024-02-14 21:19:24.036651779 +0100 @@ -25,7 +25,7 @@ along with GCC; see the file COPYING3. - CFG-aware instruction chain manipulation delete_insn, delete_insn_chain - Edge splitting and committing to edges - insert_insn_on_edge, commit_edge_insertions + insert_insn_on_edge, prepend_insn_to_edge, commit_edge_insertions - CFG updating after insn simplification purge_dead_edges, purge_all_dead_edges - CFG fixing after coarse manipulation @@ -1966,7 +1966,8 @@ rtl_split_edge (edge edge_in) /* Queue instructions for insertion on an edge between two basic blocks. The new instructions and basic blocks (if any) will not appear in the - CFG until commit_edge_insertions is called. */ + CFG until commit_edge_insertions is called. If there are already + queued instructions on the edge, PATTERN is appended to them. */ void insert_insn_on_edge (rtx pattern, edge e) @@ -1984,6 +1985,25 @@ insert_insn_on_edge (rtx pattern, edge e e->insns.r = get_insns (); end_sequence (); +} + +/* Like insert_insn_on_edge, but if there are already queued instructions + on the edge, PATTERN is prepended to them. */ + +void +prepend_insn_to_edge (rtx pattern, edge e) +{ + /* We cannot insert instructions on an abnormal critical edge. + It will be easier to find the culprit if we die now. */ + gcc_assert (!((e->flags & EDGE_ABNORMAL) && EDGE_CRITICAL_P (e))); + + start_sequence (); + + emit_insn (pattern); + emit_insn (e->insns.r); + + e->insns.r = get_insns (); + end_sequence (); } /* Update the CFG for the instructions queued on edge E. */ --- gcc/cfgexpand.cc.jj 2024-02-10 11:25:09.995474027 +0100 +++ gcc/cfgexpand.cc 2024-02-14 21:27:23.219300727 +0100 @@ -3687,7 +3687,7 @@ expand_asm_stmt (gasm *stmt) copy = get_insns (); end_sequence (); } - insert_insn_on_edge (copy, e); + prepend_insn_to_edge (copy, e); } } } --- gcc/testsuite/gcc.target/i386/pr113921.c.jj 2024-02-14 21:21:15.194178515 +0100 +++ gcc/testsuite/gcc.target/i386/pr113921.c 2024-02-14 21:20:52.745476040 +0100 @@ -0,0 +1,20 @@ +/* PR middle-end/113921 */ +/* { dg-do run } */ +/* { dg-options "-O2" } */ + +__attribute__((noipa)) long +foo (void) +{ + long v; + asm volatile goto ("jmp %l2" : "=r" (v) : "0" (27) : : lab); + return v; +lab: + return 42; +} + +int +main () +{ + if (foo () != 42) + __builtin_abort (); +}