From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 46850 invoked by alias); 24 Jul 2019 14:39:47 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 46841 invoked by uid 89); 24 Jul 2019 14:39:47 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-12.6 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.1 spammy=quality X-HELO: pio-pvt-msa2.bahnhof.se Received: from pio-pvt-msa2.bahnhof.se (HELO pio-pvt-msa2.bahnhof.se) (79.136.2.41) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 24 Jul 2019 14:39:45 +0000 Received: from localhost (localhost [127.0.0.1]) by pio-pvt-msa2.bahnhof.se (Postfix) with ESMTP id 1A05F3F5AD; Wed, 24 Jul 2019 16:39:43 +0200 (CEST) X-Spam-Score: -2.9 Received: from pio-pvt-msa2.bahnhof.se ([127.0.0.1]) by localhost (pio-pvt-msa2.bahnhof.se [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id y2kZXunE9xWp; Wed, 24 Jul 2019 16:39:42 +0200 (CEST) Received: from localhost (h-41-252.A163.priv.bahnhof.se [46.59.41.252]) (Authenticated sender: mb547485) by pio-pvt-msa2.bahnhof.se (Postfix) with ESMTPA id 732153F4B0; Wed, 24 Jul 2019 16:39:42 +0200 (CEST) Date: Wed, 24 Jul 2019 15:03:00 -0000 From: Fredrik Noring To: "Maciej W. Rozycki" Cc: Paul Burton , Matthew Fortune , =?utf-8?Q?J=C3=BCrgen?= Urban , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] MIPS: Fix GCC `noreorder' for undefined R5900 short loops Message-ID: <20190724143941.GA4262@sx9> References: <20190721171726.GA47580@sx9> <20190722155506.GA2726@sx9> <20190723165446.GA2728@sx9> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) X-SW-Source: 2019-07/txt/msg01577.txt.bz2 Hi Maciej, > > > > How can one reasonably prevent the bug when compiling a whole Linux > > > > distribution with thousands of packages if GAS merely warns and proceeds > > > > in many cases? > > > > > > I think the tools must not set a policy. By using `.set noreorder' the > > > user told the toolchain he knows better and asked to keep its hands away. > > > > > > As I say you can use `-Werror' for code auditing. And in most cases > > > manually scheduled delay slots in handcoded assembly are a sign of a > > > problem with the piece of code affected, regardless of the R5900 erratum. > > > > Well, it seems practical to use unofficial tools and a patched GAS to fix > > this assembly bug, then. It's a grave problem for the R5900 and it needs to > > be reliably detected. > > Why? What is wrong with using `-Werror'? > > Or you could use `-Wa,-fatal-warnings' to limit making warnings fatal to > the assembly stage only if you are concerned about bad high-level language > code quality interfering with your goal. It appears unfeasible to fix thousands of build scripts to work like that. Although the numbers with actual MIPS code in them likely are very small, some may source macros and other stuff via various libraries. Some distributions, such as Gentoo, do have global CFLAGS but such mechanism are unreliable. I understand that one may want to take a principled stance with "hands away" and "author asked for trouble", but fact is that whomever put a given noreorder directive into a piece of code most likely didn't have R5900 errata in mind, and R5900 users most likely don't want the assembler to generate code that is known to cause subtle bugs of all imaginable kinds, including data corruption. Hence my recommendation. It's not really an argument because I'm not going to ask that the official GAS changes behaviour on this. > > 3: > > + short_loop_war(3b) > > b 3b > > nop > > Is it needed here given that the delay slot instruction is a NOP anyway? Ah, no. That is a left-over from the note that "a branch delay slot of the loop is not NOP (EE 2.9 or later)", as it appears a NOP is not enough for EE before 2.9. Perhaps the short_loop_war macro can be improved to give errors when misused. For example, it could potentially give an error if it is placed outside of noreorder, but it doesn't seem like GAS can inform the macro whether it is inside or outside. > Also this source does not need `.set noreorder', except for the loop > around `1:' where a data dependency is present with the delay slot > instruction. That data dependency can surely be reworked too, so as to make the whole piece reorderable? > And I am surprised that it even assembles as it uses > `.cprestore' without the required offset argument (not that this pseudo-op > makes any sense here). And it's weird overall, e.g. it loads $ra > explicitly rather than using JALR (or JAL even). But these are unrelated > problems. Good to know, thanks. :) Fredrik