Skip to content

Commit

Permalink
[Intrinsics][PreISelInstrinsicLowering] llvm.memcpy.inline length no …
Browse files Browse the repository at this point in the history
…longer needs to be constant (llvm#98281)

Summary:
Following on from the discussion in

https://discourse.llvm.org/t/rfc-introducing-an-llvm-memset-pattern-inline-intrinsic/79496
and the equivalent change for llvm.memset.inline (llvm#95397), this removes
the requirement that the length of llvm.memcpy.inline is constant.
PreISelInstrinsicLowering will expand llvm.memcpy.inline with
non-constant lengths, while the codegen path for constant lengths is
left unaltered.

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D59822399
  • Loading branch information
asb authored and sayhaan committed Jul 16, 2024
1 parent 476cb49 commit a1a6b42
Show file tree
Hide file tree
Showing 7 changed files with 69 additions and 32 deletions.
2 changes: 1 addition & 1 deletion llvm/docs/LangRef.rst
Original file line number Diff line number Diff line change
Expand Up @@ -15026,7 +15026,7 @@ Arguments:
""""""""""

The first argument is a pointer to the destination, the second is a
pointer to the source. The third argument is a constant integer argument
pointer to the source. The third argument is an integer argument
specifying the number of bytes to copy, and the fourth is a
boolean indicating a volatile access.

Expand Down
3 changes: 0 additions & 3 deletions llvm/include/llvm/IR/IntrinsicInst.h
Original file line number Diff line number Diff line change
Expand Up @@ -1296,9 +1296,6 @@ class MemMoveInst : public MemTransferInst {
/// This class wraps the llvm.memcpy.inline intrinsic.
class MemCpyInlineInst : public MemCpyInst {
public:
ConstantInt *getLength() const {
return cast<ConstantInt>(MemCpyInst::getLength());
}
// Methods for support type inquiry through isa, cast, and dyn_cast:
static bool classof(const IntrinsicInst *I) {
return I->getIntrinsicID() == Intrinsic::memcpy_inline;
Expand Down
3 changes: 1 addition & 2 deletions llvm/include/llvm/IR/Intrinsics.td
Original file line number Diff line number Diff line change
Expand Up @@ -966,15 +966,14 @@ def int_memcpy : Intrinsic<[],
// Memcpy semantic that is guaranteed to be inlined.
// In particular this means that the generated code is not allowed to call any
// external function.
// The third argument (specifying the size) must be a constant.
def int_memcpy_inline
: Intrinsic<[],
[llvm_anyptr_ty, llvm_anyptr_ty, llvm_anyint_ty, llvm_i1_ty],
[IntrArgMemOnly, IntrWillReturn, IntrNoFree, IntrNoCallback,
NoCapture<ArgIndex<0>>, NoCapture<ArgIndex<1>>,
NoAlias<ArgIndex<0>>, NoAlias<ArgIndex<1>>,
WriteOnly<ArgIndex<0>>, ReadOnly<ArgIndex<1>>,
ImmArg<ArgIndex<2>>, ImmArg<ArgIndex<3>>]>;
ImmArg<ArgIndex<3>>]>;

def int_memmove : Intrinsic<[],
[llvm_anyptr_ty, llvm_anyptr_ty, llvm_anyint_ty,
Expand Down
20 changes: 2 additions & 18 deletions llvm/lib/Analysis/Lint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,8 @@ void Lint::visitCallBase(CallBase &I) {

// TODO: Check more intrinsics

case Intrinsic::memcpy: {
case Intrinsic::memcpy:
case Intrinsic::memcpy_inline: {
MemCpyInst *MCI = cast<MemCpyInst>(&I);
visitMemoryReference(I, MemoryLocation::getForDest(MCI),
MCI->getDestAlign(), nullptr, MemRef::Write);
Expand All @@ -311,23 +312,6 @@ void Lint::visitCallBase(CallBase &I) {
"Undefined behavior: memcpy source and destination overlap", &I);
break;
}
case Intrinsic::memcpy_inline: {
MemCpyInlineInst *MCII = cast<MemCpyInlineInst>(&I);
const uint64_t Size = MCII->getLength()->getValue().getLimitedValue();
visitMemoryReference(I, MemoryLocation::getForDest(MCII),
MCII->getDestAlign(), nullptr, MemRef::Write);
visitMemoryReference(I, MemoryLocation::getForSource(MCII),
MCII->getSourceAlign(), nullptr, MemRef::Read);

// Check that the memcpy arguments don't overlap. The AliasAnalysis API
// isn't expressive enough for what we really want to do. Known partial
// overlap is not distinguished from the case where nothing is known.
const LocationSize LS = LocationSize::precise(Size);
Check(AA->alias(MCII->getSource(), LS, MCII->getDest(), LS) !=
AliasResult::MustAlias,
"Undefined behavior: memcpy source and destination overlap", &I);
break;
}
case Intrinsic::memmove: {
MemMoveInst *MMI = cast<MemMoveInst>(&I);
visitMemoryReference(I, MemoryLocation::getForDest(MMI),
Expand Down
16 changes: 16 additions & 0 deletions llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,21 @@ bool PreISelIntrinsicLowering::expandMemIntrinsicUses(Function &F) const {

break;
}
case Intrinsic::memcpy_inline: {
// Only expand llvm.memcpy.inline with non-constant length in this
// codepath, leaving the current SelectionDAG expansion for constant
// length memcpy intrinsics undisturbed.
auto *Memcpy = cast<MemCpyInlineInst>(Inst);
if (isa<ConstantInt>(Memcpy->getLength()))
break;

Function *ParentFunc = Memcpy->getFunction();
const TargetTransformInfo &TTI = LookupTTI(*ParentFunc);
expandMemCpyAsLoop(Memcpy, TTI);
Changed = true;
Memcpy->eraseFromParent();
break;
}
case Intrinsic::memmove: {
auto *Memmove = cast<MemMoveInst>(Inst);
Function *ParentFunc = Memmove->getFunction();
Expand Down Expand Up @@ -291,6 +306,7 @@ bool PreISelIntrinsicLowering::lowerIntrinsics(Module &M) const {
default:
break;
case Intrinsic::memcpy:
case Intrinsic::memcpy_inline:
case Intrinsic::memmove:
case Intrinsic::memset:
case Intrinsic::memset_inline:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
; RUN: opt -mtriple=x86_64-pc-linux-gnu -passes=pre-isel-intrinsic-lowering -S -o - %s | FileCheck %s

; Constant length memcpy.inline should be left unmodified.
define void @memcpy_32(ptr %dst, ptr %src) nounwind {
; CHECK-LABEL: define void @memcpy_32(
; CHECK-SAME: ptr [[DST:%.*]], ptr [[SRC:%.*]]) #[[ATTR0:[0-9]+]] {
; CHECK-NEXT: call void @llvm.memcpy.inline.p0.p0.i64(ptr [[DST]], ptr [[SRC]], i64 32, i1 false)
; CHECK-NEXT: tail call void @llvm.memcpy.inline.p0.p0.i64(ptr [[DST]], ptr [[SRC]], i64 32, i1 true)
; CHECK-NEXT: ret void
;
call void @llvm.memcpy.inline.p0.p0.i64(ptr %dst, ptr %src, i64 32, i1 0)
tail call void @llvm.memcpy.inline.p0.p0.i64(ptr %dst, ptr %src, i64 32, i1 1)
ret void
}

define void @memcpy_x(ptr %dst, ptr %src, i64 %x) nounwind {
; CHECK-LABEL: define void @memcpy_x(
; CHECK-SAME: ptr [[DST:%.*]], ptr [[SRC:%.*]], i64 [[X:%.*]]) #[[ATTR0]] {
; CHECK-NEXT: [[TMP1:%.*]] = icmp ne i64 [[X]], 0
; CHECK-NEXT: br i1 [[TMP1]], label %[[LOOP_MEMCPY_EXPANSION:.*]], label %[[POST_LOOP_MEMCPY_EXPANSION:.*]]
; CHECK: [[LOOP_MEMCPY_EXPANSION]]:
; CHECK-NEXT: [[LOOP_INDEX:%.*]] = phi i64 [ 0, [[TMP0:%.*]] ], [ [[TMP5:%.*]], %[[LOOP_MEMCPY_EXPANSION]] ]
; CHECK-NEXT: [[TMP2:%.*]] = getelementptr inbounds i8, ptr [[SRC]], i64 [[LOOP_INDEX]]
; CHECK-NEXT: [[TMP3:%.*]] = load i8, ptr [[TMP2]], align 1
; CHECK-NEXT: [[TMP4:%.*]] = getelementptr inbounds i8, ptr [[DST]], i64 [[LOOP_INDEX]]
; CHECK-NEXT: store i8 [[TMP3]], ptr [[TMP4]], align 1
; CHECK-NEXT: [[TMP5]] = add i64 [[LOOP_INDEX]], 1
; CHECK-NEXT: [[TMP6:%.*]] = icmp ult i64 [[TMP5]], [[X]]
; CHECK-NEXT: br i1 [[TMP6]], label %[[LOOP_MEMCPY_EXPANSION]], label %[[POST_LOOP_MEMCPY_EXPANSION]]
; CHECK: [[POST_LOOP_MEMCPY_EXPANSION]]:
; CHECK-NEXT: [[TMP7:%.*]] = icmp ne i64 [[X]], 0
; CHECK-NEXT: br i1 [[TMP7]], label %[[LOOP_MEMCPY_EXPANSION2:.*]], label %[[POST_LOOP_MEMCPY_EXPANSION1:.*]]
; CHECK: [[LOOP_MEMCPY_EXPANSION2]]:
; CHECK-NEXT: [[LOOP_INDEX3:%.*]] = phi i64 [ 0, %[[POST_LOOP_MEMCPY_EXPANSION]] ], [ [[TMP11:%.*]], %[[LOOP_MEMCPY_EXPANSION2]] ]
; CHECK-NEXT: [[TMP8:%.*]] = getelementptr inbounds i8, ptr [[SRC]], i64 [[LOOP_INDEX3]]
; CHECK-NEXT: [[TMP9:%.*]] = load volatile i8, ptr [[TMP8]], align 1
; CHECK-NEXT: [[TMP10:%.*]] = getelementptr inbounds i8, ptr [[DST]], i64 [[LOOP_INDEX3]]
; CHECK-NEXT: store volatile i8 [[TMP9]], ptr [[TMP10]], align 1
; CHECK-NEXT: [[TMP11]] = add i64 [[LOOP_INDEX3]], 1
; CHECK-NEXT: [[TMP12:%.*]] = icmp ult i64 [[TMP11]], [[X]]
; CHECK-NEXT: br i1 [[TMP12]], label %[[LOOP_MEMCPY_EXPANSION2]], label %[[POST_LOOP_MEMCPY_EXPANSION1]]
; CHECK: [[POST_LOOP_MEMCPY_EXPANSION1]]:
; CHECK-NEXT: ret void
;
call void @llvm.memcpy.inline.p0.p0.i64(ptr %dst, ptr %src, i64 %x, i1 0)
tail call void @llvm.memcpy.inline.p0.p0.i64(ptr %dst, ptr %src, i64 %x, i1 1)
ret void
}
8 changes: 0 additions & 8 deletions llvm/test/Verifier/intrinsic-immarg.ll
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,6 @@ define void @memcpy_inline_is_volatile(ptr %dest, ptr %src, i1 %is.volatile) {
ret void
}

define void @memcpy_inline_variable_size(ptr %dest, ptr %src, i32 %size) {
; CHECK: immarg operand has non-immediate parameter
; CHECK-NEXT: i32 %size
; CHECK-NEXT: call void @llvm.memcpy.inline.p0.p0.i32(ptr %dest, ptr %src, i32 %size, i1 true)
call void @llvm.memcpy.inline.p0.p0.i32(ptr %dest, ptr %src, i32 %size, i1 true)
ret void
}

declare void @llvm.memmove.p0.p0.i32(ptr nocapture, ptr nocapture, i32, i1)
define void @memmove(ptr %dest, ptr %src, i1 %is.volatile) {
; CHECK: immarg operand has non-immediate parameter
Expand Down

0 comments on commit a1a6b42

Please sign in to comment.