diff --git a/include/llvm/Analysis/IRSimilarityIdentifier.h b/include/llvm/Analysis/IRSimilarityIdentifier.h index a3004ca0dc5..9e97541e542 100644 --- a/include/llvm/Analysis/IRSimilarityIdentifier.h +++ b/include/llvm/Analysis/IRSimilarityIdentifier.h @@ -183,6 +183,12 @@ struct IRInstructionData : ilist_node { llvm::hash_value(ID.Inst->getType()), llvm::hash_value(ID.getPredicate()), llvm::hash_combine_range(OperTypes.begin(), OperTypes.end())); + else if (CallInst *CI = dyn_cast(ID.Inst)) + return llvm::hash_combine( + llvm::hash_value(ID.Inst->getOpcode()), + llvm::hash_value(ID.Inst->getType()), + llvm::hash_value(CI->getCalledFunction()->getName().str()), + llvm::hash_combine_range(OperTypes.begin(), OperTypes.end())); return llvm::hash_combine( llvm::hash_value(ID.Inst->getOpcode()), llvm::hash_value(ID.Inst->getType()), @@ -398,8 +404,14 @@ struct IRInstructionMapper { InstrType visitDbgInfoIntrinsic(DbgInfoIntrinsic &DII) { return Invisible; } // TODO: Handle specific intrinsics. InstrType visitIntrinsicInst(IntrinsicInst &II) { return Illegal; } - // TODO: Handle CallInsts. - InstrType visitCallInst(CallInst &CI) { return Illegal; } + // We only allow call instructions where the function has a name and + // is not an indirect call. + InstrType visitCallInst(CallInst &CI) { + Function *F = CI.getCalledFunction(); + if (!F || CI.isIndirectCall() || !F->hasName()) + return Illegal; + return Legal; + } // TODO: We do not current handle similarity that changes the control flow. InstrType visitInvokeInst(InvokeInst &II) { return Illegal; } // TODO: We do not current handle similarity that changes the control flow. diff --git a/lib/Analysis/IRSimilarityIdentifier.cpp b/lib/Analysis/IRSimilarityIdentifier.cpp index c276e3f28d7..60b4f427e18 100644 --- a/lib/Analysis/IRSimilarityIdentifier.cpp +++ b/lib/Analysis/IRSimilarityIdentifier.cpp @@ -75,6 +75,12 @@ CmpInst::Predicate IRInstructionData::getPredicate() const { return cast(Inst)->getPredicate(); } +static StringRef getCalledFunctionName(CallInst &CI) { + assert(CI.getCalledFunction() != nullptr && "Called Function is nullptr?"); + + return CI.getCalledFunction()->getName(); +} + bool IRSimilarity::isClose(const IRInstructionData &A, const IRInstructionData &B) { @@ -129,6 +135,16 @@ bool IRSimilarity::isClose(const IRInstructionData &A, }); } + // If the instructions are functions, we make sure that the function name is + // the same. We already know that the types are since is isSameOperationAs is + // true. + if (isa(A.Inst) && isa(B.Inst)) { + CallInst *CIA = cast(A.Inst); + CallInst *CIB = cast(B.Inst); + if (getCalledFunctionName(*CIA).compare(getCalledFunctionName(*CIB)) != 0) + return false; + } + return true; } diff --git a/test/Transforms/IROutliner/illegal-calls.ll b/test/Transforms/IROutliner/illegal-calls.ll index 99c8f6fa838..18d37c2f4dc 100644 --- a/test/Transforms/IROutliner/illegal-calls.ll +++ b/test/Transforms/IROutliner/illegal-calls.ll @@ -13,7 +13,9 @@ define void @outline_constants1() { ; CHECK-NEXT: [[A:%.*]] = alloca i32, align 4 ; CHECK-NEXT: [[B:%.*]] = alloca i32, align 4 ; CHECK-NEXT: [[C:%.*]] = alloca i32, align 4 -; CHECK-NEXT: call void @outlined_ir_func_1(i32* [[A]], i32* [[B]], i32* [[C]]) +; CHECK-NEXT: store i32 2, i32* [[A]], align 4 +; CHECK-NEXT: store i32 3, i32* [[B]], align 4 +; CHECK-NEXT: store i32 4, i32* [[C]], align 4 ; CHECK-NEXT: call void @f1(i32* [[A]], i32* [[B]]) ; CHECK-NEXT: call void @outlined_ir_func_0(i32* [[A]], i32* [[B]], i32* [[C]]) ; CHECK-NEXT: ret void @@ -38,7 +40,9 @@ define void @outline_constants2() { ; CHECK-NEXT: [[A:%.*]] = alloca i32, align 4 ; CHECK-NEXT: [[B:%.*]] = alloca i32, align 4 ; CHECK-NEXT: [[C:%.*]] = alloca i32, align 4 -; CHECK-NEXT: call void @outlined_ir_func_1(i32* [[A]], i32* [[B]], i32* [[C]]) +; CHECK-NEXT: store i32 2, i32* [[A]], align 4 +; CHECK-NEXT: store i32 3, i32* [[B]], align 4 +; CHECK-NEXT: store i32 4, i32* [[C]], align 4 ; CHECK-NEXT: call void @f1(i32* [[A]], i32* [[B]]) ; CHECK-NEXT: call void @outlined_ir_func_0(i32* [[A]], i32* [[B]], i32* [[C]]) ; CHECK-NEXT: ret void diff --git a/unittests/Analysis/IRSimilarityIdentifierTest.cpp b/unittests/Analysis/IRSimilarityIdentifierTest.cpp index ba33144c8ef..6bb6d7da228 100644 --- a/unittests/Analysis/IRSimilarityIdentifierTest.cpp +++ b/unittests/Analysis/IRSimilarityIdentifierTest.cpp @@ -929,14 +929,13 @@ TEST(IRInstructionMapper, GetElementPtrDifferentInBounds) { ASSERT_NE(UnsignedVec[0], UnsignedVec[1]); } -// Checks that a call instruction is mapped to be illegal. We have to perform -// extra checks to ensure that both the name and function type are the same. -TEST(IRInstructionMapper, CallIllegal) { +// Checks that indirect call instructions are mapped to be illegal since we +// cannot guarantee the same function in two different cases. +TEST(IRInstructionMapper, CallsIllegalIndirect) { StringRef ModuleString = R"( - declare i32 @f1(i32, i32) - define i32 @f(i32 %a, i32 %b) { + define i32 @f(void()* %func) { bb0: - %0 = call i32 @f1(i32 %a, i32 %b) + call void %func() ret i32 0 })"; LLVMContext Context; @@ -954,6 +953,199 @@ TEST(IRInstructionMapper, CallIllegal) { ASSERT_EQ(UnsignedVec.size(), static_cast(0)); } +// Checks that a call instruction is mapped to be legal. Here we check that +// a call with the same name, and same types are mapped to the same +// value. +TEST(IRInstructionMapper, CallsSameTypeSameName) { + StringRef ModuleString = R"( + declare i32 @f1(i32, i32) + define i32 @f(i32 %a, i32 %b) { + bb0: + %0 = call i32 @f1(i32 %a, i32 %b) + %1 = call i32 @f1(i32 %a, i32 %b) + ret i32 0 + })"; + LLVMContext Context; + std::unique_ptr M = makeLLVMModule(Context, ModuleString); + + std::vector InstrList; + std::vector UnsignedVec; + + SpecificBumpPtrAllocator InstDataAllocator; + SpecificBumpPtrAllocator IDLAllocator; + IRInstructionMapper Mapper(&InstDataAllocator, &IDLAllocator); + getVectors(*M, Mapper, InstrList, UnsignedVec); + + ASSERT_EQ(InstrList.size(), UnsignedVec.size()); + ASSERT_EQ(UnsignedVec.size(), static_cast(3)); + ASSERT_EQ(UnsignedVec[0], UnsignedVec[1]); +} + +// Here we check that a calls with different names, but the same arguments types +// are mapped to different value. +TEST(IRInstructionMapper, CallsSameArgTypeDifferentName) { + StringRef ModuleString = R"( + declare i32 @f1(i32, i32) + declare i32 @f2(i32, i32) + define i32 @f(i32 %a, i32 %b) { + bb0: + %0 = call i32 @f1(i32 %a, i32 %b) + %1 = call i32 @f2(i32 %a, i32 %b) + ret i32 0 + })"; + LLVMContext Context; + std::unique_ptr M = makeLLVMModule(Context, ModuleString); + + std::vector InstrList; + std::vector UnsignedVec; + + SpecificBumpPtrAllocator InstDataAllocator; + SpecificBumpPtrAllocator IDLAllocator; + IRInstructionMapper Mapper(&InstDataAllocator, &IDLAllocator); + getVectors(*M, Mapper, InstrList, UnsignedVec); + + ASSERT_EQ(InstrList.size(), UnsignedVec.size()); + ASSERT_EQ(UnsignedVec.size(), static_cast(3)); + ASSERT_NE(UnsignedVec[0], UnsignedVec[1]); +} + +// Here we check that a calls with different names, and different arguments +// types are mapped to different value. +TEST(IRInstructionMapper, CallsDifferentArgTypeDifferentName) { + StringRef ModuleString = R"( + declare i32 @f1(i32, i32) + declare i32 @f2(i32) + define i32 @f(i32 %a, i32 %b) { + bb0: + %0 = call i32 @f1(i32 %a, i32 %b) + %1 = call i32 @f2(i32 %a) + ret i32 0 + })"; + LLVMContext Context; + std::unique_ptr M = makeLLVMModule(Context, ModuleString); + + std::vector InstrList; + std::vector UnsignedVec; + + SpecificBumpPtrAllocator InstDataAllocator; + SpecificBumpPtrAllocator IDLAllocator; + IRInstructionMapper Mapper(&InstDataAllocator, &IDLAllocator); + getVectors(*M, Mapper, InstrList, UnsignedVec); + + ASSERT_EQ(InstrList.size(), UnsignedVec.size()); + ASSERT_EQ(UnsignedVec.size(), static_cast(3)); + ASSERT_NE(UnsignedVec[0], UnsignedVec[1]); +} + +// Here we check that calls with different names, and different return +// types are mapped to different value. +TEST(IRInstructionMapper, CallsDifferentReturnTypeDifferentName) { + StringRef ModuleString = R"( + declare i64 @f1(i32, i32) + declare i32 @f2(i32, i32) + define i32 @f(i32 %a, i32 %b) { + bb0: + %0 = call i64 @f1(i32 %a, i32 %b) + %1 = call i32 @f2(i32 %a, i32 %b) + ret i32 0 + })"; + LLVMContext Context; + std::unique_ptr M = makeLLVMModule(Context, ModuleString); + + std::vector InstrList; + std::vector UnsignedVec; + + SpecificBumpPtrAllocator InstDataAllocator; + SpecificBumpPtrAllocator IDLAllocator; + IRInstructionMapper Mapper(&InstDataAllocator, &IDLAllocator); + getVectors(*M, Mapper, InstrList, UnsignedVec); + + ASSERT_EQ(InstrList.size(), UnsignedVec.size()); + ASSERT_EQ(UnsignedVec.size(), static_cast(3)); + ASSERT_NE(UnsignedVec[0], UnsignedVec[1]); +} + +// Here we check that calls with the same name, types, and parameters map to the +// same unsigned integer. +TEST(IRInstructionMapper, CallsSameParameters) { + StringRef ModuleString = R"( + declare i32 @f1(i32, i32) + define i32 @f(i32 %a, i32 %b) { + bb0: + %0 = tail call fastcc i32 @f1(i32 %a, i32 %b) + %1 = tail call fastcc i32 @f1(i32 %a, i32 %b) + ret i32 0 + })"; + LLVMContext Context; + std::unique_ptr M = makeLLVMModule(Context, ModuleString); + + std::vector InstrList; + std::vector UnsignedVec; + + SpecificBumpPtrAllocator InstDataAllocator; + SpecificBumpPtrAllocator IDLAllocator; + IRInstructionMapper Mapper(&InstDataAllocator, &IDLAllocator); + getVectors(*M, Mapper, InstrList, UnsignedVec); + + ASSERT_EQ(InstrList.size(), UnsignedVec.size()); + ASSERT_EQ(UnsignedVec.size(), static_cast(3)); + ASSERT_EQ(UnsignedVec[0], UnsignedVec[1]); +} + +// Here we check that calls with different tail call settings are mapped to +// different values. +TEST(IRInstructionMapper, CallsDifferentTails) { + StringRef ModuleString = R"( + declare i32 @f1(i32, i32) + define i32 @f(i32 %a, i32 %b) { + bb0: + %0 = tail call i32 @f1(i32 %a, i32 %b) + %1 = call i32 @f1(i32 %a, i32 %b) + ret i32 0 + })"; + LLVMContext Context; + std::unique_ptr M = makeLLVMModule(Context, ModuleString); + + std::vector InstrList; + std::vector UnsignedVec; + + SpecificBumpPtrAllocator InstDataAllocator; + SpecificBumpPtrAllocator IDLAllocator; + IRInstructionMapper Mapper(&InstDataAllocator, &IDLAllocator); + getVectors(*M, Mapper, InstrList, UnsignedVec); + + ASSERT_EQ(InstrList.size(), UnsignedVec.size()); + ASSERT_EQ(UnsignedVec.size(), static_cast(3)); + ASSERT_NE(UnsignedVec[0], UnsignedVec[1]); +} + +// Here we check that calls with different calling convention settings are +// mapped to different values. +TEST(IRInstructionMapper, CallsDifferentCallingConventions) { + StringRef ModuleString = R"( + declare i32 @f1(i32, i32) + define i32 @f(i32 %a, i32 %b) { + bb0: + %0 = call fastcc i32 @f1(i32 %a, i32 %b) + %1 = call i32 @f1(i32 %a, i32 %b) + ret i32 0 + })"; + LLVMContext Context; + std::unique_ptr M = makeLLVMModule(Context, ModuleString); + + std::vector InstrList; + std::vector UnsignedVec; + + SpecificBumpPtrAllocator InstDataAllocator; + SpecificBumpPtrAllocator IDLAllocator; + IRInstructionMapper Mapper(&InstDataAllocator, &IDLAllocator); + getVectors(*M, Mapper, InstrList, UnsignedVec); + + ASSERT_EQ(InstrList.size(), UnsignedVec.size()); + ASSERT_EQ(UnsignedVec.size(), static_cast(3)); + ASSERT_NE(UnsignedVec[0], UnsignedVec[1]); +} + // Checks that an invoke instruction is mapped to be illegal. Invoke // instructions are considered to be illegal because of the change in the // control flow that is currently not recognized. @@ -1378,8 +1570,8 @@ TEST(IRInstructionMapper, RepeatedIllegalLength) { bb0: %0 = add i32 %a, %b %1 = mul i32 %a, %b - %2 = call i32 @f(i32 %a, i32 %b) - %3 = call i32 @f(i32 %a, i32 %b) + %2 = alloca i32 + %3 = alloca i32 %4 = add i32 %a, %b %5 = mul i32 %a, %b ret i32 0