[PATCH] avoid builtin sincos

[Michael Büsch]

Building linuxcnc 2.6 (git) with gcc 4.7.x on recent Debian x86_64 results
in unresolved 'sincos' symbol in various kernel modules.

The compiler replaces consecutive sin/cos calls by sincos calls automagically.

This patch adds -fno-builtin-sincos -fno-builtin-sin -fno-builtin-cos to
discourage the compiler from replacing sin/cos calls.

Signed-off-by: Michael Buesch <***@bues.ch>

---

Adding EXPORT_SYMBOL to the sincos from posemath does not solve the issue.
It results in a kernel panic, when the sincos is called.
I guess there's a difference in the call ABI.
I did not investigate on this further. I think we should add explicit sincos calls
where appropriate, if we want them.


Index: src/Makefile
===================================================================
--- src.orig/Makefile 2014-10-04 16:04:37.233794613 +0200
+++ src/Makefile 2014-10-04 16:06:17.214484287 +0200
@@ -740,7 +740,7 @@
-I$(BASEPWD)/libnml/posemath -I$(BASEPWD)/rtapi -I$(BASEPWD)/hal \
-I$(BASEPWD)/emc/nml_intf -I$(BASEPWD)/emc/kinematics -I$(BASEPWD)/emc/motion \
-DSEQUENTIAL_SUPPORT -DHAL_SUPPORT -DDYNAMIC_PLCSIZE -DRT_SUPPORT -DOLD_TIMERS_MONOS_SUPPORT -DMODBUS_IO_MASTER \
- -fno-fast-math $(call cc-option,-mieee-fp) -fno-unsafe-math-optimizations \
+ -fno-fast-math $(call cc-option,-mieee-fp) -fno-unsafe-math-optimizations -fno-builtin-sincos -fno-builtin-sin -fno-builtin-cos \
$(call cc-option,-Wframe-larger-than=2560) $(call cc-option,-Wno-declaration-after-statement) \
$(INTEGER_OVERFLOW_FLAGS)
ifneq ($(KERNELRELEASE),)
Index: src/Makefile.modinc.in
===================================================================
--- src.orig/Makefile.modinc.in 2014-10-04 16:04:37.233794613 +0200
+++ src/Makefile.modinc.in 2014-10-04 16:06:17.214484287 +0200
@@ -62,7 +62,7 @@
KERNELDIR := @KERNELDIR@
CC := @CC@
RTAI = @RTAI@
-RTFLAGS = $(filter-out -ffast-math,@RTFLAGS@ @EXT_RTFLAGS@) -fno-fast-math $(call cc-option,-mieee-fp) -fno-unsafe-math-optimizations
+RTFLAGS = $(filter-out -ffast-math,@RTFLAGS@ @EXT_RTFLAGS@) -fno-fast-math $(call cc-option,-mieee-fp) -fno-unsafe-math-optimizations -fno-builtin-sincos -fno-builtin-sin -fno-builtin-cos
RTFLAGS := -Os -g -I. -***@RTDIR@/include $(RTFLAGS) -DRTAPI -D_GNU_SOURCE -Drealtime
ifneq ($(KERNELRELEASE),)
ifeq ($(RTARCH):$(RTAI):$(filter $(RTFLAGS),-msse),x86_64:3:)

[Jeff Epler]

I believe that what happens is that gcc analyzes the function

void sincos(double x, double *sx, double *cx)
{
*sx = sin(x);
*cx = cos(x);
}

and determines that it is equivalent to the call
sincos(x, sx, cx);

at runtime, this of course becomes a nonterminating recursive call.

gcc documents, more or less, that it is permitted to do this in the info
section "Other Builtins". (sin and cos are treated as builtins, and one
valid optimization of these builtins is to turn one call to each into a
single sincos call) The -fno-builtin-sin flag is enough to prevent this
optimization.

