package/collectd: Specify correct FP layout for PowerPC
PATCH 1/1 package/collectd: Specify correct FP layout for PowerPC
PowerPC stores floating point as big endian, so, in order for 'network' plugin to work correctly (and potentially any user of htond() in collectd's codebase), --with-fp-layout=endianflip as opposed to --with-fp-layout=nothing needs to be specified during configuration phase.
--- package/collectd/collectd.mk | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
package/collectd/collectd.mk +7 -1
@@ -24,9 +24,15 @@ COLLECTD_PLUGINS_DISABLE = \ COLLECTD_CONF_ENV += LIBS="-lm" +ifeq ($(BR2_powerpc)$(BR2_powerpc64)$(BR2_powerpc64le),y) +COLLECTD_FP_LAYOUT=endianflip +else +COLLECTD_FP_LAYOUT=nothing +endif + COLLECTD_CONF_OPTS += \ --with-nan-emulation \ - --with-fp-layout=nothing \ + --with-fp-layout=$(COLLECTD_FP_LAYOUT) \ --with-perl-bindings=no \ $(foreach p, $(COLLECTD_PLUGINS_DISABLE), --disable-$(p)) \ $(if $(BR2_PACKAGE_COLLECTD_AGGREGATION),--enable-aggregation,--disable-aggregation) \
Signature · 1 line
2.13.6
PATCH 1/1 package/collectd: Specify correct FP layout for PowerPC
Hello,
Quoted 16 lines · Andrey Smirnov
PowerPC stores floating point as big endian, so, in order for 'network' plugin to work correctly (and potentially any user of
htond()in collectd's codebase), --with-fp-layout=endianflip as opposed to --with-fp-layout=nothing needs to be specified during configuration phase.Signed-off-by Andrey Smirnov <[email protected]>--- package/collectd/collectd.mk | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)package/collectd/collectd.mk +0
-0
package/collectd/collectd.mk COLLECTD_PLUGINS_DISABLE = \ · line 31
Quoted 8 lines
package/collectd/collectd.mk +5
-0@@ -24,9 +24,15 @@ COLLECTD_PLUGINS_DISABLE = \ COLLECTD_CONF_ENV += LIBS="-lm" +ifeq ($(BR2_powerpc)$(BR2_powerpc64)$(BR2_powerpc64le),y) +COLLECTD_FP_LAYOUT=endianflip +else +COLLECTD_FP_LAYOUT=nothing +endif
I am not sure that the problem is PowerPC vs. rest of the world, but rather big endian vs. little endian. For example, OpenWRT is using endianflip on all big endian platforms, and "nothing" on all little endian platforms:
https://dev.openwrt.org/browser/packages/utils/collectd/Makefile?rev=28960.
So I believe the correct patch is to use:
ifeq ($(BR2_ENDIAN),"BIG")COLLECTD_FP_LAYOUT = endianflip
elseCOLLECTD_FP_LAYOUT = normal
endif
However, there is a third FP layout called "intswap", and I'm not sure in which situations it should be used. Some additional investigation of collectd configure.ac script is probably needed to understand the AC_RUN_IFELSE() tests it does to figure out what the FP layout is when doing a native build.
Best regards,
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
PATCH 1/1 package/collectd: Specify correct FP layout for PowerPC
Quoted 34 lines · Thomas Petazzoni
Hello,
Quoted 16 lines · Andrey Smirnov
PowerPC stores floating point as big endian, so, in order for 'network' plugin to work correctly (and potentially any user of
htond()in collectd's codebase), --with-fp-layout=endianflip as opposed to --with-fp-layout=nothing needs to be specified during configuration phase.Signed-off-by Andrey Smirnov <[email protected]>--- package/collectd/collectd.mk | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)package/collectd/collectd.mk +0
-0package/collectd/collectd.mk COLLECTD_PLUGINS_DISABLE = \ · line 31
Quoted 8 lines
package/collectd/collectd.mk +5
-0@@ -24,9 +24,15 @@ COLLECTD_PLUGINS_DISABLE = \ COLLECTD_CONF_ENV += LIBS="-lm" +ifeq ($(BR2_powerpc)$(BR2_powerpc64)$(BR2_powerpc64le),y) +COLLECTD_FP_LAYOUT=endianflip +else +COLLECTD_FP_LAYOUT=nothing +endifI am not sure that the problem is PowerPC vs. rest of the world, but rather big endian vs. little endian. For example, OpenWRT is using endianflip on all big endian platforms, and "nothing" on all little endian platforms:
https://dev.openwrt.org/browser/packages/utils/collectd/Makefile?rev=28960.
Quoted 8 lines
So I believe the correct patch is to use:
ifeq ($(BR2_ENDIAN),"BIG")
COLLECTD_FP_LAYOUT= endianflip
elseCOLLECTD_FP_LAYOUT= normal
endif
Yeah, that sounds right. I implemented it PPC-only just because it was the only use-case I could test, but if OpenWRT is doing that for every big-endian arch, then it makes sens to change my patch to do the same.
Quoted 6 lines
However, there is a third FP layout called "intswap", and I'm not sure in which situations it should be used. Some additional investigation of collectd configure.ac script is probably needed to understand the
AC_RUN_IFELSE()tests it does to figure out what the FP layout is when doing a native build.
I looked at that part of config.ac when I was debugging the problem and AFACT all "intswap()" does is swapping upper and lower 32-bits of a 64-bit word. I can't think of an architecture that would require such a transformation and commit that introduced that check in collectd didn't have any helpful hints either. Can we limit the scope of this fix to big vs. little endian the way you suggested and leave the "intswap" case out?
Thanks,
Andrey Smirnov
PATCH 1/1 package/collectd: Specify correct FP layout for PowerPC
Hello,
Quoted 10 lines · Andrey Smirnov
Quoted 6 lines
ifeq ($(BR2_ENDIAN),"BIG")
COLLECTD_FP_LAYOUT= endianflip
elseCOLLECTD_FP_LAYOUT= normal
endifYeah, that sounds right. I implemented it PPC-only just because it was the only use-case I could test, but if OpenWRT is doing that for every big-endian arch, then it makes sens to change my patch to do the same.
Right, but your patch makes ppc64le use endianflip, which isn't correct.
Quoted 7 lines
I looked at that part of config.ac when I was debugging the problem and AFACT all "
intswap()" does is swapping upper and lower 32-bits of a 64-bit word. I can't think of an architecture that would require such a transformation and commit that introduced that check in collectd didn't have any helpful hints either. Can we limit the scope of this fix to big vs. little endian the way you suggested and leave the "intswap" case out?
Since OpenWRT doesn't use it either, I agree we will leave this case on the side, and see if anyone complains about it.
Could you submit an updated patch?
Thanks!
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
PATCH 1/1 package/collectd: Specify correct FP layout for PowerPC
In reply to Andrey SmirnovQuoted 6 lines · Andrey Smirnov
Quoted 6 lines
However, there is a third FP layout called "intswap", and I'm not sure in which situations it should be used. Some additional investigation of collectd configure.ac script is probably needed to understand the
AC_RUN_IFELSE()tests it does to figure out what the FP layout is when doing a native build.
Quoted 7 lines
I looked at that part of config.ac when I was debugging the problem and AFACT all "
intswap()" does is swapping upper and lower 32-bits of a 64-bit word. I can't think of an architecture that would require such a transformation and commit that introduced that check in collectd didn't have any helpful hints either. Can we limit the scope of this fix to big vs. little endian the way you suggested and leave the "intswap" case out?
That would be old ARM FP which had mixed-endian doubles? I can't find a good
reference to it, just [1]. I remember from years ago that on ARM I had to do some weird word-swapping when converting doubles to/from network byte order.
Regards,
Arnout
[1] https://en.wikipedia.org/wiki/Endianness#Floating_point
Signature · 6 lines
Arnout Vandecappelle arnout at mind be Senior Embedded Software Architect +32-16-286500 Essensium/Mind http://www.mind.be G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle GPG fingerprint: 7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF