From c73c69986e3418bcc3d7e233618ea640c2b760bc Mon Sep 17 00:00:00 2001 From: Andrew Litteken Date: Sun, 23 Aug 2020 00:30:10 -0500 Subject: [PATCH] [IRSim] Adding support for isomorphic predicates Some predicates, can be considered the same as long as the operands are flipped. For example, a > b gives the same result as b > a. This maps instructions in a greater than form, to their appropriate less than form, swapping the operands in the IRInstructionData only, allowing for more flexible matching. Tests: llvm/test/Transforms/IROutliner/outlining-isomorphic-predicates.ll llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp Reviewers: jroelofs, paquette Recommit of commit 050392660249c70c00e909ae4a7151ba2c766235 Differential Revision: https://reviews.llvm.org/D87310 --- .../llvm/Analysis/IRSimilarityIdentifier.h | 22 +++ lib/Analysis/IRSimilarityIdentifier.cpp | 77 +++++++- .../outlining-isomorphic-predicates.ll | 170 ++++++++++++++++++ .../Analysis/IRSimilarityIdentifierTest.cpp | 146 ++++++++++++++- 4 files changed, 408 insertions(+), 7 deletions(-) create mode 100644 test/Transforms/IROutliner/outlining-isomorphic-predicates.ll diff --git a/include/llvm/Analysis/IRSimilarityIdentifier.h b/include/llvm/Analysis/IRSimilarityIdentifier.h index 4bc5a17b57e..99a5fcb3a57 100644 --- a/include/llvm/Analysis/IRSimilarityIdentifier.h +++ b/include/llvm/Analysis/IRSimilarityIdentifier.h @@ -122,6 +122,11 @@ struct IRInstructionData : ilist_node { /// considered similar. bool Legal; + /// This is only relevant if we are wrapping a CmpInst where we needed to + /// change the predicate of a compare instruction from a greater than form + /// to a less than form. It is None otherwise. + Optional RevisedPredicate; + /// Gather the information that is difficult to gather for an Instruction, or /// is changed. i.e. the operands of an Instruction and the Types of those /// operands. This extra information allows for similarity matching to make @@ -129,6 +134,17 @@ struct IRInstructionData : ilist_node { /// Instruction performs the same operation. IRInstructionData(Instruction &I, bool Legality, IRInstructionDataList &IDL); + /// Get the predicate that the compare instruction is using for hashing the + /// instruction. the IRInstructionData must be wrapping a CmpInst. + CmpInst::Predicate getPredicate() const; + + /// A function that swaps the predicates to their less than form if they are + /// in a greater than form. Otherwise, the predicate is unchanged. + /// + /// \param CI - The comparison operation to find a consistent preidcate for. + /// \return the consistent comparison predicate. + static CmpInst::Predicate predicateForConsistency(CmpInst *CI); + /// Hashes \p Value based on its opcode, types, and operand types. /// Two IRInstructionData instances produce the same hash when they perform /// the same operation. @@ -161,6 +177,12 @@ struct IRInstructionData : ilist_node { for (Value *V : ID.OperVals) OperTypes.push_back(V->getType()); + if (isa(ID.Inst)) + return llvm::hash_combine( + llvm::hash_value(ID.Inst->getOpcode()), + llvm::hash_value(ID.Inst->getType()), + llvm::hash_value(ID.getPredicate()), + llvm::hash_combine_range(OperTypes.begin(), OperTypes.end())); return llvm::hash_combine( llvm::hash_value(ID.Inst->getOpcode()), llvm::hash_value(ID.Inst->getType()), diff --git a/lib/Analysis/IRSimilarityIdentifier.cpp b/lib/Analysis/IRSimilarityIdentifier.cpp index 28b0382bba3..4ee152450c0 100644 --- a/lib/Analysis/IRSimilarityIdentifier.cpp +++ b/lib/Analysis/IRSimilarityIdentifier.cpp @@ -26,15 +26,84 @@ using namespace IRSimilarity; IRInstructionData::IRInstructionData(Instruction &I, bool Legality, IRInstructionDataList &IDList) : Inst(&I), Legal(Legality), IDL(&IDList) { - // Here we collect the operands to be used to determine whether two - // instructions are similar to one another. - for (Use &OI : I.operands()) + // We check for whether we have a comparison instruction. If it is, we + // find the "less than" version of the predicate for consistency for + // comparison instructions throught the program. + if (CmpInst *C = dyn_cast(&I)) { + CmpInst::Predicate Predicate = predicateForConsistency(C); + if (Predicate != C->getPredicate()) + RevisedPredicate = Predicate; + } + + // Here we collect the operands and their types for determining whether + // the structure of the operand use matches between two different candidates. + for (Use &OI : I.operands()) { + if (isa(I) && RevisedPredicate.hasValue()) { + // If we have a CmpInst where the predicate is reversed, it means the + // operands must be reversed as well. + OperVals.insert(OperVals.begin(), OI.get()); + continue; + } + OperVals.push_back(OI.get()); + } +} + +CmpInst::Predicate IRInstructionData::predicateForConsistency(CmpInst *CI) { + switch (CI->getPredicate()) { + case CmpInst::FCMP_OGT: + case CmpInst::FCMP_UGT: + case CmpInst::FCMP_OGE: + case CmpInst::FCMP_UGE: + case CmpInst::ICMP_SGT: + case CmpInst::ICMP_UGT: + case CmpInst::ICMP_SGE: + case CmpInst::ICMP_UGE: + return CI->getSwappedPredicate(); + default: + return CI->getPredicate(); + } +} + +CmpInst::Predicate IRInstructionData::getPredicate() const { + assert(isa(Inst) && + "Can only get a predicate from a compare instruction"); + + if (RevisedPredicate.hasValue()) + return RevisedPredicate.getValue(); + + return cast(Inst)->getPredicate(); } bool IRSimilarity::isClose(const IRInstructionData &A, const IRInstructionData &B) { - return A.Legal && A.Inst->isSameOperationAs(B.Inst); + + if (!A.Legal || !B.Legal) + return false; + + // Check if we are performing the same sort of operation on the same types + // but not on the same values. + if (A.Inst->isSameOperationAs(B.Inst)) + return true; + + // If there is a predicate, this means that either there is a swapped + // predicate, or that the types are different, we want to make sure that + // the predicates are equivalent via swapping. + if (isa(A.Inst) && isa(B.Inst)) { + + if (A.getPredicate() != B.getPredicate()) + return false; + + // If the predicates are the same via swap, make sure that the types are + // still the same. + auto ZippedTypes = zip(A.OperVals, B.OperVals); + + return all_of(ZippedTypes, [](std::tuple R) { + return std::get<0>(R)->getType() == std::get<1>(R)->getType(); + }); + } + + return false; } // TODO: This is the same as the MachineOutliner, and should be consolidated diff --git a/test/Transforms/IROutliner/outlining-isomorphic-predicates.ll b/test/Transforms/IROutliner/outlining-isomorphic-predicates.ll new file mode 100644 index 00000000000..16233beff87 --- /dev/null +++ b/test/Transforms/IROutliner/outlining-isomorphic-predicates.ll @@ -0,0 +1,170 @@ +; NOTE: Assertions have been autogenerated by utils/update_test_checks.py +; RUN: opt -S -verify -iroutliner < %s | FileCheck %s + +; This test checks the isomorphic comparisons can be outlined together into one +; function. + +; The following three function are identical, except that in the third, the +; operand order, and predicate are swapped, meaning it is structurally the same +; and should be outlined together. + +define void @outline_slt1() { +; CHECK-LABEL: @outline_slt1( +; CHECK-NEXT: entry: +; CHECK-NEXT: [[A:%.*]] = alloca i32, align 4 +; CHECK-NEXT: [[B:%.*]] = alloca i32, align 4 +; CHECK-NEXT: call void @outlined_ir_func_1(i32* [[A]], i32* [[B]]) +; CHECK-NEXT: ret void +; +entry: + %a = alloca i32, align 4 + %b = alloca i32, align 4 + store i32 2, i32* %a, align 4 + store i32 3, i32* %b, align 4 + %al = load i32, i32* %a + %bl = load i32, i32* %b + %0 = icmp slt i32 %al, %bl + ret void +} + +define void @outline_slt2() { +; CHECK-LABEL: @outline_slt2( +; CHECK-NEXT: entry: +; CHECK-NEXT: [[A:%.*]] = alloca i32, align 4 +; CHECK-NEXT: [[B:%.*]] = alloca i32, align 4 +; CHECK-NEXT: call void @outlined_ir_func_1(i32* [[A]], i32* [[B]]) +; CHECK-NEXT: ret void +; +entry: + %a = alloca i32, align 4 + %b = alloca i32, align 4 + store i32 2, i32* %a, align 4 + store i32 3, i32* %b, align 4 + %al = load i32, i32* %a + %bl = load i32, i32* %b + %0 = icmp slt i32 %al, %bl + ret void +} + +define void @outline_sgt() { +; CHECK-LABEL: @outline_sgt( +; CHECK-NEXT: entry: +; CHECK-NEXT: [[A:%.*]] = alloca i32, align 4 +; CHECK-NEXT: [[B:%.*]] = alloca i32, align 4 +; CHECK-NEXT: call void @outlined_ir_func_1(i32* [[A]], i32* [[B]]) +; CHECK-NEXT: ret void +; +entry: + %a = alloca i32, align 4 + %b = alloca i32, align 4 + store i32 2, i32* %a, align 4 + store i32 3, i32* %b, align 4 + %al = load i32, i32* %a + %bl = load i32, i32* %b + %0 = icmp sgt i32 %bl, %al + ret void +} + +; This has a swapped predicate, but not swapped operands, so it cannot use +; the same outlined function as the ones above. + +define void @dontoutline_sgt() { +; CHECK-LABEL: @dontoutline_sgt( +; CHECK-NEXT: entry: +; CHECK-NEXT: [[A:%.*]] = alloca i32, align 4 +; CHECK-NEXT: [[B:%.*]] = alloca i32, align 4 +; CHECK-NEXT: store i32 2, i32* [[A]], align 4 +; CHECK-NEXT: store i32 3, i32* [[B]], align 4 +; CHECK-NEXT: [[AL:%.*]] = load i32, i32* [[A]], align 4 +; CHECK-NEXT: [[BL:%.*]] = load i32, i32* [[B]], align 4 +; CHECK-NEXT: [[TMP0:%.*]] = icmp sgt i32 [[AL]], [[BL]] +; CHECK-NEXT: ret void +; +entry: + %a = alloca i32, align 4 + %b = alloca i32, align 4 + store i32 2, i32* %a, align 4 + store i32 3, i32* %b, align 4 + %al = load i32, i32* %a + %bl = load i32, i32* %b + %0 = icmp sgt i32 %al, %bl + ret void +} + +; The below functions use a different kind of predicate that is not compatible +; with the ones above, and should use a different outlined function. +; The other difference here is that the predicate with swapped operands comes +; first this time. + +define void @outline_ugt1() { +; CHECK-LABEL: @outline_ugt1( +; CHECK-NEXT: entry: +; CHECK-NEXT: [[A:%.*]] = alloca i32, align 4 +; CHECK-NEXT: [[B:%.*]] = alloca i32, align 4 +; CHECK-NEXT: call void @outlined_ir_func_0(i32* [[A]], i32* [[B]]) +; CHECK-NEXT: ret void +; +entry: + %a = alloca i32, align 4 + %b = alloca i32, align 4 + store i32 2, i32* %a, align 4 + store i32 3, i32* %b, align 4 + %al = load i32, i32* %a + %bl = load i32, i32* %b + %0 = icmp ugt i32 %al, %bl + ret void +} + +define void @outline_ugt2() { +; CHECK-LABEL: @outline_ugt2( +; CHECK-NEXT: entry: +; CHECK-NEXT: [[A:%.*]] = alloca i32, align 4 +; CHECK-NEXT: [[B:%.*]] = alloca i32, align 4 +; CHECK-NEXT: call void @outlined_ir_func_0(i32* [[A]], i32* [[B]]) +; CHECK-NEXT: ret void +; +entry: + %a = alloca i32, align 4 + %b = alloca i32, align 4 + store i32 2, i32* %a, align 4 + store i32 3, i32* %b, align 4 + %al = load i32, i32* %a + %bl = load i32, i32* %b + %0 = icmp ugt i32 %al, %bl + ret void +} + +define void @outline_ult() { +; CHECK-LABEL: @outline_ult( +; CHECK-NEXT: entry: +; CHECK-NEXT: [[A:%.*]] = alloca i32, align 4 +; CHECK-NEXT: [[B:%.*]] = alloca i32, align 4 +; CHECK-NEXT: call void @outlined_ir_func_0(i32* [[A]], i32* [[B]]) +; CHECK-NEXT: ret void +; +entry: + %a = alloca i32, align 4 + %b = alloca i32, align 4 + store i32 2, i32* %a, align 4 + store i32 3, i32* %b, align 4 + %al = load i32, i32* %a + %bl = load i32, i32* %b + %0 = icmp ult i32 %bl, %al + ret void +} + +; CHECK: define internal void @outlined_ir_func_0(i32* [[ARG0:%.*]], i32* [[ARG1:%.*]]) #0 { +; CHECK: entry_to_outline: +; CHECK-NEXT: store i32 2, i32* [[ARG0]], align 4 +; CHECK-NEXT: store i32 3, i32* [[ARG1]], align 4 +; CHECK-NEXT: [[AL:%.*]] = load i32, i32* [[ARG0]], align 4 +; CHECK-NEXT: [[BL:%.*]] = load i32, i32* [[ARG1]], align 4 +; CHECK-NEXT: [[TMP0:%.*]] = icmp ugt i32 [[AL]], [[BL]] + +; CHECK: define internal void @outlined_ir_func_1(i32* [[ARG0:%.*]], i32* [[ARG1:%.*]]) #0 { +; CHECK: entry_to_outline: +; CHECK-NEXT: store i32 2, i32* [[ARG0]], align 4 +; CHECK-NEXT: store i32 3, i32* [[ARG1]], align 4 +; CHECK-NEXT: [[AL:%.*]] = load i32, i32* [[ARG0]], align 4 +; CHECK-NEXT: [[BL:%.*]] = load i32, i32* [[ARG1]], align 4 +; CHECK-NEXT: [[TMP0:%.*]] = icmp slt i32 [[AL]], [[BL]] diff --git a/unittests/Analysis/IRSimilarityIdentifierTest.cpp b/unittests/Analysis/IRSimilarityIdentifierTest.cpp index 1eeba4fbf7c..a28847b4712 100644 --- a/unittests/Analysis/IRSimilarityIdentifierTest.cpp +++ b/unittests/Analysis/IRSimilarityIdentifierTest.cpp @@ -154,8 +154,9 @@ TEST(IRInstructionMapper, PredicateDifferentiation) { ASSERT_TRUE(UnsignedVec[0] != UnsignedVec[1]); } -// Checks that predicates with the same swapped predicate map to different -// values. +// Checks that predicates where that can be considered the same when the +// operands are swapped, i.e. greater than to less than are mapped to the same +// unsigned integer. TEST(IRInstructionMapper, PredicateIsomorphism) { StringRef ModuleString = R"( define i32 @f(i32 %a, i32 %b) { @@ -177,7 +178,7 @@ TEST(IRInstructionMapper, PredicateIsomorphism) { ASSERT_TRUE(InstrList.size() == UnsignedVec.size()); ASSERT_TRUE(UnsignedVec.size() == 3); - ASSERT_TRUE(UnsignedVec[0] != UnsignedVec[1]); + ASSERT_TRUE(UnsignedVec[0] == UnsignedVec[1]); } // Checks that the same predicate maps to the same value. @@ -1375,6 +1376,51 @@ TEST(IRSimilarityCandidate, CheckIdenticalInstructions) { ASSERT_TRUE(IRSimilarityCandidate::isSimilar(Cand1, Cand2)); } +// Checks that comparison instructions are found to be similar instructions +// when the operands are flipped and the predicate is also swapped. +TEST(IRSimilarityCandidate, PredicateIsomorphism) { + StringRef ModuleString = R"( + define i32 @f(i32 %a, i32 %b) { + bb0: + %0 = icmp sgt i32 %a, %b + %1 = add i32 %b, %a + br label %bb1 + bb1: + %2 = icmp slt i32 %a, %b + %3 = add i32 %a, %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_TRUE(InstrList.size() > 5); + ASSERT_TRUE(InstrList.size() == UnsignedVec.size()); + + std::vector::iterator Start, End; + Start = InstrList.begin(); + End = InstrList.begin(); + + std::advance(End, 1); + IRSimilarityCandidate Cand1(0, 2, *Start, *End); + + Start = InstrList.begin(); + End = InstrList.begin(); + + std::advance(Start, 3); + std::advance(End, 4); + IRSimilarityCandidate Cand2(3, 2, *Start, *End); + + ASSERT_TRUE(IRSimilarityCandidate::isSimilar(Cand1, Cand2)); +} + // Checks that IRSimilarityCandidates wrapping these two regions of instructions // are able to differentiate between instructions that have different opcodes. TEST(IRSimilarityCandidate, CheckRegionsDifferentInstruction) { @@ -1567,6 +1613,67 @@ TEST(IRSimilarityCandidate, DifferentStructure) { ASSERT_FALSE(longSimCandCompare(InstrList, true)); } +// Checks that comparison instructions are found to have the same structure +// when the operands are flipped and the predicate is also swapped. +TEST(IRSimilarityCandidate, PredicateIsomorphismStructure) { + StringRef ModuleString = R"( + define i32 @f(i32 %a, i32 %b) { + bb0: + %0 = icmp sgt i32 %a, %b + %1 = add i32 %a, %b + br label %bb1 + bb1: + %2 = icmp slt i32 %b, %a + %3 = add i32 %a, %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_TRUE(InstrList.size() > 5); + ASSERT_TRUE(InstrList.size() == UnsignedVec.size()); + + ASSERT_TRUE(longSimCandCompare(InstrList, true)); +} + +// Checks that different predicates are counted as diferent. +TEST(IRSimilarityCandidate, PredicateDifference) { + StringRef ModuleString = R"( + define i32 @f(i32 %a, i32 %b) { + bb0: + %0 = icmp sge i32 %a, %b + %1 = add i32 %b, %a + br label %bb1 + bb1: + %2 = icmp slt i32 %b, %a + %3 = add i32 %a, %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_TRUE(InstrList.size() > 5); + ASSERT_TRUE(InstrList.size() == UnsignedVec.size()); + + ASSERT_FALSE(longSimCandCompare(InstrList)); +} + // Checks that the same structure is recognized between two candidates. The // items %a and %b are used in the same way in both sets of instructions. TEST(IRSimilarityCandidate, SameStructure) { @@ -1798,6 +1905,39 @@ TEST(IRSimilarityIdentifier, MappingSimilarity) { } } +// Check that we find instances of swapped predicate isomorphism. That is, +// for predicates that can be flipped, e.g. greater than to less than, +// we can identify that instances of these different literal predicates, but are +// the same within a single swap can be found. +TEST(IRSimilarityIdentifier, PredicateIsomorphism) { + StringRef ModuleString = R"( + define i32 @f(i32 %a, i32 %b) { + bb0: + %0 = add i32 %a, %b + %1 = icmp sgt i32 %b, %a + br label %bb1 + bb1: + %2 = add i32 %a, %b + %3 = icmp slt i32 %a, %b + ret i32 0 + })"; + LLVMContext Context; + std::unique_ptr M = makeLLVMModule(Context, ModuleString); + + std::vector> SimilarityCandidates; + getSimilarities(*M, SimilarityCandidates); + + ASSERT_TRUE(SimilarityCandidates.size() == 1); + for (std::vector &Cands : SimilarityCandidates) { + ASSERT_TRUE(Cands.size() == 2); + unsigned InstIdx = 0; + for (IRSimilarityCandidate &Cand : Cands) { + ASSERT_TRUE(Cand.getStartIdx() == InstIdx); + InstIdx += 3; + } + } +} + // Checks that constants are detected as the same operand in each use in the // sequences of instructions. Also checks that we can find structural // equivalence using constants. In this case the 1 has the same use pattern as