From owner-cvs-all@FreeBSD.ORG Fri Jun 27 22:24:10 2008 Return-Path: Delivered-To: cvs-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id E59611065679; Fri, 27 Jun 2008 22:24:10 +0000 (UTC) (envelope-from marius@alchemy.franken.de) Received: from alchemy.franken.de (alchemy.franken.de [194.94.249.214]) by mx1.freebsd.org (Postfix) with ESMTP id 6A7388FC14; Fri, 27 Jun 2008 22:24:10 +0000 (UTC) (envelope-from marius@alchemy.franken.de) Received: from alchemy.franken.de (localhost [127.0.0.1]) by alchemy.franken.de (8.14.3/8.14.3/ALCHEMY.FRANKEN.DE) with ESMTP id m5RMO5fE025316; Sat, 28 Jun 2008 00:24:06 +0200 (CEST) (envelope-from marius@alchemy.franken.de) Received: (from marius@localhost) by alchemy.franken.de (8.14.3/8.14.3/Submit) id m5RMO5it025315; Sat, 28 Jun 2008 00:24:05 +0200 (CEST) (envelope-from marius) Date: Sat, 28 Jun 2008 00:24:04 +0200 From: Marius Strobl To: Christoph Mallon Message-ID: <20080627222404.GJ1215@alchemy.franken.de> References: <200806252105.m5PL5AUp064418@repoman.freebsd.org> <48654667.1040401@gmx.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <48654667.1040401@gmx.de> User-Agent: Mutt/1.4.2.3i Cc: cvs-src@FreeBSD.org, src-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/sparc64/include in_cksum.h X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 27 Jun 2008 22:24:11 -0000 On Fri, Jun 27, 2008 at 09:58:31PM +0200, Christoph Mallon wrote: > Marius Strobl wrote: > >marius 2008-06-25 21:04:59 UTC > > > > FreeBSD src repository > > > > Modified files: > > sys/sparc64/include in_cksum.h > > Log: > > SVN rev 180011 on 2008-06-25 21:04:59Z by marius > > > > Use "__asm __volatile" rather than "__asm" for instruction sequences > > that modify condition codes (the carry bit, in this case). Without > > "__volatile", the compiler might add the inline assembler instructions > > between unrelated code which also uses condition codes, modifying the > > latter. > > This prevents the TCP pseudo header checksum calculation done in > > tcp_output() from having effects on other conditions when compiled > > with GCC 4.2.1 at "-O2" and "options INET6" left out. [1] > > > > Reported & tested by: Boris Kochergin [1] > > MFC after: 3 days > > This approach seems wrong to me and I think it works only by chance. The > condition codes ("cc") should be added to the clobbered list of the asm > statement instead of making the statement volatile: __asm("..." : $OUT : > $IN : "cc"); > This very case is also mentioned in the GCC documentation: > "If your assembler instruction can alter the condition code register, > add `cc' to the list of clobbered registers. GCC on some machines > represents the condition codes as a specific hardware register; `cc' > serves to name this register. On other machines, the condition code is > handled differently, and specifying `cc' has no effect. But it is > valid no matter what the machine." (Section 5.35 Assembler Instructions > with C Expression Operands) > > I wrote a letter directly to Marius about this issue two days ago, but I > got no response so far. Because this change has a MFC after 3 days, I'm > writing to this list. I wasn't aware that the clobber list allows to explicitly specify the condition codes, thanks for the hint. Though it unfortunately took me longer than two days to verify it's effect on the generated code; sparc64 could still have been one of the archs where "cc" has no effect. Besides I don't think using "__volatile" for this is that wrong, given that the sparc64 code generated by using "cc" and "__volatile" is nearly identical and given that at least i386 relies on "__volatile" telling GCC that the inline assembler uses the condition codes since quite some time. So the condition codes are probably part of what GCC treats as "important side-effects". Regarding the MFC, they don't happen automatically and the change was not wrong in general so there was no need to hurry :) Thanks again, Marius