[PATCH] D71710: [instrinsics] Add @llvm.memcpy.inline instrinsics

Eli Friedman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 15 13:38:22 PST 2020


efriedma added a comment.

In D71710#1821546 <https://reviews.llvm.org/D71710#1821546>, @gchatelet wrote:

> In D71710#1806641 <https://reviews.llvm.org/D71710#1806641>, @efriedma wrote:
>
> > It would be nice to do a bit more work so various optimizations handle llvm.memcpy.inline in a way that isn't completely conservative (for example, in alias analysis), but I guess that doesn't need to be in the initial patch.
>
>
> I'm not too familiar with AA but my understanding is that the definition in `llvm/include/llvm/IR/Intrinsics.td` already describes the arguments with enough precision to be helpful for the AA pass.
>
>   def int_memcpy_inline
>       : Intrinsic<[],
>         [ llvm_anyptr_ty, llvm_anyptr_ty, llvm_anyint_ty, llvm_i1_ty ],
>         [ IntrArgMemOnly, NoCapture<0>, NoCapture<1>, WriteOnly<0>, ReadOnly<1>,
>         ImmArg<2>, ImmArg<3> ]>;
>
>
> Did you have something specific in mind? Or am I missing something completely?


The attributes are helpful, but not enough to figure out how many bytes are modified.



================
Comment at: llvm/include/llvm/IR/IntrinsicInst.h:659
+      return isa<IntrinsicInst>(V) && classof(cast<IntrinsicInst>(V));
+    }
+  };
----------------
gchatelet wrote:
> efriedma wrote:
> > Maybe hide the base class getLength() with a version that returns a ConstantInt instead?  You could use it, for example, in Lint.cpp.
> For `Lint.cpp` it has to go through `findValue` which erases the type, not sure what you prefer, make `findValue` a template or `cast` (as it's currently done)
You don't need to go through findValue to find a value that's already a ConstantInt.


================
Comment at: llvm/test/CodeGen/X86/memcpy-inline.ll:12
+; CHECK-NEXT:    retq
+  tail call void @llvm.memcpy.inline.p0i8.p0i8.i64(i8* %a, i8* %b, i64 8, i1 0 )
+  ret void
----------------
gchatelet wrote:
> efriedma wrote:
> > It would be a good idea to have a testcase for a larger size that wouldn't normally be inlined.  (Also, maybe a test on another target that doesn't have a "memcpy" instruction.)
> > Also, maybe a test on another target that doesn't have a "memcpy" instruction.
> 
> I'm not sure what you want to test with this.
Probably not that important, given the tests you have already; nevermind.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71710





More information about the llvm-commits mailing list