[PGO] Fixed assertion due to mismatched memcpy size type.

[apazos]

Summary

Memcpy intrinsics have size argument of any integer type, like i32 or i64.
Fixed size type along with its value when cloning the intrinsic.

[apazos]

Added llvm-commits.

[davidxl]

lgtm

[xur]

Looks good to me. Thanks for fixing.

[mcrosier]

lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp

368 ↗

(On Diff #111545)

I believe this can be just a cast, rather than a dyn_cast (we know the cloned instruction is a MemIntrinsic).

370 ↗

(On Diff #111545)

You might consider adding a message within the assert:
assert(SizeType && "Expected an integer size type.");

[apazos]

Added assert message.

llvm/trunk/lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp

bool MemOPSizeOpt::perform(MemIntrinsic *MI) {
   if (SavedRemainCount > 0 || Version != NumVals)
     // Otherwise we need update with the un-promoted records back.
     annotateValueSite(*Func.getParent(), *MI, VDs.slice(Version),
                       SavedRemainCount, IPVK_MemOPSize, NumVals);
 
   DEBUG(dbgs() << "\n\n== Basic Block After==\n");
 
   for (uint64_t SizeId : SizeIds) {
-    ConstantInt *CaseSizeId = ConstantInt::get(Type::getInt64Ty(Ctx), SizeId);
     BasicBlock *CaseBB = BasicBlock::Create(
         Ctx, Twine("MemOP.Case.") + Twine(SizeId), &Func, DefaultBB);
     Instruction *NewInst = MI->clone();
     // Fix the argument.
-    dyn_cast<MemIntrinsic>(NewInst)->setLength(CaseSizeId);
+    MemIntrinsic * MemI = dyn_cast<MemIntrinsic>(NewInst);
+    IntegerType *SizeType = dyn_cast<IntegerType>(MemI->getLength()->getType());
+    assert(SizeType && "Expected integer type size argument.");
+    ConstantInt *CaseSizeId = ConstantInt::get(SizeType, SizeId);
+    MemI->setLength(CaseSizeId);
     CaseBB->getInstList().push_back(NewInst);
     IRBuilder<> IRBCase(CaseBB);
     IRBCase.CreateBr(MergeBB);
     SI->addCase(CaseSizeId, CaseBB);
     DEBUG(dbgs() << *CaseBB << "\n");
   }
   setProfMetadata(Func.getParent(), SI, CaseCounts, MaxCount);
 

llvm/trunk/test/Transforms/PGOProfile/memop_clone.ll

+; RUN: opt < %s -pgo-memop-opt -S | FileCheck %s
+
+define i32 @test(i8* %a, i8* %b) !prof !1 {
+; CHECK_LABEL: test
+; CHECK: MemOP.Case.3:
+; CHECK: tail call void @llvm.memcpy.p0i8.p0i8.i32(i8* undef, i8* %a, i32 3, i32 1, i1 false)
+; CHECK: MemOP.Case.2:
+; CHECK: tail call void @llvm.memcpy.p0i8.p0i8.i32(i8* undef, i8* %a, i32 2, i32 1, i1 false)
+; CHECK: MemOP.Default:
+; CHECK: tail call void @llvm.memcpy.p0i8.p0i8.i32(i8* undef, i8* %a, i32 undef, i32 1, i1 false)
+; CHECK: MemOP.Case.33:
+; CHECK: tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* undef, i8* %b, i64 3, i32 1, i1 false)
+; CHECK  MemOP.Case.24:
+; CHECK: tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* undef, i8* %b, i64 2, i32 1, i1 false)
+; CHECK: MemOP.Default2:
+; CHECK: tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* undef, i8* %b, i64 undef, i32 1, i1 false)
+  tail call void @llvm.memcpy.p0i8.p0i8.i32(i8* undef, i8* %a, i32 undef, i32 1, i1 false), !prof !2
+  tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* undef, i8* %b, i64 undef, i32 1, i1 false), !prof !2
+  unreachable
+}
+
+declare void @llvm.memcpy.p0i8.p0i8.i32(i8* nocapture writeonly, i8* nocapture readonly, i32, i32, i1)
+declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture writeonly, i8* nocapture readonly, i64, i32, i1)
+
+!1 = !{!"function_entry_count", i64 5170}
+!2 = !{!"VP", i32 1, i64 2585, i64 3, i64 1802, i64 2, i64 783}
+