From patchwork Sun Jul 21 05:33:04 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Oleg Endo X-Patchwork-Id: 1962848 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=fZoA2J+Z; 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 4WRXBr6pY2z1yYm for ; Sun, 21 Jul 2024 15:33:39 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 6CD5E385E457 for ; Sun, 21 Jul 2024 05:33:35 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-pl1-x641.google.com (mail-pl1-x641.google.com [IPv6:2607:f8b0:4864:20::641]) by sourceware.org (Postfix) with ESMTPS id 544AC385DDC5 for ; Sun, 21 Jul 2024 05:33:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 544AC385DDC5 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 544AC385DDC5 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::641 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1721539991; cv=none; b=O7i6hiobdr/kGPN/b98msYDJfc6vxb1Qk8cmQ4WNV5gLBv58YvUX3kqeFu3c4wxp15OGUxwMe2T4TCs5rqwR7jUrLunvQrC1EP1OskazlPMMq4dusmCUnCsHsLv/AgBoFjLHVomEkslK/pgMnR88M2texWmN9xCDmCqgmOG8qTo= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1721539991; c=relaxed/simple; bh=5t1jsONQo2l1meLaIAscxpDPyKmbr/jqlmkpJBoMtJA=; h=DKIM-Signature:Message-ID:Subject:From:To:Date:MIME-Version; b=NerzkFqthHVaikurX/bFnHNUM2TOS5kbQ9s9YSBtcpLCTK/tN76kFAA9SwNgvMJupCTamdzWmtHYoTyfd2YIXq0d74okjXBSn716h6cNIAoTysq72Tm6SKa8J/EVtpWddwRAiikojw+CiP/SZlU/i2Ik5ykcA0Rw1bicd64CkFQ= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pl1-x641.google.com with SMTP id d9443c01a7336-1fc569440e1so27287045ad.3 for ; Sat, 20 Jul 2024 22:33:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1721539987; x=1722144787; darn=gcc.gnu.org; h=mime-version:user-agent:references:in-reply-to:date:to:from:subject :message-id:from:to:cc:subject:date:message-id:reply-to; bh=HfeWVjdmTY4BncrGPONLhxyqnIg3FMf76PYshWncA0Y=; b=fZoA2J+ZAmEekKm3jA6EA/cG4XRSyCZTCuODPSp26/M0kGyvaUyo5uj4S8/YP8FoL5 12Lerm1Ynot0YdZqKk3mPf4o6RTklWIyzoS68uqRe9XWKQNJIOL/ywyrf9Lz2kF2E1Aj SNPzuCVSYTglg1g4pKh1538O+BTdfPIxfx0HZ2cOLz/rMgjPUp4OLOICdLIGiSxbE7Z1 tk9AhFODiVwBdbD/+V0zCOgO8zwsDOXdt3DTU9l80De52ivCo1bNvT+lbKy9Z1k6jDSi 8zOo7HHpKK3L7rH6nSI8EFQuzR3G9QhbJz9YvNaub7kDc9lP8gFeZxTw2roTcdmCmHi/ um3w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1721539987; x=1722144787; h=mime-version:user-agent:references:in-reply-to:date:to:from:subject :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=HfeWVjdmTY4BncrGPONLhxyqnIg3FMf76PYshWncA0Y=; b=kj1ldL55XYXbwuup9TYGR7xCMRLOE0WfEmfpIg2gClIfJTKWCmMfDvQBj5NQeqL7Vs qiI8HmWxwSa3qngRzy3a1nLm13PCYH8P3WjcKL5z+XnvNTc6XUdY70322E9tJm8zb2B7 RTsjWlhie/jZ8jg8VQ2F63DU9TZPdg5TTXRZAH0R9Be8qVn7n9aIQzApdfSbQCQ1ITv3 2oT2ZVRVH1uXD90Ob0u97FMieTRTsmxoZTm6PJkrtSkWiK3Z3u2knuZmph9rwGnaWAFh N5dJjG+90X0TBFbEly1Z/kWAEWrQbTpwYCffoWl7nDk2bXmbF20RPftJ2Io6rPMrJAEE lT4Q== X-Forwarded-Encrypted: i=1; AJvYcCWuevkbrFDIE8u2ftF/Oku3suR9DG/WHDiLG80MiOOk5PENZ0m1ysRMBLzyKgUoXdTpwFyzu2n8AygdzqoQZPfR8/Ii/n2lIw== X-Gm-Message-State: AOJu0Yyk3jUcVznSwPQ6vXFD57uo+Ai0pAVc/XM4QOp0JZ8ONgQU3Xyq mrMO3C4nXeeUGygQB6s8o7X+l3SPO5bKWlqgmlwTOU9xjjyxpjnO X-Google-Smtp-Source: AGHT+IE0MJ5Ls/UjZmUGQ7QJqnmzyAbJaxmJo/BqYaL2r+iUzkPGAViMtaEj8id6VesriKiwOY4gfg== X-Received: by 2002:a17:902:c411:b0:1fb:29e1:7635 with SMTP id d9443c01a7336-1fd7453a8d7mr43829815ad.13.1721539987047; Sat, 20 Jul 2024 22:33:07 -0700 (PDT) Received: from localhost.localdomain ([2407:c800:3620:e800:7910:9280:8105:5a2]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-1fd6f4647f5sm30652665ad.252.2024.07.20.22.33.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 20 Jul 2024 22:33:06 -0700 (PDT) Message-ID: Subject: [SH, committed]: Fix outage caused by secondary combine pass (was: Re: [RFC/PATCH] libgcc: sh: Use soft-fp for non-hosted SH3/SH4) From: Oleg Endo To: Jeff Law , =?iso-8859-1?q?S=E9bastien?= Michelland , gcc-patches@gcc.gnu.org Date: Sun, 21 Jul 2024 14:33:04 +0900 In-Reply-To: References: <20240703100855.3855337-1-sebastien.michelland@lcis.grenoble-inp.fr> <313332187cfb0b6a147de33b251c432dfc95a826.camel@t-online.de> User-Agent: Evolution 3.50.4 (3.50.4-1.fc39) MIME-Version: 1.0 X-Spam-Status: No, score=-9.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_ENVFROM_END_DIGIT, 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 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, I've committed the attached patch to fix the full gcc + libstdc++ build on sh-elf. Best regards, Oleg Endo On Sat, 2024-07-06 at 07:35 -0600, Jeff Law wrote: > > On 7/5/24 1:28 AM, Sébastien Michelland wrote: > > Hi Oleg! > > > > > I don't understand why this is being limited to SH3 and SH4 only? > > > Almost all SH4 systems out there have an FPU (unless special > > > configurations > > > are used).  So I'd say if switching to soft-fp, then for SH-anything, not > > > just SH3/SH4. > > > > > > If it yields some improvements for some users, I'm all for it. > > > > Yeah I just defaulted to SH3/SH4 conservatively because that's the only > > hardware I have. (My main platform also happens to be one of these SH4 > > without an FPU, the SH4AL-DSP.) > > > > Once this is tested/validated on simulator, I'll happily simplify the > > patch to apply to all SH. > > > > > I think it would make sense to test it using sh-sim on SH2 big-endian and > > > little endian at least, as that doesn't have an FPU and hence would run > > > tests utilizing soft-fp. > > > > > > After building the toolchain for --target=sh-elf, you can use this to run > > > the testsuite in the simulator: > > > > > > make -k check RUNTESTFLAGS="--target_board=sh-sim\{-m2/-ml,-m2/-mb}" > > > > > > (add make -j parameter according to you needs -- it will be slow) > > > > Alright, it might take a little bit. > > > > Building the combined tree of gcc/binutils/newlib masters (again > > following [1]) gives me an ICE in libstdc++v3/src/libbacktrace, > > irrespective of my libgcc change: > This is almost certainly a poorly written pattern. I just fixed a bunch > of these, but not this one. Essentially a recent change in the generic > parts of the compiler is exposing some bugs in the SH backend. > Specifically: > > > ;; Store (negated) T bit as all zeros or ones in a reg. > > ;; subc Rn,Rn ! Rn = Rn - Rn - T; T = T > > ;; not Rn,Rn ! Rn = 0 - Rn > > ;; > > ;; Note the call to sh_split_treg_set_expr may clobber > > ;; the T reg. We must express this, even though it's > > ;; not immediately obvious this pattern changes the > > ;; T register. > > (define_insn_and_split "mov_neg_si_t" > > [(set (match_operand:SI 0 "arith_reg_dest" "=r") > > (neg:SI (match_operand 1 "treg_set_expr"))) > > (clobber (reg:SI T_REG))] > > "TARGET_SH1" > > { > > gcc_assert (t_reg_operand (operands[1], VOIDmode)); > > return "subc %0,%0"; > > } > > "&& can_create_pseudo_p () && !t_reg_operand (operands[1], VOIDmode)" > > [(const_int 0)] > > { > > sh_treg_insns ti = sh_split_treg_set_expr (operands[1], curr_insn); > > emit_insn (gen_mov_neg_si_t (operands[0], get_t_reg_rtx ())); > > > > if (ti.remove_trailing_nott ()) > > emit_insn (gen_one_cmplsi2 (operands[0], operands[0])); > > > > DONE; > > } > > [(set_attr "type" "arith")]) > > > As written this pattern could match after register allocation is > complete and thus we can't create new pseudos (the condition TARGET_SH1 > controls that behavior). operands[1] won't necessarily be the T > register in that case. > > The split condition fails because we can't create new pseudos, so it's > left as-is. At final assembly time the assertion triggers. > > the "&& can_create_pseudo ()" part of the split condition should be > moved into the main condition. I think that's all that's necessary to > fix this problem. It'd probably be best of Oleg went through the > various define_insn_and_split patterns that utilize can_create_pseudo in > their split condition and evaluated them. > > I only fixed the most obvious cases in my change from this morning. I > don't typically work on the SH port and for changes which aren't > obviously correct, Oleg is in a better position to evaluate the proper fix. > > jeff From 9e740e7d71d02369774e1380902bddd9681c463f Mon Sep 17 00:00:00 2001 From: Oleg Endo Date: Sun, 21 Jul 2024 14:11:21 +0900 Subject: [PATCH] SH: Fix outage caused by recently added 2nd combine pass after reg alloc I've also confirmed on the CSiBE set that the secondary combine pass is actually beneficial on SH. It does result in some code size reductions. gcc/CHangeLog: * config/sh/sh.md (mov_neg_si_t): Allow insn and split after register allocation. (*treg_noop_move): New insn. --- gcc/config/sh/sh.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/gcc/config/sh/sh.md b/gcc/config/sh/sh.md index 3e97825..7eee12c 100644 --- a/gcc/config/sh/sh.md +++ b/gcc/config/sh/sh.md @@ -8407,21 +8407,29 @@ { gcc_assert (t_reg_operand (operands[1], VOIDmode)); return "subc %0,%0"; } - "&& can_create_pseudo_p () && !t_reg_operand (operands[1], VOIDmode)" + "&& !t_reg_operand (operands[1], VOIDmode)" [(const_int 0)] { sh_treg_insns ti = sh_split_treg_set_expr (operands[1], curr_insn); emit_insn (gen_mov_neg_si_t (operands[0], get_t_reg_rtx ())); if (ti.remove_trailing_nott ()) emit_insn (gen_one_cmplsi2 (operands[0], operands[0])); DONE; } [(set_attr "type" "arith")]) +;; no-op T bit move which can result from other optimizations. +(define_insn_and_split "*treg_noop_move" + [(set (reg:SI T_REG) (reg:SI T_REG))] + "TARGET_SH1" + "#" + "&& 1" + [(const_int 0)]) + ;; Invert the T bit. ;; On SH2A we can use the nott insn. On anything else this must be done with ;; multiple insns like: ;; movt Rn -- libgit2 1.7.2