[PATCH] D105072: [GISel] Support llvm.memcpy.inline

[Jon Roelofs via Phabricator via llvm-commits]

jroelofs added inline comments.


================
Comment at: llvm/include/llvm/IR/IntrinsicInst.h:895
 
-/// This class wraps the llvm.memcpy intrinsic.
+/// This class wraps the llvm.memcpy and llvm.memcpy.inline intrinsics.
 class MemCpyInst : public MemTransferInst {
----------------
paquette wrote:
> Looks like this is used in a few passes too (e.g. the MemCpyOptimizer pass). Does this change impact those passes at all?
Yes. Maybe I ought to put that in its own diff.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:1357
 
-  unsigned Limit = TLI.getMaxStoresPerMemcpy(OptSize);
+  unsigned Limit = IsInlined ? std::numeric_limits<unsigned>::max()
+                             : TLI.getMaxStoresPerMemcpy(OptSize);
----------------
paquette wrote:
> Probably worth a comment for folks not familiar with the code.
> 
> It'd probably clearer to be able to call a `inlineMemCpy` function here and just return true, but I'm not sure if that's easy right now.
Good point. I'll give that a try.


================
Comment at: llvm/lib/CodeGen/MachineVerifier.cpp:1510
 
+    if (!MI->getOperand(3).isImm())
+      report("'tail' flag (operand 3) must be an immediate type", MI);
----------------
paquette wrote:
> I think we only expect 0 or 1 for these values, so maybe we should check the value too?
Good call!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105072