From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3275 invoked by alias); 21 Jul 2019 17:17:43 -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 3045 invoked by uid 89); 21 Jul 2019 17:17:43 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-27.6 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.1 spammy=USB, six, chip, capability X-HELO: ste-pvt-msa1.bahnhof.se Received: from ste-pvt-msa1.bahnhof.se (HELO ste-pvt-msa1.bahnhof.se) (213.80.101.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 21 Jul 2019 17:17:40 +0000 Received: from localhost (localhost [127.0.0.1]) by ste-pvt-msa1.bahnhof.se (Postfix) with ESMTP id 24F5E4BA64; Sun, 21 Jul 2019 19:17:38 +0200 (CEST) X-Spam-Score: -2.899 Received: from ste-pvt-msa1.bahnhof.se ([127.0.0.1]) by localhost (ste-pvt-msa1.bahnhof.se [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id XUM3hfv0Jetk; Sun, 21 Jul 2019 19:17:26 +0200 (CEST) Received: from localhost (h-41-252.A163.priv.bahnhof.se [46.59.41.252]) (Authenticated sender: mb547485) by ste-pvt-msa1.bahnhof.se (Postfix) with ESMTPA id 767D24BA62; Sun, 21 Jul 2019 19:17:26 +0200 (CEST) Date: Sun, 21 Jul 2019 17:39:00 -0000 From: Fredrik Noring To: Paul Burton , Matthew Fortune Cc: "Maciej W. Rozycki" , =?utf-8?Q?J=C3=BCrgen?= Urban , gcc-patches@gcc.gnu.org Subject: [PATCH] MIPS: Fix GCC `noreorder' for undefined R5900 short loops Message-ID: <20190721171726.GA47580@sx9> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline User-Agent: Mutt/1.10.1 (2018-07-13) X-SW-Source: 2019-07/txt/msg01351.txt.bz2 Hi Paul, Matthew, Paul -- as I'm preparing the R5900 kernel patches there was a USB DMA series and breakage that needed attention. The fixes ending with ff2437befd8f ("usb: host: Fix excessive alignment restriction for local memory allocations") are now merged with Linus' kernel, and recommended for the R5900 series. The initial R5900 patch submission is getting closer. :) The present problem is related to GCC and the R5900 short loop bug[1]. It turns out GCC emits assembly code like loop: addiu $5,$5,1 addiu $4,$4,1 lb $2,-1($5) .set noreorder .set nomacro bne $2,$3,loop sb $2,-1($4) .set macro .set reorder that is undefined for the R5900 (this short loop has five instructions), for simple C code such as while ((*s++ = *p++) != '\n') ; in the kernel. The noreorder directive prohibits GAS from corrections, and GAS really ought to give an error for it, I think. In the meantime, I have a tool that does machine code analysis of ELF objects to catch undefined R5900 short loops, including those made with assembler macros in the kernel. [ In theory, GAS could actually insert NOPs prior to the noreorder directive, to make the loop longer that six instructions, but GAS does not have that kind of capability. Another option that GCC prevents is to place a NOP in the delay slot. ] A reasonable fix for GCC is perhaps to update gcc/config/mips/mips.md to not make explicit use of the branch delay slot, as suggested by the patch below? Then GCC will emit loop: addiu $5,$5,1 addiu $4,$4,1 lb $2,-1($5) sb $2,-1($4) bne $2,$3,loop that GAS will adjust in the ELF object to 4: 24a50001 addiu a1,a1,1 8: 24840001 addiu a0,a0,1 c: 80a2ffff lb v0,-1(a1) 10: a082ffff sb v0,-1(a0) 14: 1443fffb bne v0,v1,4 18: 00000000 nop where a NOP is placed in the delay slot to avoid the bug. For longer loops, this relies on GAS making appropriate use of the delay slot. I'm not sure if GAS is good at that, though? Fredrik References: [1] https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gas/config/tc-mips.c;h=b7b4b6989a12d3091a02de7155fbea3adbf1c9d7;hb=HEAD#l7133 On the R5900 short loops need to be fixed by inserting a NOP in the branch delay slot. The short loop bug under certain conditions causes loops to execute only once or twice. We must ensure that the assembler never generates loops that satisfy all of the following conditions: - a loop consists of less than or equal to six instructions (including the branch delay slot); - a loop contains only one conditional branch instruction at the end of the loop; - a loop does not contain any other branch or jump instructions; - a branch delay slot of the loop is not NOP (EE 2.9 or later). We need to do this because of a hardware bug in the R5900 chip. diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md index e17b1d522f0..acd31a8960c 100644 --- a/gcc/config/mips/mips.md +++ b/gcc/config/mips/mips.md @@ -1091,6 +1091,7 @@ (define_delay (and (eq_attr "type" "branch") (not (match_test "TARGET_MIPS16")) + (not (match_test "TARGET_FIX_R5900")) (eq_attr "branch_likely" "yes")) [(eq_attr "can_delay" "yes") (nil) @@ -1100,6 +1101,7 @@ ;; not annul on false. (define_delay (and (eq_attr "type" "branch") (not (match_test "TARGET_MIPS16")) + (not (match_test "TARGET_FIX_R5900")) (ior (match_test "TARGET_CB_NEVER") (and (eq_attr "compact_form" "maybe") (not (match_test "TARGET_CB_ALWAYS")))