[MemoryLocation] Model value parameter of memset.pattern intrinsic

[preames]

This parameter may be a pointer if the pointer is being written to the destination, but is neither read or written.

[llvmbot]

@llvm/pr-subscribers-llvm-analysis

Author: Philip Reames (preames)

Changes

This parameter may be a pointer if the pointer is being written to the destination, but is neither read or written.

---

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

2 Files Affected:

• (modified) llvm/lib/Analysis/MemoryLocation.cpp (+4)
• (modified) llvm/test/Transforms/GVN/memset-pattern.ll (+2-5)

diff --git a/llvm/lib/Analysis/MemoryLocation.cpp b/llvm/lib/Analysis/MemoryLocation.cpp
index 6e3232772706a..7c474fe1a28b6 100644
--- a/llvm/lib/Analysis/MemoryLocation.cpp
+++ b/llvm/lib/Analysis/MemoryLocation.cpp
@@ -186,6 +186,10 @@ MemoryLocation MemoryLocation::getForArgument(const CallBase *Call,
     case Intrinsic::experimental_memset_pattern:
       assert((ArgIdx == 0 || ArgIdx == 1) &&
              "Invalid argument index for memory intrinsic");
+      if (ArgIdx == 1) {
+        assert(Arg->getType()->isPointerTy());
+        return MemoryLocation(Arg, LocationSize::precise(0), AATags);
+      }
       if (ConstantInt *LenCI = dyn_cast<ConstantInt>(II->getArgOperand(2)))
         return MemoryLocation(
             Arg,
diff --git a/llvm/test/Transforms/GVN/memset-pattern.ll b/llvm/test/Transforms/GVN/memset-pattern.ll
index 20ac87e916757..6fe50867df8dc 100644
--- a/llvm/test/Transforms/GVN/memset-pattern.ll
+++ b/llvm/test/Transforms/GVN/memset-pattern.ll
@@ -49,11 +49,8 @@ define i32 @load_forward_over_memset_pattern(ptr %P, ptr noalias %Q) {
 
 define i32 @load_forward_over_memset_pattern2(ptr %P, ptr noalias %Q) nounwind ssp {
 ; CHECK-LABEL: @load_forward_over_memset_pattern2(
-; CHECK-NEXT:    [[V1:%.*]] = load i32, ptr [[Q:%.*]], align 4
-; CHECK-NEXT:    tail call void @llvm.experimental.memset.pattern.p0.p0.i64(ptr [[P:%.*]], ptr [[Q]], i64 8, i1 false)
-; CHECK-NEXT:    [[V2:%.*]] = load i32, ptr [[Q]], align 4
-; CHECK-NEXT:    [[SUB:%.*]] = sub i32 [[V1]], [[V2]]
-; CHECK-NEXT:    ret i32 [[SUB]]
+; CHECK-NEXT:    tail call void @llvm.experimental.memset.pattern.p0.p0.i64(ptr [[P:%.*]], ptr [[Q:%.*]], i64 8, i1 false)
+; CHECK-NEXT:    ret i32 0
 ;
   %v1 = load i32, ptr %Q
   tail call void @llvm.experimental.memset.pattern(ptr %P, ptr %Q, i64 8, i1 false)

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Philip Reames (preames)

Changes

This parameter may be a pointer if the pointer is being written to the destination, but is neither read or written.

---

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

2 Files Affected:

• (modified) llvm/lib/Analysis/MemoryLocation.cpp (+4)
• (modified) llvm/test/Transforms/GVN/memset-pattern.ll (+2-5)

diff --git a/llvm/lib/Analysis/MemoryLocation.cpp b/llvm/lib/Analysis/MemoryLocation.cpp
index 6e3232772706a..7c474fe1a28b6 100644
--- a/llvm/lib/Analysis/MemoryLocation.cpp
+++ b/llvm/lib/Analysis/MemoryLocation.cpp
@@ -186,6 +186,10 @@ MemoryLocation MemoryLocation::getForArgument(const CallBase *Call,
     case Intrinsic::experimental_memset_pattern:
       assert((ArgIdx == 0 || ArgIdx == 1) &&
              "Invalid argument index for memory intrinsic");
+      if (ArgIdx == 1) {
+        assert(Arg->getType()->isPointerTy());
+        return MemoryLocation(Arg, LocationSize::precise(0), AATags);
+      }
       if (ConstantInt *LenCI = dyn_cast<ConstantInt>(II->getArgOperand(2)))
         return MemoryLocation(
             Arg,
diff --git a/llvm/test/Transforms/GVN/memset-pattern.ll b/llvm/test/Transforms/GVN/memset-pattern.ll
index 20ac87e916757..6fe50867df8dc 100644
--- a/llvm/test/Transforms/GVN/memset-pattern.ll
+++ b/llvm/test/Transforms/GVN/memset-pattern.ll
@@ -49,11 +49,8 @@ define i32 @load_forward_over_memset_pattern(ptr %P, ptr noalias %Q) {
 
 define i32 @load_forward_over_memset_pattern2(ptr %P, ptr noalias %Q) nounwind ssp {
 ; CHECK-LABEL: @load_forward_over_memset_pattern2(
-; CHECK-NEXT:    [[V1:%.*]] = load i32, ptr [[Q:%.*]], align 4
-; CHECK-NEXT:    tail call void @llvm.experimental.memset.pattern.p0.p0.i64(ptr [[P:%.*]], ptr [[Q]], i64 8, i1 false)
-; CHECK-NEXT:    [[V2:%.*]] = load i32, ptr [[Q]], align 4
-; CHECK-NEXT:    [[SUB:%.*]] = sub i32 [[V1]], [[V2]]
-; CHECK-NEXT:    ret i32 [[SUB]]
+; CHECK-NEXT:    tail call void @llvm.experimental.memset.pattern.p0.p0.i64(ptr [[P:%.*]], ptr [[Q:%.*]], i64 8, i1 false)
+; CHECK-NEXT:    ret i32 0
 ;
   %v1 = load i32, ptr %Q
   tail call void @llvm.experimental.memset.pattern(ptr %P, ptr %Q, i64 8, i1 false)

[preames]

Notes:

1. There are more changes needed here - in particular CaptureTracking, and possibly BasicAA.
2. Relating to (1), I couldn't find an obvious spot to just set the capture attributes and readnone on the parameter of the intrinsic signature if-and-only-if it were a pointer. Is there something I'm missing?

[nikic]

We should definitely handle this via attributes on the intrinsic instead. I have recently done the groundwork to allow intrinsic attributes to depend on the type, so doing this should be pretty simple now.

I'd expect the lowest complexity ways to achieve this would be to either a) add something like IntrCustom<ArgIndex<1>, "getMemsetPatternAttrs"> and implement the logic in C++ or b) add a pseudo-property like IntrReadNoneNoCaptureIfPtr. (Could probably also use it in some other places like llvm.is.constant.)

[preames]

@nikic Do you have a pointer to an example I can copy-from? Or a link to the recent work you mentioned? I tried finding something in tree before posting this, but apparently don't have the right search terms.

[nikic]

@preames See #135642.