Date: Sat, 28 Jun 2008 00:24:04 +0200 From: Marius Strobl <marius@alchemy.franken.de> To: Christoph Mallon <christoph.mallon@gmx.de> Cc: cvs-src@FreeBSD.org, src-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/sparc64/include in_cksum.h Message-ID: <20080627222404.GJ1215@alchemy.franken.de> In-Reply-To: <48654667.1040401@gmx.de> References: <200806252105.m5PL5AUp064418@repoman.freebsd.org> <48654667.1040401@gmx.de>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080627222404.GJ1215>