On Fri, Oct 08, 2021 at 12:39:15PM -0500, Segher Boessenkool wrote:
> On Thu, Oct 07, 2021 at 08:04:23PM -0500, Paul A. Clarke wrote:
> > On Thu, Oct 07, 2021 at 06:39:06PM -0500, Segher Boessenkool wrote:
> > > > + __asm__ __volatile__ ("mffsce %0" : "=f" (__fpscr_save.__fr));
> > >
> > > The __volatile__ does likely not do what you want. As far as I can see
> > > you do not want one here anyway?
> > >
> > > "volatile" does not order asm wrt fp insns, which you likely *do* want.
> >
> > Reading the GCC docs, it looks like the "volatile" qualifier for "asm"
> > has no effect at all (6.47.1):
> >
> > | The optional volatile qualifier has no effect. All basic asm blocks are
> > | implicitly volatile.
> >
> > So, it could be removed without concern.
>
> This is not a basic asm (it contains a ":"; that is not just an easy way
> to see it, it is the *definition* of basic vs. extended asm).
Ah, basic vs extended. I learned something today... thanks for your
patience!
> The manual explains:
>
> """
> Note that the compiler can move even 'volatile asm' instructions
> relative to other code, including across jump instructions. For
> example, on many targets there is a system register that controls the
> rounding mode of floating-point operations. Setting it with a 'volatile
> asm' statement, as in the following PowerPC example, does not work
> reliably.
>
> asm volatile("mtfsf 255, %0" : : "f" (fpenv));
> sum = x + y;
>
> The compiler may move the addition back before the 'volatile asm'
> statement. To make it work as expected, add an artificial dependency to
> the 'asm' by referencing a variable in the subsequent code, for example:
>
> asm volatile ("mtfsf 255,%1" : "=X" (sum) : "f" (fpenv));
> sum = x + y;
> """
I see. Thanks for the reference. If I understand correctly, volatile
prevents some optimizations based on the defined inputs/outputs, but
the asm could still be subject to reordering.
In this particular case, I don't think it's an issue with respect to
reordering. The code in question is:
+ __asm__ __volatile__ ("mffsce %0" : "=f" (__fpscr_save.__fr));
+ __enables_save.__fpscr = __fpscr_save.__fpscr & 0xf8;
The output (__fpscr_save) is a source for the following assignment,
so the order should be respected, no?
With respect to volatile, I worry about removing it, because I do
indeed need that instruction to execute in order to clear the FPSCR
exception enable bits. That side-effect is not otherwise known to the
compiler.
> > > You do not need any of that __ either.
> >
> > I'm surprised that I don't. A .h file needs to be concerned about the
> > namespace it inherits, no?
>
> These are local variables in a function though. You get such
> complexities in macros, but never in functions, where everything is
> scoped. Local variables are a great thing. And macros are a bad thing!
They are local variables in a function *in an include file*, though.
If a user's preprocessor macro just happens to match a local variable name
there could be problems, right?
a.h:
inline void foo () {
int A = 0;
}
a.c:
#define A a+b
#include <a.h>
$ gcc -c -I. a.c
In file included from a.c:1:
a.c: In function ‘foo’:
a.h:1:12: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘+’
token
#define A a+b
^
a.c:2:17: note: in expansion of macro ‘A’
int foo() { int A = 0; }
^
a.h:1:13: error: ‘b’ undeclared (first use in this function)
#define A a+b
^
a.c:2:17: note: in expansion of macro ‘A’
int foo() { int A = 0; }
^
a.h:1:13: note: each undeclared identifier is reported only once for each
function it appears in
#define A a+b
^
a.c:2:17: note: in expansion of macro ‘A’
int foo() { int A = 0; }
^
PC
