[llvm] [IR] Initial introduction of memset_pattern (PR #97583)
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 14 05:15:34 PDT 2024
nikic wrote:
> > An alternative we should probably consider here is to not have a separate memset.pattern at all, and instead have memset be memset.pattern. If we're going to add a type overload to memset anyway for non-8-bit targets, there's probably not a lot of point to having a separate memset.pattern?
>
> I agree unifying could make sense in the future and could be done either by increasing the scope of memset.pattern to subsume memset, or by modifying memset so it subsumes memset.pattern. Are you happy to defer this to the future or do you see implementing such a unification as a blocker for this initial introduction? My assumption so far has been that introducing this intrinsic as an incremental step is likely better due to being non-invasive to existing users and frontends, even if we later modify memset to make memset.pattern redundant.
Not sure. My thinking here is that you are already adding memset_pattern as part of MemSetInst, so from a C++ API perspective, they become essentially the same thing, and we already have to review all the MemSetInst code to make sure it works with the "pattern" semantics. And I do think that adding it to MemSetInst is the right thing to do. I think the memset lowering code also ends up being exactly the same between memset/memset_pattern, as the distinction is all in the store lowering. So overall, it may be simpler to implement this by adding an extra overload to memset, rather than adding a new intrinsic. I don't want to hold this PR up forever either though :)
https://github.com/llvm/llvm-project/pull/97583
More information about the llvm-commits
mailing list