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

[RISC-V] Make mem* functions always available #532

Merged
merged 1 commit into from
Sep 17, 2024

Conversation

hvdijk
Copy link
Collaborator

@hvdijk hvdijk commented Sep 17, 2024

Overview

[RISC-V] Make mem* functions always available

Reason for change

When we link in builtin functions, we attempted to detect when we need memcpy/memmove/memset to provide them when needed. This detection proved to not be entirely reliable: compiler passes run after linking in the builtin functions are permitted to insert additional references. When running SYCL-CTS, it turned out that function inlining can insert additional references to memcpy for the purpose of copying function arguments which, after inlining, no longer exist as separate function arguments.

Description of change

For non-RISC-V targets, we already unconditionally provide these functions in host::utils::getRelocations(), so it seems reasonable to just always provide them for RISC-V too.

Anything else we should know?

This also allows us to avoid some complexity in the definition of memmove that was only there to deal with the situation where memcpy might be unavailable. As that can no longer happen, that no longer needs to be taken into account.

Checklist

  • Read and follow the project Code of Conduct.
  • Make sure the project builds successfully with your changes.
  • Run relevant testing locally to avoid regressions.
  • Run clang-format-17 on all modified code.

@hvdijk hvdijk force-pushed the riscv-mem-functions branch 2 times, most recently from 534d42a to bddb9a3 Compare September 17, 2024 10:57
When we link in builtin functions, we attempted to detect when we need
memcpy/memmove/memset to provide them when needed. This detection proved
to not be entirely reliable: compiler passes run after linking in the
builtin functions are permitted to insert additional references. When
running SYCL-CTS, it turned out that function inlining can insert
additional references to memcpy for the purpose of copying function
arguments which, after inlining, no longer exist as separate function
arguments.

For non-RISC-V targets, we already unconditionally provide these
functions in host::utils::getRelocations(), so it seems reasonable to
just always provide them for RISC-V too.

This also allows us to avoid some complexity in the definition of
memmove that was only there to deal with the situation where memcpy
might be unavailable. As that can no longer happen, that no longer needs
to be taken into account.
@hvdijk hvdijk merged commit ecf7503 into uxlfoundation:main Sep 17, 2024
3 checks passed
@hvdijk hvdijk deleted the riscv-mem-functions branch September 17, 2024 16:02
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