[SimplifyLibCalls] Don't change alignment when creating memset

[sstefan1]

Summary

Fix for PR49984

This was discovered during Attributor testing. Memset was always created with alignment of 1 and in case when strncpy alignment was changed it triggered an assertion in the AttrBuilder. Memset will now be created with appropriate alignment.

[jdoerfert]

LGTM

[lebedev.ri]

Please explain the problem in patch's description.

[lebedev.ri]

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

577–578

The comment is now wrong

[sstefan1]

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

577–578

Yeah, forgot to update. I guess alignment shouldn't be mentioned anymore.

[sstefan1]

update comment

[sstefan1]

@lebedev.ri good to go?

[xbolva00]

Looks ok as a fix. I am just wondering if some other places should be checked as well.

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

604

Could we expect same problem here in the future?

[sstefan1]

In D100875#2705331, @xbolva00 wrote:

Looks ok as a fix. I am just wondering if some other places should be checked as well.

Good question. I've just checked and this is the only place to call addParamAttributes() which has the assertion. Other places just set attributes.

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

604

No, as long as addParamAttributes() is not called bellow.

[xbolva00]

Ok, thank you.

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

Value *LibCallSimplifier::optimizeStrNCpy(CallInst *CI, IRBuilderBase &B) {
   uint64_t SrcLen = GetStringLength(Src);
   if (SrcLen) {
     annotateDereferenceableBytes(CI, 1, SrcLen);
     --SrcLen; // Unbias length.
   } else {
     return nullptr;
   }
 
   if (SrcLen == 0) {
-    // strncpy(x, "", y) -> memset(align 1 x, '\0', y)
-    CallInst *NewCI = B.CreateMemSet(Dst, B.getInt8('\0'), Size, Align(1));
+    // strncpy(x, "", y) -> memset(x, '\0', y)
+    Align MemSetAlign =
+        CI->getAttributes().getParamAttributes(0).getAlignment().valueOrOne();
+    CallInst *NewCI = B.CreateMemSet(Dst, B.getInt8('\0'), Size, MemSetAlign);
     AttrBuilder ArgAttrs(CI->getAttributes().getParamAttributes(0));
     NewCI->setAttributes(NewCI->getAttributes().addParamAttributes(
         CI->getContext(), 0, ArgAttrs));
     return Dst;
   }
 
   // strncpy(a, "a", 4) - > memcpy(a, "a\0\0\0", 4)
   if (Len > SrcLen + 1) {
     if (Len <= 128) {
       StringRef Str;
       if (!getConstantStringInfo(Src, Str))
         return nullptr;
       std::string SrcStr = Str.str();
       SrcStr.resize(Len, '\0');
       Src = B.CreateGlobalString(SrcStr, "str");
     } else {
       return nullptr;
     }
   }
 
   Type *PT = Callee->getFunctionType()->getParamType(0);
   // strncpy(x, s, c) -> memcpy(align 1 x, align 1 s, c) [s and c are constant]
   CallInst *NewCI = B.CreateMemCpy(Dst, Align(1), Src, Align(1),
                                    ConstantInt::get(DL.getIntPtrType(PT), Len));
   NewCI->setAttributes(CI->getAttributes());
   NewCI->removeAttributes(AttributeList::ReturnIndex,
                           AttributeFuncs::typeIncompatible(NewCI->getType()));
   return Dst;
 }
 
 Value *LibCallSimplifier::optimizeStringLength(CallInst *CI, IRBuilderBase &B,

llvm/test/Transforms/InstCombine/strncpy-1.ll

 ; CHECK-NEXT:    call void @llvm.memset.p0i8.i32(i8* noalias noundef nonnull align 1 dereferenceable(5) [[DST:%.*]], i8 0, i32 5, i1 false)
 ; CHECK-NEXT:    ret i8* [[DST]]
 ;
   %src = getelementptr [1 x i8], [1 x i8]* @null, i32 0, i32 0
   %ret = call i8* @strncpy(i8* noalias nonnull %dst, i8* nonnull %src, i32 5);
   ret i8* %ret
 }
 
+define i8* @test4(i8* %dst, i32 %n) {
+; CHECK-LABEL: @test4(
+; CHECK-NEXT:    call void @llvm.memset.p0i8.i32(i8* noalias noundef nonnull align 16 dereferenceable(5) [[DST:%.*]], i8 0, i32 5, i1 false)
+; CHECK-NEXT:    ret i8* [[DST]]
+;
+  %src = getelementptr [1 x i8], [1 x i8]* @null, i32 0, i32 0
+  %ret = call i8* @strncpy(i8* align(16) noalias nonnull %dst, i8* nonnull %src, i32 5);
+  ret i8* %ret
+}
+
 ; Check cases that shouldn't be simplified.
 
 define void @test_no_simplify1() {
 ; CHECK-LABEL: @test_no_simplify1(
 ; CHECK-NEXT:    [[TMP1:%.*]] = call i8* @strncpy(i8* noundef nonnull dereferenceable(1) getelementptr inbounds ([32 x i8], [32 x i8]* @a, i32 0, i32 0), i8* noundef nonnull dereferenceable(1) getelementptr inbounds ([32 x i8], [32 x i8]* @b, i32 0, i32 0), i32 32)
 ; CHECK-NEXT:    ret void
 ;
   %dst = getelementptr [32 x i8], [32 x i8]* @a, i32 0, i32 0