Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixup architecture woes #151

Merged
merged 3 commits into from
Jan 20, 2017
Merged

Fixup architecture woes #151

merged 3 commits into from
Jan 20, 2017

Conversation

staticfloat
Copy link
Contributor

@staticfloat staticfloat commented Jan 16, 2017

  • Significantly cleanup ARCH handling. Previously, behavior differed if the same value of ARCH was defined within Make.inc or defined on the command line. This made me very sad.

  • Fix arm floating-point status register code so that the tests actually work.

  • Add testing for ppc64le and arm architectures on Travis through the magic of binfmt-support and qemu.

I would like to publicly thank @yuyichao for his help in debugging the arm code.

#else
#define __rfs(__fpsr)
#define __rfs(__fpsr) (*__fpsr = 0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory this should be (*(__fpsr) = 0) =)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And in theory if __wfs is called with non-zero in sf buid the functions below should return non-zero to indicate a failure.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we're not targeting softfloat, I don't think we should bother with too much work to fix it. I'm happy to let it compile and make __rfs() reasonable.

@ginggs
Copy link
Contributor

ginggs commented Jan 16, 2017

I tested building a snapshot of the sf/archmageddon branch on the Ubuntu PPA builders.
Builds and tests were successful on amd64, i386, ppc64el and for the first time on armhf!

Unfortunately, the arm64 (aarch64) build now fails while linking the tests.

@staticfloat
Copy link
Contributor Author

@ginggs could you share the error that you get on AArch64?

@ginggs
Copy link
Contributor

ginggs commented Jan 16, 2017

make[1]: Leaving directory '/<<BUILDDIR>>/openlibm-0.5.4+armageddon+dfsg'
   dh_auto_test -a -O--parallel
	make -j4 test
make[1]: Entering directory '/<<BUILDDIR>>/openlibm-0.5.4+armageddon+dfsg'
make -C test test-double
make -C test test-float
make[2]: Entering directory '/<<BUILDDIR>>/openlibm-0.5.4+armageddon+dfsg/test'
make[2]: Entering directory '/<<BUILDDIR>>/openlibm-0.5.4+armageddon+dfsg/test'
gcc -Wdate-time -D_FORTIFY_SOURCE=2 -g -O2 -fdebug-prefix-map=/<<BUILDDIR>>/openlibm-0.5.4+armageddon+dfsg=. -fstack-protector-strong -Wformat -Werror=format-security  -Wl,-Bsymbolic-functions -Wl,-z,relro -Wl,-z,now  test-float.c -D__BSD_VISIBLE -I ../include -I../src -L.. -lopenlibm -Wl,-rpath=/<<BUILDDIR>>/openlibm-0.5.4+armageddon+dfsg -o test-float
gcc -Wdate-time -D_FORTIFY_SOURCE=2 -g -O2 -fdebug-prefix-map=/<<BUILDDIR>>/openlibm-0.5.4+armageddon+dfsg=. -fstack-protector-strong -Wformat -Werror=format-security  -Wl,-Bsymbolic-functions -Wl,-z,relro -Wl,-z,now  test-double.c -D__BSD_VISIBLE -I ../include -I../src -L.. -lopenlibm -Wl,-rpath=/<<BUILDDIR>>/openlibm-0.5.4+armageddon+dfsg -o test-double
../libopenlibm.so: undefined reference to `_ItL_pi_lo'
../libopenlibm.so: undefined reference to `__kernel_cosl'
../libopenlibm.so: undefined reference to `powl'
../libopenlibm.so: undefined reference to `coshl'
../libopenlibm.so: undefined reference to `logl'
../libopenlibm.so: undefined reference to `_ItL_pS1'
../libopenlibm.so: undefined reference to `_ItL_pS3'
../libopenlibm.so: undefined reference to `_ItL_pS5'
../libopenlibm.so: undefined reference to `sinhl'
../libopenlibm.so: undefined reference to `_ItL_qS1'
../libopenlibm.so: undefined reference to `_ItL_qS5'
../libopenlibm.so: undefined reference to `_ItL_qS3'
../libopenlibm.so: undefined reference to `expl'
../libopenlibm.so: undefined reference to `_ItL_atanlo'
../libopenlibm.so: undefined reference to `lgammal_r'
../libopenlibm.so: undefined reference to `__kernel_sinl'
../libopenlibm.so: undefined reference to `_ItL_pS0'
../libopenlibm.so: undefined reference to `_ItL_pS2'
../libopenlibm.so: undefined reference to `_ItL_pS4'
../libopenlibm.so: undefined reference to `_ItL_pS6'
../libopenlibm.so: undefined reference to `_ItL_aT'
../libopenlibm.so: undefined reference to `_ItL_atanhi'
../libopenlibm.so: undefined reference to `_ItL_qS2'
../libopenlibm.so: undefined reference to `_ItL_qS4'
../libopenlibm.so: undefined reference to `__kernel_tanl'
collect2: error: ld returned 1 exit status
../libopenlibm.so: undefined reference to `_ItL_pi_lo'
../libopenlibm.so: undefined reference to `__kernel_cosl'
../libopenlibm.so: undefined reference to `powl'
../libopenlibm.so: undefined reference to `coshl'
../Makefile:15: recipe for target 'test-double' failed
libopenlibm.so: undefined referencemake[2]: *** [test-double] Error 1
 to `logl'