With that in mind, it seems to me that only in sincos.c is it necessary
to avoid the use of the builtin functions. Consequently, I wonder what
happens if you *do* fix the export of the sincos symbol, then do
something like the below patch (untested)

FWIW Alec Ari has been working on what is effectively a fork of RTAI at
https://github.com/NTULINUX/RTAI -- after some IRC discussion about what
I think is the same issue, he ended up adding -ffreestanding to
rtai-config --cflags, which inhibits recognition of *all* built-in
functions except when used with the __builtin_ prefix.

Jeff

-- >8 --
Subject: [PATCH] sincos: restrict no-builtin to just one location

---
src/libnml/posemath/sincos.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/src/libnml/posemath/sincos.c b/src/libnml/posemath/sincos.c
index 4ea47b8..a10c2a8 100644
--- a/src/libnml/posemath/sincos.c
+++ b/src/libnml/posemath/sincos.c
@@ -32,6 +32,9 @@

#include "posemath.h"

+__attribute__((optimize("no-builtin-sin")))
+__attribute__((optimize("no-builtin-cos")))
+__attribute__((optimize("no-builtin-sincos")))
void sincos(double x, double *sx, double *cx)
{
*sx = sin(x);

[Michael Büsch]

On Sun, 5 Oct 2014 09:12:41 -0500

Post by Jeff Epler
I believe that what happens is that gcc analyzes the function
void sincos(double x, double *sx, double *cx)
{
*sx = sin(x);
*cx = cos(x);
}
and determines that it is equivalent to the call
sincos(x, sx, cx);
at runtime, this of course becomes a nonterminating recursive call.

Hm, yes. The backtrace looked like a corrupted stack, so this sounds plausible.

Post by Jeff Epler
With that in mind, it seems to me that only in sincos.c is it necessary
to avoid the use of the builtin functions. Consequently, I wonder what
happens if you *do* fix the export of the sincos symbol, then do
something like the below patch (untested)

Can test, when I'm back to the machine, later.

Post by Jeff Epler
FWIW Alec Ari has been working on what is effectively a fork of RTAI at
https://github.com/NTULINUX/RTAI -- after some IRC discussion about what
I think is the same issue, he ended up adding -ffreestanding to
rtai-config --cflags, which inhibits recognition of *all* built-in
functions except when used with the __builtin_ prefix.

Sounds like a good idea, because it essentially is freestanding.


Another thing is the sincos configure test we do.
What does it test? Userspace or kernelspace? Or both?
What happens, if I have a userspace with sincos and kernelspace without sincos (like in my case)? Does it work correctly and compile in the workaround for kernelspace posemath and leave it out for userspace posemath?

Please CC me in replies, so I can respond faster without reading the list.

[Jeff Epler]

Post by Michael Büsch
Another thing is the sincos configure test we do.
What does it test? Userspace or kernelspace? Or both?

Configure tests userspace. It's always assumed there's no sincos in
kernel space -- sincos.c:
22 #if defined(__KERNEL__)
23 #undef HAVE_SINCOS
24 #endif

A slight complicating factor is HAVE___SINCOS, which is again determined
by configure checking userspace; if HAVE___SINCOS were defined then I
think the kernel would end up without a definition of sincos. But on my
debian 7 system, the configure result for the function __sincos is 'no',
so this doesn't come up.

Post by Michael Büsch
What happens, if I have a userspace with sincos and kernelspace
without sincos (like in my case)? Does it work correctly and compile in
the workaround for kernelspace posemath and leave it out for userspace
posemath?

Yes, that is what is intended to happen.

Jeff

Post by Michael Büsch
Please CC me in replies, so I can respond faster without reading the list.
--
Michael

[Michael Büsch]

On Thu, 9 Oct 2014 09:54:01 -0500

Post by Jeff Epler

Post by Michael Büsch
Another thing is the sincos configure test we do.
What does it test? Userspace or kernelspace? Or both?

Configure tests userspace. It's always assumed there's no sincos in
22 #if defined(__KERNEL__)
23 #undef HAVE_SINCOS
24 #endif

Ok, it seems I missed that part.
This should be ok, as long as sincos is not added to rtai.

[Jeff Epler]

I looked at disassembly with

Post by Jeff Epler
+__attribute__((optimize("no-builtin-sin")))
+__attribute__((optimize("no-builtin-cos")))
+__attribute__((optimize("no-builtin-sincos")))
void sincos(double x, double *sx, double *cx)

on gcc version 4.9.1 and irritatingly the __attribute__ does not inhibit
the transformation to a sincos call. Nor does #pragma GCC optimize.
Only a commandline flag does the job.

The same goes for gcc 4.6, 4.7 and 4.8 all as packaged by Debian.

So my proposed patch doesn't seem likely to work out. Even if the gcc
folks viewed this as a bug and fixed it tomorrow, we'd still have years
of debian/ubuntu releases to support that behave like this.

Methodology: with the following contents in sc.c

/* ****************************************************** */
extern double sin(double);
extern double cos(double);

#ifdef USE_PRAGMA_FREESTANDING
#pragma GCC optimize "freestanding"
#endif

#ifdef USE_PRAGMA_NO_BUILTIN
#pragma GCC optimize "no-builtin-sin"
#pragma GCC optimize "no-builtin-cos"
#pragma GCC optimize "no-builtin-sincos"
#endif

#ifdef USE_ATTRIBUTE_FREESTANDING
__attribute__((optimize("freestanding")))
#endif

#ifdef USE_ATTRIBUTE_NO_BUILTIN
__attribute__((optimize("no-builtin-sin")))
__attribute__((optimize("no-builtin-cos")))
__attribute__((optimize("no-builtin-sincos")))
#endif

void fn(double x, double *sx, double *cx) {
*sx = sin(x);
*cx = cos(x);
}
/* ****************************************************** */

build using various compilers and flags:

set -o pipefail
for cc in gcc-4.6 gcc-4.7 gcc-4.8 gcc-4.9 clang-3.4 clang-3.5; do
for opt in -ffreestanding -fno-builtin-sin -DUSE_PRAGMA_FREESTANDING \
-DUSE_PRAGMA_NO_BUILTIN -DUSE_ATTRIBUTE_FREESTANDING \
-DUSE_ATTRIBUTE_NO_BUILTIN; do
echo -n $cc - $opt ""
if $cc -O -Wall -m64 $opt -S -o- sc.c | grep -q sincos; then
echo "BAD"
else
echo "GOOD"
fi
done
echo
done

this inspects the assembly for having the string 'sincos' in it, presumably a
'call sincos'. I got the following output:

4.6 - -ffreestanding GOOD
4.6 - -fno-builtin-sin GOOD
4.6 - -DUSE_PRAGMA_FREESTANDING BAD
4.6 - -DUSE_PRAGMA_NO_BUILTIN BAD
4.6 - -DUSE_ATTRIBUTE_FREESTANDING BAD
4.6 - -DUSE_ATTRIBUTE_NO_BUILTIN BAD

4.7 - -ffreestanding GOOD
4.7 - -fno-builtin-sin GOOD
4.7 - -DUSE_PRAGMA_FREESTANDING BAD
4.7 - -DUSE_PRAGMA_NO_BUILTIN BAD
4.7 - -DUSE_ATTRIBUTE_FREESTANDING BAD
4.7 - -DUSE_ATTRIBUTE_NO_BUILTIN BAD

4.8 - -ffreestanding GOOD
4.8 - -fno-builtin-sin GOOD
4.8 - -DUSE_PRAGMA_FREESTANDING BAD
4.8 - -DUSE_PRAGMA_NO_BUILTIN BAD
4.8 - -DUSE_ATTRIBUTE_FREESTANDING BAD
4.8 - -DUSE_ATTRIBUTE_NO_BUILTIN BAD

4.9 - -ffreestanding GOOD
4.9 - -fno-builtin-sin GOOD
4.9 - -DUSE_PRAGMA_FREESTANDING BAD
4.9 - -DUSE_PRAGMA_NO_BUILTIN BAD
4.9 - -DUSE_ATTRIBUTE_FREESTANDING BAD
4.9 - -DUSE_ATTRIBUTE_NO_BUILTIN BAD

Additionally, the #pragma and __attribute__ directives suggested trigger
warnings under clang 3.3, 3.4 and 3.5 (-Wunknown-pragmas and
-Wunknown-attributes / -Wattributes). However, no version of clang that
I tested performed the transformation to sincos either.

Here's another alternative that seems to work everywhere:
#ifdef USE_ASM_TRICKERY
extern double sin_(double) __asm__("sin");
#define sin sin_
#endif
I can't say I'm thrilled about this one either.

I'd love to hear what anybody else thinks is the best choice in this
mess. In particular, can anybody identify a problem with Michael's
original suggestion to add
-fno-builtin-sincos -fno-builtin-sin -fno-builtin-cos
(or alternately to add -ffreestanding) to the realtime build flags?

(for i386 and for uspace, the flag is not needed afaik; that's the core
of why I'm reluctant to apply it unconditionally, I think)

Jeff

[andy pugh]

Post by Jeff Epler
I'd love to hear what anybody else thinks is the best choice in this
mess.

I suspect many of us are too out of our depth to even opinionate.

Is this likely to get good answers from gcc forums or similar?

[Jeff Epler]

Post by andy pugh
Is this likely to get good answers from gcc forums or similar?

That idea's crazy enough that it just might work. We'll find out.

http://thread.gmane.org/gmane.comp.gcc.help/47496

Jeff

[W. Martinjak]

Hi Jeff,

Post by Jeff Epler

Post by andy pugh
Is this likely to get good answers from gcc forums or similar?

That idea's crazy enough that it just might work. We'll find out.
http://thread.gmane.org/gmane.comp.gcc.help/47496

I was curious and have toyed.
I've tried some variations of this attributes.


__attribute__((__optimize__("no-builtin-cos,no-builtin-sin")))
__attribute__((__optimize__("no-builtin-sin")))
__attribute__((__optimize__("no-builtin")))
__attribute__((__optimize__("no-builtin-sensless")))

no error


__attribute__((__optimize__("no-builtin-")))

error: missing argument to ‘-fno-builtin-’):


__attribute__((__optimize__("no-builtin_")))

error: unrecognized command line option ‘-fno-builtin_’


__attribute__((__optimize__("no-builtin_sin")))

error: unrecognized command line option ‘-fno-builtin_sin’


Very funny, it seems there is a check if the -f option exists but then it's ignored.

Maybe it is only working for "real" optimization options.
In the gcc man page the no-builtin options are mentioned under "C Language Options".

Hope it helps a little bit.

Matsche

Post by Jeff Epler
Jeff
------------------------------------------------------------------------------
Meet PCI DSS 3.0 Compliance Requirements with EventLog Analyzer
Achieve PCI DSS 3.0 Compliant Status with Out-of-the-box PCI DSS Reports
Are you Audit-Ready for PCI DSS 3.0 Compliance? Download White paper
Comply to PCI DSS 3.0 Requirement 10 and 11.5 with EventLog Analyzer
http://p.sf.net/sfu/Zoho
_______________________________________________
Emc-developers mailing list
https://lists.sourceforge.net/lists/listinfo/emc-developers

[andy pugh]

Post by Jeff Epler
I believe that what happens is that gcc analyzes the function
void sincos(double x, double *sx, double *cx)
{
*sx = sin(x);
*cx = cos(x);
}
and determines that it is equivalent to the call
sincos(x, sx, cx);

I assume that the compiler is clever enough to spot that
void sincos(double x, double *sx, double *cx)
{
*cx = cos(x);
*sx = sin(x);
}

is _also_ sincos?

I can imagine "breaking" the equivalence by combinations of tans...