[Intrinsics][PreISelIntrinsicLowering] llvm.memset.inline length no longer needs to be constant

[asb]

As requested in https://discourse.llvm.org/t/rfc-introducing-an-llvm-memset-pattern-inline-intrinsic/79496 this patch removes the requirement that the length of llvm.memset.inline is a constant, and adjusts PreISelIntrinsicLowering so it supports expanding such the intrinsic in the case it has a non-constant length.

[llvmbot]

@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-llvm-transforms

Author: Alex Bradbury (asb)

Changes

As requested in https://discourse.llvm.org/t/rfc-introducing-an-llvm-memset-pattern-inline-intrinsic/79496 this patch removes the requirement that the length of llvm.memset.inline is a constant, and adjusts PreISelIntrinsicLowering so it supports expanding such the intrinsic in the case it has a non-constant length.

---

Full diff: https://github.com/llvm/llvm-project/pull/95397.diff

6 Files Affected:

• (modified) llvm/docs/LangRef.rst (-1)
• (modified) llvm/include/llvm/IR/IntrinsicInst.h (-3)
• (modified) llvm/include/llvm/IR/Intrinsics.td (+1-1)
• (modified) llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp (+14)
• (added) llvm/test/Transforms/PreISelIntrinsicLowering/X86/memset-inline-non-constant-len.ll (+32)
• (modified) llvm/test/Verifier/intrinsic-immarg.ll (-8)

diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index 9fb2c048a5c86..b46688db703c7 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -15223,7 +15223,6 @@ at the destination location. If the argument is known to be
 aligned to some boundary, this can be specified as an attribute on
 the argument.
 
-``len`` must be a constant expression.
 If ``<len>`` is 0, it is no-op modulo the behavior of attributes attached to
 the arguments.
 If ``<len>`` is not a well-defined value, the behavior is undefined.
