This is an archive of the discontinued LLVM Phabricator instance.

[libc] Add aliases to C memory functions for integration tests
ClosedPublic

Authored by jhuber6 on Mar 15 2023, 4:18 PM.

Details

Summary

The integration tests require the C memory functions as the compiler may
emit calls to them directly. The tests normally use the __internal__
variant that is built for testing, but these memory functions were
linked directly to preserve the entrypoint. Instead, we forward delcare
the internal versions and map the entrypoints to them manually inside
the integration test. This allows us to use the internal versions of
these files like the rest of the test objects.

Diff Detail

Event Timeline

jhuber6 created this revision.Mar 15 2023, 4:18 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMar 15 2023, 4:18 PM
jhuber6 requested review of this revision.Mar 15 2023, 4:18 PM
michaelrj added inline comments.Mar 15 2023, 4:39 PM
libc/cmake/modules/LLVMLibCTestRules.cmake
451–452

if these appends are both done unconditionally to the same list they can probably be combined.

jhuber6 added inline comments.Mar 15 2023, 4:44 PM
libc/cmake/modules/LLVMLibCTestRules.cmake
451–452

Soon I'm going to try to add the GPU integration test support, which will be different. I think we should make the former inherit all the dependencies from the STARTUP argument. What do you think @sivachandra?

sivachandra accepted this revision.Mar 15 2023, 5:00 PM
sivachandra added inline comments.
libc/cmake/modules/LLVMLibCTestRules.cmake
451–452

Yes, that would be good. You can still combine them into a single APPEND operation and add some comment about why the memory functions are being added - a modified version of what was present on lines 477 to 480.

libc/test/IntegrationTest/test.cpp
29

Nit: Empty lines between the definitions to improve readability.

This revision is now accepted and ready to land.Mar 15 2023, 5:00 PM
jhuber6 updated this revision to Diff 505664.Mar 15 2023, 5:04 PM

Merging the dependency list and addressing comments. I'll try to remove the other dependencies by picking them up from the startup in a later patch.