[llvm] 45a4f34 - Revert "[IRSim] Adding support for isomorphic predicates"

Andrew Litteken via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 23 13:14:29 PST 2020


Author: Andrew Litteken
Date: 2020-12-23T15:14:19-06:00
New Revision: 45a4f34bd194acdfe3472406c303a97dc4e2526f

URL: https://github.com/llvm/llvm-project/commit/45a4f34bd194acdfe3472406c303a97dc4e2526f
DIFF: https://github.com/llvm/llvm-project/commit/45a4f34bd194acdfe3472406c303a97dc4e2526f.diff

LOG: Revert "[IRSim] Adding support for isomorphic predicates"

Reverting due to unit test errors between commits.

This reverts commit 050392660249c70c00e909ae4a7151ba2c766235.

Added: 
    

Modified: 
    llvm/include/llvm/Analysis/IRSimilarityIdentifier.h
    llvm/lib/Analysis/IRSimilarityIdentifier.cpp
    llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp

Removed: 
    llvm/test/Transforms/IROutliner/outlining-isomorphic-predicates.ll


################################################################################
diff  --git a/llvm/include/llvm/Analysis/IRSimilarityIdentifier.h b/llvm/include/llvm/Analysis/IRSimilarityIdentifier.h
index 99a5fcb3a578..4bc5a17b57e5 100644
--- a/llvm/include/llvm/Analysis/IRSimilarityIdentifier.h
+++ b/llvm/include/llvm/Analysis/IRSimilarityIdentifier.h
@@ -122,11 +122,6 @@ struct IRInstructionData : ilist_node<IRInstructionData> {
   /// 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<CmpInst::Predicate> RevisedPredicate;
-
   /// Gather the information that is 
diff icult 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
@@ -134,17 +129,6 @@ struct IRInstructionData : ilist_node<IRInstructionData> {
   /// 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.
@@ -177,12 +161,6 @@ struct IRInstructionData : ilist_node<IRInstructionData> {
     for (Value *V : ID.OperVals)
       OperTypes.push_back(V->getType());
 
-    if (isa<CmpInst>(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/llvm/lib/Analysis/IRSimilarityIdentifier.cpp b/llvm/lib/Analysis/IRSimilarityIdentifier.cpp
index 4ee152450c05..28b0382bba36 100644
--- a/llvm/lib/Analysis/IRSimilarityIdentifier.cpp
+++ b/llvm/lib/Analysis/IRSimilarityIdentifier.cpp
@@ -26,84 +26,15 @@ using namespace IRSimilarity;
 IRInstructionData::IRInstructionData(Instruction &I, bool Legality,
                                      IRInstructionDataList &IDList)
     : Inst(&I), Legal(Legality), IDL(&IDList) {
-  // 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<CmpInst>(&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 
diff erent candidates.
-  for (Use &OI : I.operands()) {
-    if (isa<CmpInst>(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;
-    }
-
+  // Here we collect the operands to be used to determine whether two
+  // instructions are similar to one another.
+  for (Use &OI : I.operands())
     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<CmpInst>(Inst) &&
-         "Can only get a predicate from a compare instruction");
-
-  if (RevisedPredicate.hasValue())
-    return RevisedPredicate.getValue();
-  
-  return cast<CmpInst>(Inst)->getPredicate();
 }
 
 bool IRSimilarity::isClose(const IRInstructionData &A,
                            const IRInstructionData &B) {
-
-  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 
diff erent, we want to make sure that
-  // the predicates are equivalent via swapping.
-  if (isa<CmpInst>(A.Inst) && isa<CmpInst>(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<llvm::Value *, llvm::Value *> R) {
-      return std::get<0>(R)->getType() == std::get<1>(R)->getType();
-    });
-  }
-
-  return false;
+  return A.Legal && A.Inst->isSameOperationAs(B.Inst);
 }
 
 // TODO: This is the same as the MachineOutliner, and should be consolidated

diff  --git a/llvm/test/Transforms/IROutliner/outlining-isomorphic-predicates.ll b/llvm/test/Transforms/IROutliner/outlining-isomorphic-predicates.ll
deleted file mode 100644
index 16233beff87c..000000000000
--- a/llvm/test/Transforms/IROutliner/outlining-isomorphic-predicates.ll
+++ /dev/null
@@ -1,170 +0,0 @@
-; 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 
diff erent kind of predicate that is not compatible
-; with the ones above, and should use a 
diff erent outlined function.
-; The other 
diff erence 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/llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp b/llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp
index 06a98a926350..1eeba4fbf7c5 100644
--- a/llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp
+++ b/llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp
@@ -154,9 +154,8 @@ TEST(IRInstructionMapper, PredicateDifferentiation) {
   ASSERT_TRUE(UnsignedVec[0] != UnsignedVec[1]);
 }
 
-// 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.
+// Checks that predicates with the same swapped predicate map to 
diff erent
+// values.
 TEST(IRInstructionMapper, PredicateIsomorphism) {
   StringRef ModuleString = R"(
                           define i32 @f(i32 %a, i32 %b) {
@@ -178,7 +177,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.
@@ -1376,49 +1375,6 @@ 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<Module> M = makeLLVMModule(Context, ModuleString);
-
-  std::vector<IRInstructionData *> InstrList;
-  std::vector<unsigned> UnsignedVec;
-
-  getVectors(*M, InstrList, UnsignedVec);
-
-
-  ASSERT_TRUE(InstrList.size() > 5);
-  ASSERT_TRUE(InstrList.size() == UnsignedVec.size());
-
-  std::vector<IRInstructionData *>::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 
diff erentiate between instructions that have 
diff erent opcodes.
 TEST(IRSimilarityCandidate, CheckRegionsDifferentInstruction) {
@@ -1611,61 +1567,6 @@ 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<Module> M = makeLLVMModule(Context, ModuleString);
-
-  std::vector<IRInstructionData *> InstrList;
-  std::vector<unsigned> UnsignedVec;
-
-  getVectors(*M, InstrList, UnsignedVec);
-
-  ASSERT_TRUE(InstrList.size() > 5);
-  ASSERT_TRUE(InstrList.size() == UnsignedVec.size());
-
-  ASSERT_TRUE(longSimCandCompare(InstrList, true));
-}
-
-// Checks that 
diff erent 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<Module> M = makeLLVMModule(Context, ModuleString);
-
-  std::vector<IRInstructionData *> InstrList;
-  std::vector<unsigned> UnsignedVec;
-
-  getVectors(*M, 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) {
@@ -1897,39 +1798,6 @@ 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 
diff erent 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<Module> M = makeLLVMModule(Context, ModuleString);
-
-  std::vector<std::vector<IRSimilarityCandidate>> SimilarityCandidates;
-  getSimilarities(*M, SimilarityCandidates);
-
-  ASSERT_TRUE(SimilarityCandidates.size() == 1);
-  for (std::vector<IRSimilarityCandidate> &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


        


More information about the llvm-commits mailing list