[PATCH] D86025: [CodeGen] Respect libfunc availability when lowering intrinsic memcpy

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 17 15:11:10 PDT 2020


arsenm added a comment.

In D86025#2222691 <https://reviews.llvm.org/D86025#2222691>, @aemerson wrote:

> In D86025#2222500 <https://reviews.llvm.org/D86025#2222500>, @arsenm wrote:
>
>> In D86025#2222453 <https://reviews.llvm.org/D86025#2222453>, @mstorsjo wrote:
>>
>>>> GlobalISel should be able to handle arbitrary memcpys with ease since it's possible to emit a loop. With SelectionDAG you have to expand the arbitrary case in the IR, but it still handles constant size cases.
>>>
>>> Ok, so where would I start out doing that, more concretely? (I have close to no experience in either transforms that change IR or GlobalISel.)
>>
>> The actual expansion code (at least in the non-loop case) is already implemented, it's just in the wrong place. Currently the memory intrinsics can be expanded by the code in CombinerHelper::optimizeMemcpy/optimizeMemmove/optimizeMemset, and treat this as an optimization. I think this should be treated as a legalization decision, and moved into LegalizerHelper. LegalizerHelper::lower would then dispatch to a lowerMemcpy(), which would look similar to optimizeMemcpy today. After that, more code would be needed for the loop cases. I'll probably get to this in a month or two, but if you want to take care of it that would be great.
>
> We can discuss this in a separate GISel thread, but I don't see how memcpy inlining is any more of an instruction legality decision than an optimization one. I'm not strongly against it, but IMO it's no more correct than a combine.

Because whether or not emitting a library call is a valid strategy depends on the target. An inlined memcpy may not be optional, in which case it is not an optimization. The legalizer code would essentially be identical to the combiner code. The combiner could also do the same thing, but these should share code.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86025/new/

https://reviews.llvm.org/D86025



More information about the llvm-commits mailing list