[InstCombine] Remove memset of undef value

[nikic]

Summary

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 https://github.com/llvm/llvm-project/issues/52930 is resolved, these and related folds can be limited to poison -- I've added FIXMEs to that effect.

[nikic]

Ping. My fallback plan here is to add this to MemCpyOpt instead and apply it only if the target memory is uninitialized (and thus already undef). I think that would be the worse option though.

[nlopes]

Well, I think this is ok. It's not regressing behavior vs undef store removal. So if this is important for Rust, let's maybe keep it,
Fingers crossed that the GSoC student will make good progress on the uninitialized memory thing.

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

Instruction *InstCombinerImpl::SimplifyAnyMemSet(AnyMemSetInst *MI) {
   // that the store must be storing the constant value (else the memory
   // wouldn't be constant), and this must be a noop.
   if (AA->pointsToConstantMemory(MI->getDest())) {
     // 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;
   }
 
+  // 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());
   if (!LenC || !FillC || !FillC->getType()->isIntegerTy(8))
     return nullptr;
   const uint64_t Len = LenC->getLimitedValue();
   assert(Len && "0-sized memory setting should be removed already.");
   const Align Alignment = assumeAligned(MI->getDestAlignment());

llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp

Instruction *InstCombinerImpl::visitStoreInst(StoreInst &SI) {
   // store X, GEP(null, Y) -> turns into 'unreachable' in SimplifyCFG
   if (canSimplifyNullStoreOrGEP(SI)) {
     if (!isa<PoisonValue>(Val))
       return replaceOperand(SI, 0, PoisonValue::get(Val->getType()));
     return nullptr;  // Do not modify these!
   }
 
   // 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);
 
   return nullptr;
 }
 
 /// Try to transform:
 ///   if () { *P = v1; } else { *P = v2 }

llvm/test/Transforms/InstCombine/memset.ll

 ; CHECK-LABEL: @memset_to_constant(
 ; CHECK-NEXT:    ret void
 ;
   %p = bitcast i128* @Unknown to i8*
   call void @llvm.memset.p0i8.i32(i8* %p, i8 0, i32 16, i1 false)
   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)
   ret void
 }
 
 define void @memset_undef_volatile(i8* %p) {
 ; CHECK-LABEL: @memset_undef_volatile(
 ; CHECK-NEXT:    call void @llvm.memset.p0i8.i32(i8* [[P:%.*]], i8 undef, i32 8, i1 true)
 ; CHECK-NEXT:    ret void
 ;
   call void @llvm.memset.p0i8.i32(i8* %p, i8 undef, i32 8, i1 true)
   ret void
 }
 
 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)
   ret void
 }
 
 define void @memset_poison_volatile(i8* %p) {
 ; CHECK-LABEL: @memset_poison_volatile(
 ; CHECK-NEXT:    call void @llvm.memset.p0i8.i32(i8* [[P:%.*]], i8 poison, i32 8, i1 true)
 ; CHECK-NEXT:    ret void
 ;
   call void @llvm.memset.p0i8.i32(i8* %p, i8 poison, i32 8, i1 true)
   ret void
 }
 
 declare void @llvm.memset.p0i8.i32(i8* nocapture writeonly, i8, i32, i1) argmemonly nounwind

llvm/test/Transforms/InstCombine/store.ll

 ; 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
 ;
   store i32 undef, i32* %P
   ret void
 }