[InstCombine] Remove memset of undef value

This removes memset with undef char. We already do this for stores of undef value. This comes with the caveat that this optimization is not, strictly speaking, legal for undef values, because we might be overwriting a poison value. However, our entire load/store model currently still operates on undef values, so we need to support undef here as well for internal consistency. Once #52930 is resolved, these and related folds can be limited to poison -- I've added FIXMEs to that effect. Differential Revision: https://reviews.llvm.org/D124173
llvm · Apr 29, 2022 · 1881711 · 1881711
1 parent 24a133e
commit 1881711
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 2 deletions.
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -258,6 +258,15 @@ Instruction *InstCombinerImpl::SimplifyAnyMemSet(AnyMemSetInst *MI) {
     return MI;
   }
 
+  // Remove memset with an undef value.
+  // FIXME: This is technically incorrect because it might overwrite a poison
+  // value. Change to PoisonValue once #52930 is resolved.
+  if (isa<UndefValue>(MI->getValue())) {
+    // Set the size of the copy to 0, it will be deleted on the next iteration.
+    MI->setLength(Constant::getNullValue(MI->getLength()->getType()));
+    return MI;
+  }
+
   // Extract the length and alignment and fill if they are constant.
   ConstantInt *LenC = dyn_cast<ConstantInt>(MI->getLength());
   ConstantInt *FillC = dyn_cast<ConstantInt>(MI->getValue());

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
@@ -1435,6 +1435,8 @@ Instruction *InstCombinerImpl::visitStoreInst(StoreInst &SI) {
   }
 
   // store undef, Ptr -> noop
+  // FIXME: This is technically incorrect because it might overwrite a poison
+  // value. Change to PoisonValue once #52930 is resolved.
   if (isa<UndefValue>(Val))
     return eraseInstFromFunction(SI);
 

diff --git a/llvm/test/Transforms/InstCombine/memset.ll b/llvm/test/Transforms/InstCombine/memset.ll
@@ -33,9 +33,10 @@ define void @memset_to_constant() {
   ret void
 }
 
+; FIXME: This is technically incorrect because it might overwrite a poison
+; value. Stop folding it once #52930 is resolved.
 define void @memset_undef(i8* %p) {
 ; CHECK-LABEL: @memset_undef(
-; CHECK-NEXT:    call void @llvm.memset.p0i8.i32(i8* noundef nonnull align 1 dereferenceable(8) [[P:%.*]], i8 undef, i32 8, i1 false)
 ; CHECK-NEXT:    ret void
 ;
   call void @llvm.memset.p0i8.i32(i8* %p, i8 undef, i32 8, i1 false)
@@ -53,7 +54,6 @@ define void @memset_undef_volatile(i8* %p) {
 
 define void @memset_poison(i8* %p) {
 ; CHECK-LABEL: @memset_poison(
-; CHECK-NEXT:    call void @llvm.memset.p0i8.i32(i8* noundef nonnull align 1 dereferenceable(8) [[P:%.*]], i8 poison, i32 8, i1 false)
 ; CHECK-NEXT:    ret void
 ;
   call void @llvm.memset.p0i8.i32(i8* %p, i8 poison, i32 8, i1 false)

diff --git a/llvm/test/Transforms/InstCombine/store.ll b/llvm/test/Transforms/InstCombine/store.ll
@@ -1,6 +1,8 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -passes=instcombine -S | FileCheck %s
 
+; FIXME: This is technically incorrect because it might overwrite a poison
+; value. Stop folding it once #52930 is resolved.
 define void @store_of_undef(i32* %P) {
 ; CHECK-LABEL: @store_of_undef(
 ; CHECK-NEXT:    ret void