-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[IR] Make @llvm.memset prototype byte width dependent #106537
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
base: users/s-barannikov/byte/1-datalayout
Are you sure you want to change the base?
[IR] Make @llvm.memset prototype byte width dependent #106537
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
nikic
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is the right direction for memset. I would prefer actually overloading it and making the memset/memset.pattern distinction redundant. See also #97583 (comment) and following comments.
Thanks for the link. I'm in favor of the RFC and in fact I considered this approach, too. It will take long time for the llvm.memset to be replaced with llvm.memset.pattern though, and until then we need to get llvm.memset working with non-8-bit bytes somehow. The suggested approach requires minimum changes, is backward compatible, and follows the semantics specified by the LangRef:
That said, I can keep this patch downstream if you think it does something wrong. |
98c3676 to
31ee84f
Compare
f78b4f6 to
edcea1e
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
edcea1e to
4cee863
Compare
31ee84f to
b46d554
Compare
4cee863 to
65a421f
Compare
b46d554 to
4048990
Compare
65a421f to
97cce99
Compare
4048990 to
2488060
Compare
97cce99 to
0bf73e3
Compare
2488060 to
3539d10
Compare
28662b7 to
2080c3e
Compare
This patch changes the type of the value argument of @llvm.memset and similar intrinsics from i8 to iN, where N is the byte width specified in data layout string. Note that the argument still has fixed type (not overloaded), but type checker will complain if the type does not match the byte width. Ideally, the type of the argument would be dependent on the address space of the pointer argument. It is easy to do this (and I did it downstream as a PoC), but since data layout string doesn't currently allow different byte widths for different address spaces, I refrained from doing it now.
2080c3e to
5332872
Compare
3539d10 to
670e94d
Compare

This patch changes the type of the value argument of @llvm.memset and
similar intrinsics from i8 to iN, where N is the byte width specified
in data layout string.
Note that the argument still has fixed type (not overloaded), but type
checker will complain if the type does not match the byte width.
Ideally, the type of the argument would be dependent on the address
space of the pointer argument. It is easy to do this (and I did it
downstream as a PoC), but since data layout string doesn't currently
allow different byte widths for different address spaces, I refrained
from doing it now.