diff --git a/llvm/include/llvm/IR/IntrinsicInst.h b/llvm/include/llvm/IR/IntrinsicInst.h
index 9010e1a1c896b..623ba80585a00 100644
--- a/llvm/include/llvm/IR/IntrinsicInst.h
+++ b/llvm/include/llvm/IR/IntrinsicInst.h
@@ -1202,9 +1202,6 @@ class MemSetInst : public MemSetBase<MemIntrinsic> {
 /// This class wraps the llvm.memset.inline intrinsic.
 class MemSetInlineInst : public MemSetInst {
 public:
-  ConstantInt *getLength() const {
-    return cast<ConstantInt>(MemSetInst::getLength());
-  }
   // Methods for support type inquiry through isa, cast, and dyn_cast:
   static bool classof(const IntrinsicInst *I) {
     return I->getIntrinsicID() == Intrinsic::memset_inline;
diff --git a/llvm/include/llvm/IR/Intrinsics.td b/llvm/include/llvm/IR/Intrinsics.td
index 107442623ab7b..64d4a717c16e0 100644
--- a/llvm/include/llvm/IR/Intrinsics.td
+++ b/llvm/include/llvm/IR/Intrinsics.td
@@ -1006,7 +1006,7 @@ def int_memset_inline
       [llvm_anyptr_ty, llvm_i8_ty, llvm_anyint_ty, llvm_i1_ty],
       [IntrWriteMem, IntrArgMemOnly, IntrWillReturn, IntrNoFree, IntrNoCallback,
        NoCapture<ArgIndex<0>>, WriteOnly<ArgIndex<0>>,
-       ImmArg<ArgIndex<2>>, ImmArg<ArgIndex<3>>]>;
+       ImmArg<ArgIndex<3>>]>;
 
 // FIXME: Add version of these floating point intrinsics which allow non-default
 // rounding modes and FP exception handling.
diff --git a/llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp b/llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
index 0777acf633187..8572cdc160456 100644
--- a/llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
+++ b/llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
@@ -263,6 +263,19 @@ bool PreISelIntrinsicLowering::expandMemIntrinsicUses(Function &F) const {
 
       break;
     }
+    case Intrinsic::memset_inline: {
+      // Only expand llvm.memset.inline with non-constant length in this
+      // codepath, leaving the current SelectionDAG expansion for constant
+      // length memset intrinsics undisturbed.
+      auto *Memset = cast<MemSetInlineInst>(Inst);
+      if (isa<ConstantInt>(Memset->getLength()))
+        break;
+
+      expandMemSetAsLoop(Memset);
+      Changed = true;
+      Memset->eraseFromParent();
+      break;
+    }
     default:
       llvm_unreachable("unhandled intrinsic");
     }
@@ -280,6 +293,7 @@ bool PreISelIntrinsicLowering::lowerIntrinsics(Module &M) const {
     case Intrinsic::memcpy:
     case Intrinsic::memmove:
     case Intrinsic::memset:
+    case Intrinsic::memset_inline:
       Changed |= expandMemIntrinsicUses(F);
       break;
     case Intrinsic::load_relative:
diff --git a/llvm/test/Transforms/PreISelIntrinsicLowering/X86/memset-inline-non-constant-len.ll b/llvm/test/Transforms/PreISelIntrinsicLowering/X86/memset-inline-non-constant-len.ll
new file mode 100644
index 0000000000000..1ed14e02117b8
--- /dev/null
+++ b/llvm/test/Transforms/PreISelIntrinsicLowering/X86/memset-inline-non-constant-len.ll
@@ -0,0 +1,32 @@
+; 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 memset.inline should be left unmodified.
+define void @memset_32(ptr %a, i8 %value) nounwind {
+; CHECK-LABEL: define void @memset_32(
+; CHECK-SAME: ptr [[A:%.*]], i8 [[VALUE:%.*]]) #[[ATTR0:[0-9]+]] {
+; CHECK-NEXT:    tail call void @llvm.memset.inline.p0.i64(ptr [[A]], i8 [[VALUE]], i64 32, i1 false)
+; CHECK-NEXT:    ret void
+;
+  tail call void @llvm.memset.inline.p0.i64(ptr %a, i8 %value, i64 32, i1 0)
+  ret void
+}
+
+define void @memset_x(ptr %a, i8 %value, i64 %x) nounwind {
+; CHECK-LABEL: define void @memset_x(
+; CHECK-SAME: ptr [[A:%.*]], i8 [[VALUE:%.*]], i64 [[X:%.*]]) #[[ATTR0]] {
+; CHECK-NEXT:    [[TMP1:%.*]] = icmp eq i64 0, [[X]]
+; CHECK-NEXT:    br i1 [[TMP1]], label %[[SPLIT:.*]], label %[[LOADSTORELOOP:.*]]
+; CHECK:       [[LOADSTORELOOP]]:
+; CHECK-NEXT:    [[TMP2:%.*]] = phi i64 [ 0, [[TMP0:%.*]] ], [ [[TMP4:%.*]], %[[LOADSTORELOOP]] ]
+; CHECK-NEXT:    [[TMP3:%.*]] = getelementptr inbounds i8, ptr [[A]], i64 [[TMP2]]
+; CHECK-NEXT:    store i8 [[VALUE]], ptr [[TMP3]], align 1
+; CHECK-NEXT:    [[TMP4]] = add i64 [[TMP2]], 1
+; CHECK-NEXT:    [[TMP5:%.*]] = icmp ult i64 [[TMP4]], [[X]]
+; CHECK-NEXT:    br i1 [[TMP5]], label %[[LOADSTORELOOP]], label %[[SPLIT]]
+; CHECK:       [[SPLIT]]:
+; CHECK-NEXT:    ret void
+;
+  tail call void @llvm.memset.inline.p0.i64(ptr %a, i8 %value, i64 %x, i1 0)
+  ret void
+}
diff --git a/llvm/test/Verifier/intrinsic-immarg.ll b/llvm/test/Verifier/intrinsic-immarg.ll
index 47189c0b7d052..b1b9f7ee4be11 100644
--- a/llvm/test/Verifier/intrinsic-immarg.ll
+++ b/llvm/test/Verifier/intrinsic-immarg.ll
@@ -71,14 +71,6 @@ define void @memset_inline_is_volatile(ptr %dest, i8 %value, i1 %is.volatile) {
   ret void
 }
 
-define void @memset_inline_variable_size(ptr %dest, i8 %value, i32 %size) {
-  ; CHECK: immarg operand has non-immediate parameter
-  ; CHECK-NEXT: i32 %size
-  ; CHECK-NEXT: call void @llvm.memset.inline.p0.i32(ptr %dest, i8 %value, i32 %size, i1 true)
-  call void @llvm.memset.inline.p0.i32(ptr %dest, i8 %value, i32 %size, i1 true)
-  ret void
-}
-
 
 declare i64 @llvm.objectsize.i64.p0(ptr, i1, i1, i1)
 define void @objectsize(ptr %ptr, i1 %a, i1 %b, i1 %c) {

[arsenm]

We probably should have some more intelligent heuristic / control for this, but I guess this is fine for now

[asb]

@nikic just wanted to get another LGTM for making this change to the intrinsic limitations (which I'd intend to apply across the other inline mem* intrinsics in future PRs).

[nikic]

LGTM