Skip to content

test: enable pthread-related libc-tests#560

Merged
abrown merged 1 commit intoWebAssembly:mainfrom
kateinoigakukun:pr-7dbd10cc1c8d0643efe3d3a41f2c73e208f0ef0e
Jan 20, 2025
Merged

test: enable pthread-related libc-tests#560
abrown merged 1 commit intoWebAssembly:mainfrom
kateinoigakukun:pr-7dbd10cc1c8d0643efe3d3a41f2c73e208f0ef0e

Conversation

@kateinoigakukun
Copy link
Contributor

@kateinoigakukun kateinoigakukun commented Jan 19, 2025

This change enables the pthread-related tests in the libc-test suite. The tests are enabled only for the wasm32-wasip1-threads target, which is the only target that supports threads at the moment.

The following pthread tests are still disabled:

  • pthread_cancel-points.c
  • pthread_cancel.c
  • pthread_robust.c

This is a preparative change for swiftwasm/swift#5598

test/Makefile Outdated
# Always include the `libc-test` infrastructure headers.
CFLAGS += -I$(INFRA_HEADERS_DIR)
ifneq ($(findstring -threads,$(TARGET_TRIPLE)),)
LDFLAGS += -Wl,--import-memory,--export-memory,--shared-memory,--max-memory=1073741824
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did you find that doing this same thing via the //! add-flags.py(LDFLAGS): ... annotation in the test files didn't work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it should work but I did it in this way as I think those flags are not a property of test cases but a property of the compiler target. Same with the engine flags.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see what you mean. Practically, though, it might be more convenient to move this to the test files since there may be discrepancies between targets; e.g., what if other threads tests don't require setting up 1G of max memory?

Copy link
Collaborator

Choose a reason for hiding this comment

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

And, honestly, the simpler we can make this Makefile, the better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, it makes sense to me 👍

test/Makefile Outdated
ENGINE += --wasm component-model
OBJPAT := $(OBJDIR)/%.component.wasm
else
ENGINE += --wasi threads
Copy link
Collaborator

Choose a reason for hiding this comment

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

Similarly, does the //! add-flags.py(RUN): ... annotation not work here?

@kateinoigakukun kateinoigakukun force-pushed the pr-7dbd10cc1c8d0643efe3d3a41f2c73e208f0ef0e branch from 890d59d to 0e11f56 Compare January 20, 2025 23:14
This change enables the pthread-related tests in the libc-test suite.
The tests are enabled only for the `wasm32-wasip1-threads` target, which
is the only target that supports threads at the moment.
@kateinoigakukun kateinoigakukun force-pushed the pr-7dbd10cc1c8d0643efe3d3a41f2c73e208f0ef0e branch from 0e11f56 to d80cfde Compare January 20, 2025 23:42
Copy link
Collaborator

@abrown abrown left a comment

Choose a reason for hiding this comment

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

Thank you for this!

@abrown abrown merged commit f1c557c into WebAssembly:main Jan 20, 2025
8 checks passed
@kateinoigakukun kateinoigakukun deleted the pr-7dbd10cc1c8d0643efe3d3a41f2c73e208f0ef0e branch January 21, 2025 00:12
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.

2 participants