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

v8: add additional include dirs on AIX #289

Closed
wants to merge 2 commits into from
Closed

Conversation

abmusse
Copy link

@abmusse abmusse commented Sep 9, 2024

On AIX, we now include src/wasm/float16.h from within src/utils/utils.h and src/wasm/float16.h includes additional header files.

For reference this patch added the changes.

Richard ran a test build last week (updated to V8 13.0.177) and it failed with:

13:16:49 In file included from ../deps/v8/src/wasm/float16.h:9,
13:16:49                  from ../deps/v8/src/utils/utils.h:31,
13:16:49                  from ../deps/v8/src/utils/memcopy.h:17,
13:16:49                  from ../deps/v8/src/zone/zone-chunk-list.h:9,
13:16:49                  from ../deps/v8/src/codegen/maglev-safepoint-table.h:14,
13:16:49                  from ../deps/v8/src/objects/code.h:8,
13:16:49                  from ../deps/v8/src/objects/visitors.h:10,
13:16:49                  from ../deps/v8/src/snapshot/serializer-deserializer.h:8,
13:16:49                  from ../deps/v8/src/snapshot/snapshot.h:14,
13:16:49                  from /home/iojs/build/ws/out/Release/obj.target/v8_snapshot/geni/snapshot.cc:7:
13:16:49 ../deps/v8/third_party/fp16/src/include/fp16.h:5:10: fatal error: fp16/fp16.h: No such file or directory
13:16:49     5 | #include <fp16/fp16.h>
13:16:49       |          ^~~~~~~~~~~~~
13:16:49 compilation terminated.
13:16:49 gmake[2]: *** [Makefile:266: /home/iojs/build/ws/out/Release/obj.target/v8_snapshot/geni/snapshot.o] Error 1
13:16:49 gmake[2]: *** Waiting for unfinished jobs....
13:16:49 In file included from ../deps/v8/src/wasm/float16.h:9,
13:16:49                  from ../deps/v8/src/utils/utils.h:31,
13:16:49                  from ../deps/v8/src/objects/field-index.h:11,
13:16:49                  from ../deps/v8/src/objects/objects.h:26,
13:16:49                  from ../deps/v8/src/objects/tagged-value.h:8,
13:16:49                  from ../deps/v8/src/objects/tagged-field.h:13,
13:16:49                  from ../deps/v8/src/objects/heap-object.h:13,
13:16:49                  from ../deps/v8/src/objects/fixed-array.h:12,
13:16:49                  from ../deps/v8/src/objects/contexts.h:9,
13:16:49                  from ../deps/v8/src/execution/thread-local-top.h:14,
13:16:49                  from ../deps/v8/src/execution/isolate-data.h:12,
13:16:49                  from ../deps/v8/src/execution/isolate.h:31,
13:16:49                  from ../deps/v8/src/init/setup-isolate-deserialize.cc:6:
13:16:49 ../deps/v8/third_party/fp16/src/include/fp16.h:5:10: fatal error: fp16/fp16.h: No such file or directory
13:16:49     5 | #include <fp16/fp16.h>
13:16:49       |          ^~~~~~~

We need to update the include dirs on AIX/IBM i to find these headers.

Update Test Build: https://ci.nodejs.org/job/node-test-commit-aix/nodes=aix72-ppc64/53642/console

CC

@richardlau
@targos

@abmusse abmusse changed the title aix: set include_dirs to find required header files v8: set include_dirs to find required header files on AIX Sep 9, 2024
@abmusse abmusse changed the title v8: set include_dirs to find required header files on AIX v8: add additional include dirs on AIX Sep 9, 2024
tools/v8_gypfiles/v8.gyp Outdated Show resolved Hide resolved
@abmusse
Copy link
Author

abmusse commented Sep 10, 2024

@targos
I ran a test build with your changes
https://ci.nodejs.org/job/node-test-commit-aix/nodes=aix72-ppc64/53672/console

Looks like it was successful!

I will now rebase my commits not on top of the new changes

abmusse and others added 2 commits September 10, 2024 16:55
On AIX, we now include src/wasm/float16.h from within src/utils/utils.h
and src/wasm/float16.h includes additional header files.
@abmusse
Copy link
Author

abmusse commented Sep 11, 2024

@targos
Copy link
Member

targos commented Sep 11, 2024

Thanks @abmusse. I added the changes to the canary-base branch in nodejs/node@eea66ff.
It will be picked up by the next canary update.

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.

3 participants