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

full_fidelity_syntax_type.ml no longer builds on Aarch64 hosts #8469

Closed
swalk-cavium opened this issue Mar 21, 2019 · 18 comments
Closed

full_fidelity_syntax_type.ml no longer builds on Aarch64 hosts #8469

swalk-cavium opened this issue Mar 21, 2019 · 18 comments
Labels
arm64 community supported Unsupported architecture, operating system, or distribution external

Comments

@swalk-cavium
Copy link
Contributor

HHVM Version

First noticed at this commit
swalk@lab-7:~/extra/play.031919a/hhvm$ git branch -v

  • master d9c334e Rename Local_helpers.scope_with_handler to Scope.with_unnamed_locals

Operating System and Version

swalk@lab-7:~/extra/play.031919a/hhvm$ uname -a
Linux lab-7 4.4.0-138-generic #164-Ubuntu SMP Tue Oct 2 17:15:48 UTC 2018 aarch64 aarch64 aarch64 GNU/Linux

Standalone code, or other way to reproduce the problem

Part of the build log

17308 gcc.compile.c++ bin.v2/libs/wave/build/gcc-5.4.0/release/link-static/threading-multi/instantiate_defined_grammar.o
17309 + ocamlfind ocamlopt -c -ccopt '-DUSE_UNISTD=1' -g -w A -package core_kernel -package visitors.ppx -package visitors.runtime -package pcre -package l wt -package lwt.unix -package lwt_ppx -package lwt_log -w -3-4-6-29-35-44-48-50-52 -package unix -package str -package bigarray -package ppx_deriving .std -I src/parser -I src/search -I src/dfind -I src/typing -I src/annotated_ast -I src/deps -I src/naming -I src/diff -I src/globals -I src/decl -I src/pocket_universes -I src/facts -I src/server -I src/hackfmt -I src/watchman -I src/fsnotify_linux -I src/errors -I src/find -I src/monitor -I src/ config -I src/procs -I src/utils -I src/heap -I src/options -I src/libancillary -I src/ast -I src/hhbc -I src/ide_rpc -I src/stubs -I src/socket -I s rc/common -I src/watchman_event_watcher -I src/hhi -I src/client -I src/search/signatures -I src/typing/tast_check -I src/typing/nast_check -I src/ha ckfmt/debug -I src/hackfmt/error -I src/utils/config_file -I src/utils/process -I src/utils/collections -I src/utils/sys -I src/utils/parent -I src/u tils/hashlib -I src/utils/hg -I src/utils/lint -I src/utils/disk -I src/utils/hh_json -I src/injection/default_injector -I src/stubs/logging -I src/t hird-party/avl -I src/third-party/inotify -I src/third-party/core -I src/parser/ppl -I src/parser/smart_constructors -I src/parser/coroutine -I src/p arser/schema -o src/parser/full_fidelity_syntax_type.cmx src/parser/full_fidelity_syntax_type.ml
17310 ocamlfind: [WARNING] Package `threads': Linking problems may arise because of the missing -thread or -vmthread switch
17311 /tmp/camlasmfb4f89.s: Assembler messages:
17312 /tmp/camlasmfb4f89.s:8150: Error: immediate out of range //<<--- HERE
17313 File "src/parser/full_fidelity_syntax_type.ml", line 1:
17314 Error: Assembler error, input left in file /tmp/camlasmfb4f89.s
17315 Command exited with code 2.

Shows immediate out of range

8146 movz x7, #3, lsl #0
8147 str x7, [x5, #8]
8148 ldr x12, [sp, #8]
8149 str x12, [x5, #16]
8150 .L232: sub x27, x27, #5688 //<<--- HERE (only 12-bits in encoding - unsigned)
8151 cmp x27, x28
8152 add x1, x27, #8
8153 b.lo .L233
8154 movz x8, #11, lsl #16

Actual result

Build fails

Expected result

Build pass.

@fredemmott fredemmott added arm64 community supported Unsupported architecture, operating system, or distribution external labels Mar 21, 2019
@fredemmott
Copy link
Contributor

Is there an issue filed with ocaml, or a more recent version that works?

@swalk-cavium
Copy link
Contributor Author

@fredemmott - I haven't looked yet. This looks similar to the issue from last year when we had to make a patch to the ocaml code gen.

@swalk-cavium
Copy link
Contributor Author

Some recent change is downloading a newer version of the ocaml language,
but the patching infrastructure built in the cmake scripts wasn't considered.

swalk@lab-7:~/extra/play.032219a/hhvm$ find . -type f -print |& grep arm64 | grep emit | xargs /usr/bin/sum
25012 37 ./third-party/ocaml/src/asmcomp/arm64/emit.mlp
26280 37 ./hphp/hack/_build/.opam/hack-switch/.opam-switch/sources/ocaml-base-compiler.4.05.0/asmcomp/arm64/emit.mlp

@fredemmott
Copy link
Contributor

cc @vsiles

@swalk-cavium swalk-cavium changed the title full_fidelity_syntax_type.ml no longer build on Aarch64 hosts full_fidelity_syntax_type.ml no longer builds on Aarch64 hosts Mar 22, 2019
@vsiles
Copy link

vsiles commented Mar 22, 2019

Indeed that rings a bell. I'll check on a way to patch ocaml during the opam switch installation (or directly bundle a patched version, it might even be easier)

@vsiles
Copy link

vsiles commented Mar 22, 2019

@fredemmott the patch seems to be in more recent ocaml version. Out of curiosity while I read opam's doc, are there hard reasons not to update the ocaml version ?

@fredemmott
Copy link
Contributor

It should be the same as the internal version - if updating the internal version is fine, go ahead :)

@vsiles
Copy link

vsiles commented Mar 22, 2019

@vassilmladenov I'll write about that next week :)

@vsiles
Copy link

vsiles commented Mar 23, 2019

@swalk-cavium I don't have an arm64 to test this, so if you have the opportunity, could you test the build after changing https://github.com/facebook/hhvm/blob/master/hphp/hack/opam_setup.sh#L56 to 4.07.0 ? I just want to know if upgrading is a viable option.

@swalk-cavium
Copy link
Contributor Author

@vsiles - A different problem is exposed with this change. The numbers on the left are
line numbers into the build output.

17437 boot/ocamlrun ./ocamlopt -g -nostdlib -I stdlib -I otherlibs/dynlink -strict-sequence -principal -absname -w +a-4-9-41-42-44-45-48 -warn-error A -bin-annot -safe-string -strict-formats -I utils -I parsing -I typing -I byte comp -I middle_end -I middle_end/base_types -I asmcomp -I driver -I toplevel -c asmcomp/compilenv.ml
17438 File "/mnt/diskb/swalk/play.032519a/hhvm/third-party/ocaml/src/none", line 1:
17439 Warning 58: no cmx file was found in path for module Compilenv, and its interface was not compiled with -opaque
17440 File "/mnt/diskb/swalk/play.032519a/hhvm/third-party/ocaml/src/asmcomp/selection.ml", line 1:
17441 Error: Some fatal warnings were triggered (1 occurrences)
17442 Makefile:1270: recipe for target 'asmcomp/selection.cmx' failed
17443 make[5]: *** [asmcomp/selection.cmx] Error 2

It appears that there is some other fix in the 4.07.0 version for GOT related symbols.

@vsiles
Copy link

vsiles commented Mar 25, 2019

Thank you for that. I'd make a PR to fix the 4.05.0 asap

@swalk-cavium
Copy link
Contributor Author

@vsiles - I'm not sure how to interpret your comment. Are you making the fix? If not,
how are patches to be applied after the opam has pulled in new packages?

@vsiles
Copy link

vsiles commented Mar 26, 2019

It's because my english sucks :D I have a fix in mind: it is possible to apply some patch using opam before it builds the package, so I'll be able to run the patched version + opam

@vsiles
Copy link

vsiles commented Mar 28, 2019

@swalk-cavium See #8474 . As I don't have ARM64 hw to effectively test it, could you give it a go ?

@swalk-cavium
Copy link
Contributor Author

@vsiles - Yes. I can do that.

@swalk-cavium
Copy link
Contributor Author

swalk@lab-7:~/extra/play.032819a/hhvm$ hphp/hhvm/hhvm --version
HipHop VM 4.1.0-dev (rel)
Compiler: heads/master-0-gd715738b7db0870c3d1b1daafd0980a84064af54
Repo schema: b172210ccdac82abf293712173af6e3080a14365

swalk@lab-7:~/extra/play.032819a/hhvm$ uname -a
Linux lab-7 4.4.0-138-generic #164-Ubuntu SMP Tue Oct 2 17:15:48 UTC 2018 aarch64 aarch64 aarch64 GNU/Linux

@vsiles - This issue can be closed as soon as your PR lands. Thanks for the fix.

@vsiles
Copy link

vsiles commented Apr 4, 2019

@swalk-cavium the PR has landed, can you recheck that everything's alright ? If that's fixed, please close the issue.

@swalk-cavium
Copy link
Contributor Author

@vsiles - aarch64 builds again without having to merge your PR. This issue is resolved. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arm64 community supported Unsupported architecture, operating system, or distribution external
Projects
None yet
Development

No branches or pull requests

3 participants