../libopenlibm.so: undefined reference to `_ItL_pS1'
..make[2]: Leaving directory '/<<BUILDDIR>>/openlibm-0.5.4+armageddon+dfsg/test'
/libopenlibm.so: undefined reference to `_ItL_pS3'
../libopenlibm.so: undefined reference to `_ItL_pS5'
../libopenlibm.so: undefined reference to `sinhl'
../libopenlibm.so: undefined reference to `_ItL_qS1'
../libopenlibm.so: undefined reference to `_ItL_qS5'
../libopenlibm.so: undefined reference to `_ItL_qS3'
../libopenlibm.so: undefined reference to `expl'
../libopenlibm.so: undefined reference to `_ItL_atanlo'
../libopenlibm.so: undefined reference to `lgammal_r'
../libopenlibm.so: undefined reference to `__kernel_sinl'
../libopenlibm.so: undefined reference to `_ItL_pS0'
../libopenlibm.so: undefined reference to `_ItL_pS2'
../libopenlibm.so: undefined reference to `_ItL_pS4'
../Makefile:63: recipe for target 'test/test-double' failed
libopenlibm.so: undefined make[1]: *** [test/test-double] Error 2
reference to `_ItL_pS6'
../libopenlibm.so: undefinedmake[1]: *** Waiting for unfinished jobs....
 reference to `_ItL_aT'
../libopenlibm.so: undefined reference to `_ItL_atanhi'
../libopenlibm.so: undefined reference to `_ItL_qS2'
../libopenlibm.so: undefined reference to `_ItL_qS4'
../libopenlibm.so: undefined reference to `__kernel_tanl'
collect2: error: ld returned 1 exit status
Makefile:18: recipe for target 'test-float' failed
make[2]: *** [test-float] Error 1
make[2]: Leaving directory '/<<BUILDDIR>>/openlibm-0.5.4+armageddon+dfsg/test'
Makefile:66: recipe for target 'test/test-float' failed
make[1]: *** [test/test-float] Error 2
make[1]: Leaving directory '/<<BUILDDIR>>/openlibm-0.5.4+armageddon+dfsg'
dh_auto_test: make -j4 test returned exit code 2

@ginggs
Copy link
Contributor

ginggs commented Jan 16, 2017

@staticfloat there you go.

@staticfloat
Copy link
Contributor Author

What are the make invocations that you are using, both to build the library, and to build the tests?

@ginggs
Copy link
Contributor

ginggs commented Jan 16, 2017

make -j4 and make -j4 test

@staticfloat
Copy link
Contributor Author

Can you please post the full build log somewhere? Or link to where the build logs are available on the build service you're using to compile? Also the output of gcc -dumpmachine on your build machine would be useful.

@ginggs
Copy link
Contributor

ginggs commented Jan 16, 2017

I tried attaching the gzipped build log but got the message:

Something went really wrong, and we can’t process that file.

Links to the source package and build logs are temporarily available from this page:

* Previously, behavior differed if the same value of `ARCH` was defined
  within `Make.inc` or defined on the command line.  Don't do that.

* Provide saner defaults for `ARCH` and `MARCH`, and more importantly,
  allow for the proper overriding of both.

* Split `AArch64` code further away from the other `arm` code.
* Use an actual compiler definition to determine whether we have a
  floating-point unit or not.

* Use a modern (VFPU) assembly instruction to get/set the fpsr

* If we don't have an fpsr, at least zero out the return value
@staticfloat
Copy link
Contributor Author

Thank you @ginggs, you found a place in src/Make.files where I hadn't properly updated the new $(ARCH) renaming properly. I wonder to myself why the qemu-based tests didn't have the same problem, but I suppose that's because the cross-compilers act differently than your compilers. In any case, I've pushed a new (rebased) commit set to this branch, feel free to try it out again.

@ginggs
Copy link
Contributor

ginggs commented Jan 17, 2017

@staticfloat thanks! Builds and tests are now successful across the board on amd64, i386, ppc64el, armhf and arm64 (aarch64).

@ViralBShah
Copy link
Member

Good to merge?

@staticfloat
Copy link
Contributor Author

Yes, I think so.

@ViralBShah ViralBShah merged commit ba43b36 into master Jan 20, 2017
@ViralBShah ViralBShah deleted the sf/archmageddon branch January 20, 2017 07:47
mato added a commit to mato/ocaml-freestanding that referenced this pull request Jul 19, 2017
Update to ba43b3616c59bdce38bb1138dbb507b73b2148a4 to get JuliaMath/openlibm#151, needed for aarch64.
mato added a commit to mato/ocaml-freestanding that referenced this pull request Aug 1, 2017
This is to include JuliaMath/openlibm#151 and JuliaMath/openlibm#158 for
arm64 support.

(was Merge commit '16ca29e08c1d4821498ff33f628d3a51bfea4ade' into arm64)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants